Skip to content

Conversation

@newpavlov
Copy link
Member

Based on #760

@newpavlov newpavlov requested a review from josephlr December 3, 2025 17:13
@newpavlov newpavlov changed the title Replace util_libc with impl_utils macro Replace util_libc and sanitizer with impl_utils macro Dec 3, 2025
@newpavlov newpavlov changed the title Replace util_libc and sanitizer with impl_utils macro Replace util_libc and sanitizer modules with impl_utils macro Dec 3, 2025
(get_errno) => {
cfg_if! {
if #[cfg(any(target_os = "netbsd", target_os = "openbsd", target_os = "android", target_os = "cygwin"))] {
use libc::__errno as errno_location;
Copy link
Member Author

@newpavlov newpavlov Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, we could move these imports into backends, but I think it will result in a more convoluted and harder to review code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personal opinion: that would be better

Copy link
Contributor

@tamird tamird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

(get_errno) => {
cfg_if! {
if #[cfg(any(target_os = "netbsd", target_os = "openbsd", target_os = "android", target_os = "cygwin"))] {
use libc::__errno as errno_location;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personal opinion: that would be better

@briansmith
Copy link
Contributor

What is the purpose of this change?

@newpavlov
Copy link
Member Author

newpavlov commented Dec 5, 2025

It's my take on #760 which removes reliance on the uncommon #[path = "..."] attribute. It's debatable whether the reliance on macros is better than the attribute. Personally, I am fine with either.

Though one change which I think we should certainly do is to move the sanitizer module out of the backends folder since it is not a "backend".

@briansmith
Copy link
Contributor

Though one change which I think we should certainly do is to move the sanitizer module out of the backends folder since it is not a "backend".

It is an implementation detail of some backends so I think it is fine there. IMO not worth the churn. But, IDC much.

@briansmith
Copy link
Contributor

It's my take on #760 which removes reliance on the uncommon #[path = "..."] attribute.

It is an ugly thing but I don't think it's worth introducing additional complexity. In other projects, I tend to use mod X_Y; use X_Y as X instead of #[path = "X_Y.rs"] mod X to get the same effect.

I don't think the macro is an improvement.

@tamird
Copy link
Contributor

tamird commented Dec 5, 2025

It's my take on #760 which removes reliance on the uncommon #[path = "..."] attribute.

It is an ugly thing but I don't think it's worth introducing additional complexity. In other projects, I tend to use mod X_Y; use X_Y as X instead of #[path = "X_Y.rs"] mod X to get the same effect.

I don't think the macro is an improvement.

That pattern wouldn't work here because the module compiles only on certain targets. This is why I think #763 (comment) is an improvement; it would make that more clear.

@briansmith
Copy link
Contributor

That pattern wouldn't work here because the module compiles only on certain targets. This is why I think #763 (comment) is an improvement; it would make that more clear.

It does work just fine; just the mod X_Y; use X_Y as X has to be inside the conditional compilation logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants