Forbid the use of #[target_feature] on main by LeSeulArtichaut · Pull Request #108651 · 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
Conversation31 Commits3 Checks0 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 }})
r? @jackh726
(rustbot has picked a reviewer for you, use r? to override)
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
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r? Nilstrieb
@bors delegate
r=me when CI is green
Uh, I don't think target_feature should be an only_local attribute.... that's unsound, isn't it? When it always pretends it isn't there across crates? Sounds like we need to encode it in the metadata (which is as simple as removing the @only_local: true)
Hm, target_feature was made local_only in #98450 as an optimization...
Ig it is fine, actually? Maybe? If the codegen only happens inside the crate, then target_feature being local is totally fine. At least we don't have any tests that tried to access target_feature in non-local crate...
This isn't unsound, the safeaty check probably goes through target feature queries instead of the attr.
So I guess you should try figuring out how that works and whether you can use the same way - although encoding target_feature is also a possible solution.
the safeaty check probably goes through target feature queries instead of the attr
Yes, see
| let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features; |
|---|
| // The body might be a constant, so it doesn't have codegen attributes. |
| let self_features = &self.tcx.body_codegen_attrs(self.body_did.to_def_id()).target_features; |
Also we need to forbid #[target_feature] on start, no?
Now it looks good and green 😊
@bors r+
📌 Commit dc6e0d9 has been approved by Nilstrieb
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-review
Status: Awaiting review from the assignee but also interested parties.
labels
compiler-errors added a commit to compiler-errors/rust that referenced this pull request
…ure-on-main, r=Nilstrieb
Forbid the use of #[target_feature] on main
Fixes rust-lang#108645.
@bors r- this will fail after the revert, needs to add the unstable feature to the test
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
(I'll need to add the feature gates back if #108654 lands first)
@bors r-
Fair chance this will land after #108654, so this needs blessing and fixing.
Some changes occurred in src/tools/cargo
cc @ehuss
(Committed a submodule change by accident, sorry for the noise)
📌 Commit 28dca77acf1d7280b51372abf3dd50c14616b628 has been approved by Nilstrieb
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
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
📌 Commit 29b1789 has been approved by Nilstrieb
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
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#108651 (Forbid the use of
#[target_feature]onmain) - rust-lang#109009 (rustdoc: use restricted Damerau-Levenshtein distance for search)
- rust-lang#109026 (Introduce
Rc::into_inner, as a parallel toArc::into_inner) - rust-lang#109029 (Gate usages of
dyn*and const closures in macros) - rust-lang#109031 (Rename
config.toml.exampletoconfig.example.toml) - rust-lang#109032 (Use
TyCtxt::trait_solver_nextin some places) - rust-lang#109047 (typo)
- rust-lang#109052 (Add eslint check for rustdoc-gui tester)
Failed merges:
r? @ghost
@rustbot modify labels: rollup
Labels
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.