-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed consistency of Apple simulator target's ABI #103455
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
|
I'm not a reviewer for compiler changes, but FWIW this looks right to me. |
r? compiler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I'm not entirely sure what the implications of changing this is for users of these targets.
Previously we only had This got worse when Apple released a way for iOS apps to be built and run on macOS (called Catalyst). We expose these via the So, if you want to fully #[cfg(all(target_os = "ios", any(target_abi = "sim", all(target_arch = "x86_64", not(target_abi = "macabi")))))] That will work before or after this lands, but is tricky to write and requires a bunch of knowledge about how things work. after this, you can just write #[cfg(all(target_os = "ios", target_abi = "sim"))] And have it work fully. Note that this already does work today on Also, this makes the tvos and watchos simulator targets consistent too -- previously they were similarly busted. |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (ce1a7e4): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. |
…, r=petrochenkov Cleanup Apple-related code in rustc_target While working on rust-lang#103455, the consistency of the `rustc_target` code for Apple's platforms was "kind of bad." There were two "base" files (`apple_base.rs` and `apple_sdk_base.rs`) that the targets each pulled some parts out of, each and all of them were written slightly differently, and sometimes missed comments other implementations had. So to hopefully make future maintenance, like implementing rust-lang/compiler-team#556, easier, this makes all of them use similar patterns and the same target base logic everywhere instead of picking bits from both. This also has some other smaller upsides like less stringly-typed functions.
…, r=petrochenkov Cleanup Apple-related code in rustc_target While working on rust-lang#103455, the consistency of the `rustc_target` code for Apple's platforms was "kind of bad." There were two "base" files (`apple_base.rs` and `apple_sdk_base.rs`) that the targets each pulled some parts out of, each and all of them were written slightly differently, and sometimes missed comments other implementations had. So to hopefully make future maintenance, like implementing rust-lang/compiler-team#556, easier, this makes all of them use similar patterns and the same target base logic everywhere instead of picking bits from both. This also has some other smaller upsides like less stringly-typed functions.
…cy, r=davidtwco Fixed consistency of Apple simulator target's ABI Currently there's a few Apple device simulator targets that are inconsistent since some set `target_abi = "sim"` (the correct thing to do) while a bunch of others don't set anything (`""`). Due to this its very hard to reliability check if some Rust code is running inside a simulator. This changes all of them to do the same thing and set `sim` as their `target_abi`. The new way to identity a simulator during compilation is as simple as `cfg(all(target_vendor="apple", target_abi = "sim"))` or even `cfg(target_abi = "sim")` being less pedantic about it. The issues with the current form (and inspiration for this) are also summarized in `@thomcc's` [Tweet](https://twitter.com/at_tcsc/status/1576685244702691328).
Currently there's a few Apple device simulator targets that are inconsistent since some set
target_abi = "sim"
(the correct thing to do) while a bunch of others don't set anything (""
). Due to this its very hard to reliability check if some Rust code is running inside a simulator. This changes all of them to do the same thing and setsim
as theirtarget_abi
.The new way to identity a simulator during compilation is as simple as
cfg(all(target_vendor="apple", target_abi = "sim"))
or evencfg(target_abi = "sim")
being less pedantic about it.The issues with the current form (and inspiration for this) are also summarized in @thomcc's Tweet.