Use consistent cfg attributes #1655
Conversation
|
Note that there is a proposal to deprecate target_vendor: rust-lang/rust#100343 / rust-lang/lang-team#102 |
Well that's great... I guess we shouldn't go with this this then. I've pushed another commit to clean up the remaining attributes which I've found while doing the previous work. |
Thomasdezeeuw
force-pushed
the
target-apple
branch
from
2d600f0 to
1c5c321
Compare
Thomasdezeeuw
changed the title
There was a problem hiding this comment.
This looks good to me. However, I did notice while reading rust-lang/lang-team#102 that there's also a target_family = "apple" cfg that's set for both iOS and macOS. Would it be a bit simpler to use that rather than testing for both target_os cfgs?
| ))] | ||
| let socket_type = socket_type | libc::SOCK_NONBLOCK | libc::SOCK_CLOEXEC; | ||
|
|
||
| let socket = syscall!(socket(domain, socket_type, 0))?; | ||
|
|
||
| // Mimick `libstd` and set `SO_NOSIGPIPE` on apple systems. | ||
| #[cfg(target_vendor = "apple")] | ||
| #[cfg(any(target_os = "ios", target_os = "macos"))] |
There was a problem hiding this comment.
Nit, take it or leave it: rust-lang/lang-team#102 also mentions the existence of target_family = "apple", which is set in addition to target_family = "unix" when building for both iOS and macOS. It might be a little bit simpler to use target_family = "apple" here instead?
| #[cfg(any(target_os = "ios", target_os = "macos"))] | ||
| type Filter = i16; | ||
| #[cfg(target_os = "netbsd")] | ||
| type Filter = u32; | ||
|
|
||
| // Type of the `flags` field in the `kevent` structure. | ||
| #[cfg(any(target_os = "dragonfly", target_os = "freebsd", target_os = "openbsd"))] | ||
| type Flags = libc::c_ushort; | ||
| #[cfg(any(target_os = "macos", target_os = "ios"))] | ||
| #[cfg(any(target_os = "ios", target_os = "macos"))] |
There was a problem hiding this comment.
similarly, would target_family="apple" make sense for these?
| #[cfg(any(target_os = "ios", target_os = "macos"))] | ||
| libc::SO_LINGER_SEC, | ||
| #[cfg(not(target_vendor = "apple"))] | ||
| #[cfg(not(any(target_os = "ios", target_os = "macos")))] |
There was a problem hiding this comment.
Nit: I believe we could use target_family = "apple" here?
This has not yet been implemented. $ rustc --print cfg --target x86_64-apple-darwin | grep target_family
target_family="unix"
$ rustc -V
rustc 1.69.0-nightly (5b8f28453 2023-02-12) |
I agree I would prefer to use |
No description provided.