rustc_target: Move some target options from Target to TargetOptions
#77729
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
camelid
added
A-codegen
This comment has been minimized.
This comment has been minimized.
petrochenkov
force-pushed
the
mergetarg
branch
from
881a401
to
7f49798
Compare
Merged
This comment has been minimized.
This comment has been minimized.
petrochenkov
force-pushed
the
mergetarg
branch
from
7f49798
to
05084ab
Compare
|
ping @estebank, it's been 3 weeks. |
|
r? @Mark-Simulacrum, by GitHub suggestion. |
|
@bors r+ rollup=never -- I did a lookthrough and didn't spot any obvious mistakes, but of course with such large mechanical diffs I may have missed something. I'm not too worried though, in practice it seems like for tier 1 platforms we'll catch it quickly (perhaps even in CI) if there were accidental typos or whatever. |
|
📌 Commit 05084ab85537332a0e36853fb8487bd0c9f7e386 has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 05084ab85537332a0e36853fb8487bd0c9f7e386 with merge 4ac1d90f32b90b154035fa4f93e22c5627bc87b4... |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
This needs a rebase, new targets were added. |
petrochenkov
added
S-waiting-on-author
petrochenkov
force-pushed
the
mergetarg
branch
from
05084ab
to
c0c0597
Compare
|
@bors r=Mark-Simulacrum |
|
📌 Commit c0c0597 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
This was referenced Nov 8, 2020
bjorn3
pushed a commit
to rust-lang/rustc_codegen_cranelift
that referenced
this pull request
Nov 13, 2020
rustc_target: Further cleanup use of target options Follow up to rust-lang/rust#77729. Implements items 2 and 4 from the list in rust-lang/rust#77729 (comment). The first commit collapses uses of `target.options.foo` into `target.foo`. The second commit renames some target options to avoid tautology: `target.target_endian` -> `target.endian` `target.target_c_int_width` -> `target.c_int_width` `target.target_os` -> `target.os` `target.target_env` -> `target.env` `target.target_vendor` -> `target.vendor` `target.target_family` -> `target.os_family` `target.target_mcount` -> `target.mcount` r? `@Mark-Simulacrum`
Merged
workingjubilee
added a commit
to workingjubilee/rustc
that referenced
this pull request
Nov 15, 2021
Android is not GNU For a long time, the Android targets had `target_env=""`, but this changed to `"gnu"` in Rust 1.49.0. I tracked this down to rust-lang#77729 which started setting `"gnu"` in the `linux_base` target options, and this was inherited by `android_base`. Then rust-lang#78929 split the env into `linux_gnu_base`, but `android_base` was also changed to follow that. Android was not specifically mentioned in either pull request, so I believe this was an accident. Moving it back to `linux_base` will use an empty `env` again. r? `@Mark-Simulacrum` cc `@petrochenkov`
workingjubilee
added a commit
to workingjubilee/rustc
that referenced
this pull request
Nov 15, 2021
Android is not GNU For a long time, the Android targets had `target_env=""`, but this changed to `"gnu"` in Rust 1.49.0. I tracked this down to rust-lang#77729 which started setting `"gnu"` in the `linux_base` target options, and this was inherited by `android_base`. Then rust-lang#78929 split the env into `linux_gnu_base`, but `android_base` was also changed to follow that. Android was not specifically mentioned in either pull request, so I believe this was an accident. Moving it back to `linux_base` will use an empty `env` again. r? ``@Mark-Simulacrum`` cc ``@petrochenkov``
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Nov 15, 2021
Android is not GNU For a long time, the Android targets had `target_env=""`, but this changed to `"gnu"` in Rust 1.49.0. I tracked this down to rust-lang#77729 which started setting `"gnu"` in the `linux_base` target options, and this was inherited by `android_base`. Then rust-lang#78929 split the env into `linux_gnu_base`, but `android_base` was also changed to follow that. Android was not specifically mentioned in either pull request, so I believe this was an accident. Moving it back to `linux_base` will use an empty `env` again. r? ```@Mark-Simulacrum``` cc ```@petrochenkov```
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Nov 16, 2021
Android is not GNU For a long time, the Android targets had `target_env=""`, but this changed to `"gnu"` in Rust 1.49.0. I tracked this down to rust-lang#77729 which started setting `"gnu"` in the `linux_base` target options, and this was inherited by `android_base`. Then rust-lang#78929 split the env into `linux_gnu_base`, but `android_base` was also changed to follow that. Android was not specifically mentioned in either pull request, so I believe this was an accident. Moving it back to `linux_base` will use an empty `env` again. r? ````@Mark-Simulacrum```` cc ````@petrochenkov````
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The only reason for
TargettoTargetOptionsto be separate structures is that options inTargetOptionshave reasonable defaults and options inTargetdon't.(Otherwise all the options logically belong to a single
Targetstruct.)This PR moves a number of options with reasonable defaults from
TargettoTargetOptions, so they no longer needs to be specified explicitly for majority of the targets.The move also allows to inherit the options from
rustc_target/src/spec/*_base.rsfiles in a nicer way.I didn't change any specific option values here.
The moved options are
target_c_int_width(defaults to"32"),target_endian(defaults to"little"),target_os(defaults to"none"),target_env(defaults to""),target_vendor(defaults to"unknown") andlinker_flavor(defaults toLinkerFlavor::Gcc).Next steps (in later PRs):
TargetOptionsintoTargetIf not, always accessDone in rustc_target: Further cleanup use of target options #78875.TargetOptionsfields throughDerefmaking it a part ofTargetat least logically (session.target.target.options.foo->session.target.target.foo)EliminateDone in No more target.target #77943.session::config::Configand useTargetinstead (session.target.target.foo->session.target.foo)Avoid tautologies in option names (Done in rustc_target: Further cleanup use of target options #78875.target.target_os->target.os)Resolve _ rustc_target: The differences betweenDone in rustc_target: Change os and vendor values to "none" and "unknown" for some targets #78951.target_os = "none"andtarget_os = "unknown", andtarget_vendor = "unknown"andtarget_vendor = ""are unclear #77730 (rustc_target: The differences betweentarget_os = "none"andtarget_os = "unknown", andtarget_vendor = "unknown"andtarget_vendor = ""are unclear) noticed during implementation of this PR.