rustc_target: introduce Abi, Env, Os by tamird · Pull Request #148531 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation66 Commits7 Checks11 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
Improve type safety by using an enum rather than strings.
I'm not really sure this is better since only a few vendors have special semantics. r? @nnethercote
Some changes occurred in compiler/rustc_codegen_cranelift
cc @bjorn3
These commits modify compiler targets.
(See the Target Tier Policy.)
Some changes occurred in compiler/rustc_codegen_ssa
Some changes occurred in cfg and check-cfg configuration
cc @Urgau
The Miri subtree was changed
cc @rust-lang/miri
Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS)
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
Area: Attributes (`#[…]`, `#![…]`)
Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
labels
This comment has been minimized.
rustc_target: introduce Vendor rustc_target: introduce Vendor, Abi, Env, Os
rustc_target: introduce Vendor, Abi, Env, Os rustc_target: introduce Vendor, ABI, Env, OS
This comment has been minimized.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a bunch of nits but this generally looks good. Linker is another candidate for enum-ification, if you aren't yet bored of this stuff :)
View changes since this review
| Unikraft = "unikraft", |
|---|
| Uwp = "uwp", |
| Vex = "vex", |
| Win7 = "win7", |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea what "win7" is? Makes me think of Windows 7, which is a weird name for a vendor.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dates back to #118150 which explains:
I went with naming the target x86_64-win7-windows-msvc, inserting the win7 in the vendor field (usually set to to pc). This is done to avoid ecosystem churn, as quite a few crates have cfg(target_os = "windows") or cfg(target_env = "msvc"), but nearly no cfg(target_vendor = "pc"). Since my goal is to be able to seamlessly swap to the win7 target, I figured it'd be easier this way.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, ok. Thanks for the info!
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
FWIW for a related change recently we had quite a bit of discussion: rust-lang/compiler-team#926.
IMO these changes are so similar to the change in that MCP that additional discussion isn't needed.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed all but one comment and left things unsquashed to (somewhat) ease review. I do plan to squash once you're happy with it.
View changes since this review
| Unikraft = "unikraft", |
|---|
| Uwp = "uwp", |
| Vex = "vex", |
| Win7 = "win7", |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dates back to #118150 which explains:
I went with naming the target x86_64-win7-windows-msvc, inserting the win7 in the vendor field (usually set to to pc). This is done to avoid ecosystem churn, as quite a few crates have cfg(target_os = "windows") or cfg(target_env = "msvc"), but nearly no cfg(target_vendor = "pc"). Since my goal is to be able to seamlessly swap to the win7 target, I figured it'd be easier this way.
rustc_target: introduce Vendor, ABI, Env, OS rustc_target: introduce Vendor, Abi, Env, Os
r=me with the commits squashed, thanks!
@bors delegate=tamird
✌️ @tamird, you can now approve this pull request!
If @nnethercote told you to "r=me" after making some further change, please make that change, then do @bors r=@nnethercote
This comment has been minimized.
📌 Commit cd47df7 has been approved by nnethercote
It is now in the queue for this repository.
Improve type safety by using an enum rather than strings.
Improve type safety by using an enum rather than strings.
Improve type safety by using an enum rather than strings.
rustc_target: introduce Vendor, Abi, Env, Os rustc_target: introduce Abi, Env, Os
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
📌 Commit 98a534e has been approved by nnethercote
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
Zalathar added a commit to Zalathar/rust that referenced this pull request
rustc_target: introduce Abi, Env, Os
Improve type safety by using an enum rather than strings.
I'm not really sure this is better since only a few vendors have special semantics. r? @nnethercote
bors added a commit that referenced this pull request
Rollup of 16 pull requests
Successful merges:
- #146627 (Simplify
jemallocsetup) - #147753 (Suggest add bounding value for RangeTo)
- #147832 (rustdoc: Don't pass
RenderOptionstoDocContext) - #147974 (Improve diagnostics for buffer reuse with borrowed references)
- #148080 ([rustdoc] Fix invalid jump to def macro link generation)
- #148465 (Adjust spans into the
forloops context before creating the new desugaring spans.) - #148500 (Update git index before running diff-index)
- #148531 (rustc_target: introduce Abi, Env, Os)
- #148536 (cmse: add test for
asyncandconstfunctions) - #148770 (implement
feature(c_variadic_naked_functions)) - #148780 (fix filecheck typos in tests)
- #148819 (Remove specialized warning for removed target)
- #148830 (miri subtree update)
- #148833 (Update rustbook dependencies)
- #148834 (fix(rustdoc): Color doctest errors)
- #148841 (Remove more
#[must_use]from portable-simd)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit that referenced this pull request
Rollup merge of #148531 - tamird:vendor-enum, r=nnethercote
rustc_target: introduce Abi, Env, Os
Improve type safety by using an enum rather than strings.
I'm not really sure this is better since only a few vendors have special semantics. r? @nnethercote
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
Bors, this has already been merged.
@bors r- retry
Status: This is awaiting some action (such as code changes or more information) from the author.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
tautschnig added a commit to tautschnig/kani that referenced this pull request
github-merge-queue bot pushed a commit to model-checking/kani that referenced this pull request
Relevant upstream PR:
- rust-lang/rust#148531 (rustc_target: introduce Abi, Env, Os)
Resolves: #4471
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
Labels
Area: Attributes (`#[…]`, `#![…]`)
Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS)
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.