Skip to content
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

rustc_target: Remove some redundant target properties #98214

Merged
merged 1 commit into from
Jun 24, 2022

Conversation

petrochenkov
Copy link
Contributor

is_like_emscripten is equivalent to os == "emscripten", so it's removed.
is_like_fuchsia is equivalent to os == "fuchsia", so it's removed.
is_like_osx also falls into the same category and is equivalent to vendor == "apple", but it's commonly used so I kept it as is for now.

is_like_(solaris,windows,wasm) are combinations of different operating systems or architectures (see compiler/rustc_target/src/spec/tests/tests_impl.rs) so they are also kept as is.

I think is_like_wasm (and maybe is_like_osx) are sufficiently closed sets, so we can remove these fields as well and replace them with methods like fn is_like_wasm() { arch == "wasm32" || arch == "wasm64" }.
On other hand, is_like_solaris and is_like_windows are sufficiently open and I can imagine custom targets introducing other values for os.
This is kind of a gray area.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 17, 2022
@rust-highfive
Copy link
Collaborator

r? @compiler-errors

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 17, 2022
@petrochenkov
Copy link
Contributor Author

is_like_wasm was added pretty recently together with wasm64.
Seems reasonable to remove the field, new architectures are not added every day, and you just can't realistically do that in a custom target spec.

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 24, 2022

📌 Commit 37fd294 has been approved by compiler-errors

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 24, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 24, 2022
rustc_target: Remove some redundant target properties

`is_like_emscripten` is equivalent to `os == "emscripten"`, so it's removed.
`is_like_fuchsia` is equivalent to `os == "fuchsia"`, so it's removed.
`is_like_osx` also falls into the same category and is equivalent to `vendor == "apple"`, but it's commonly used so I kept it as is for now.

`is_like_(solaris,windows,wasm)` are combinations of different operating systems or architectures (see compiler/rustc_target/src/spec/tests/tests_impl.rs) so they are also kept as is.

I think `is_like_wasm` (and maybe `is_like_osx`) are sufficiently closed sets, so we can remove these fields as well and replace them with methods like `fn is_like_wasm() { arch == "wasm32" || arch == "wasm64" }`.
On other hand, `is_like_solaris` and `is_like_windows` are sufficiently open and I can imagine custom targets introducing other values for `os`.
This is kind of a gray area.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2022
Rollup of 9 pull requests

Successful merges:

 - rust-lang#91264 (Add macro support in jump to definition feature)
 - rust-lang#96955 (Remove (transitive) reliance on sorting by DefId in pretty-printer)
 - rust-lang#97633 (Session object: Set OS/ABI)
 - rust-lang#98039 (Fix `panic` message for `BTreeSet`'s `range` API and document `panic` cases)
 - rust-lang#98214 (rustc_target: Remove some redundant target properties)
 - rust-lang#98280 (Improve suggestion for calling fn-like expr on type mismatch)
 - rust-lang#98394 (Fixup missing renames from `#[main]` to `#[rustc_main]`)
 - rust-lang#98411 (Update tendril)
 - rust-lang#98419 (Remove excess rib while resolving closures)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 33eb3c0 into rust-lang:master Jun 24, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants