-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
passes: check_attr
on more targets
#77015
passes: check_attr
on more targets
#77015
Conversation
r? @lcnr (rust_highfive has picked a reviewer for you, use r? to override) |
This sounds like a behavioral change to me, can you add a test/example where this is the case? |
So something like this now errors? struct Test;
enum Foo {
#[repr(u8)]
Variant,
} |
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.
Implementation LGTM, behavioral changes probably need t-lang confirmation
1f0d583
to
f82d36f
Compare
I've added the example you gave in a previous comment as a test. |
so r=me on the implementation, I think this needs signoff by @rust-lang/lang. This change now errors on disallowed attributes on enum variants, which previously either warned with "unused attribute" or did nothing. struct Test;
enum Foo {
#[repr(u8)]
//[before]~^ WARN unused attribute
//[after]~^^ ERROR attribute should be applied
Variant,
}
#[repr(u8)] //~ ERROR attribute should be applied
fn main() {} Or the following, which currently does not warn, @davidtwco, can you also add that one as a test? struct Test;
enum Foo {
#[inline]
//~^ ERROR attribute should be applied
Variant,
}
fn main() {} |
Feel free to remove the nomination if you are able to resolve this before the meeting. |
This commit modifies `check_attr` so that: - Enum variants are now checked (some attributes would not have been prohibited on variants previously). - `check_expr_attributes` and `check_stmt_attributes` are removed as `check_attributes` can perform the same checks. Signed-off-by: David Wood <[email protected]>
f82d36f
to
57e8fc5
Compare
We discussed this in the lang team meeting today. The consensus was that we'd like to do this, but that we aren't willing to do so without knowing the practical breakage from crater. (For low enough impact we're probably fine calling it a bug fix and just accepting as-is, but for high enough levels we'd want to delay it to an edition or give up and make it an allowable lint instead, with other possibilities in between too.) @bors try |
⌛ Trying commit 57e8fc5 with merge 894d32a5432c6a6461bb2aac7c3d5f2e87dce83f... |
☀️ Try build successful - checks-actions, checks-azure |
@craterbot check |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🎉 Experiment
|
https://crates.io/crates/mitrid_core/0.9.4 looks like the only regression here. Has ~250 downloads and wasn't updated in the last two years. Do we want to add a future compat period here or can we just go ahead as is? |
This seems reasonable to me. Thoughts from other @rust-lang/lang folks? |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Update from today's lang-team meeting:
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. The RFC will be merged soon. |
@bors r+ rollup=never |
📌 Commit 57e8fc5 has been approved by |
@bors retry |
☀️ Test successful - checks-actions |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Pkgsrc changes: * Adjust patches, convert tabs to spaces so that tests pass. * Remove patches which are no longer needed (upstream changed) * Minor adjustments for SunOS, e.g. disable stack probes. * Adjust cargo checksum patching accordingly. * Remove commented-out use of PATCHELF on NetBSD, which doesn't work anyway... Upstream changes: Version 1.49.0 (2020-12-31) ============================ Language ----------------------- - [Unions can now implement `Drop`, and you can now have a field in a union with `ManuallyDrop<T>`.][77547] - [You can now cast uninhabited enums to integers.][76199] - [You can now bind by reference and by move in patterns.][76119] This allows you to selectively borrow individual components of a type. E.g. ```rust #[derive(Debug)] struct Person { name: String, age: u8, } let person = Person { name: String::from("Alice"), age: 20, }; // `name` is moved out of person, but `age` is referenced. let Person { name, ref age } = person; println!("{} {}", name, age); ``` Compiler ----------------------- - [Added tier 1\* support for `aarch64-unknown-linux-gnu`.][78228] - [Added tier 2 support for `aarch64-apple-darwin`.][75991] - [Added tier 2 support for `aarch64-pc-windows-msvc`.][75914] - [Added tier 3 support for `mipsel-unknown-none`.][78676] - [Raised the minimum supported LLVM version to LLVM 9.][78848] - [Output from threads spawned in tests is now captured.][78227] - [Change os and vendor values to "none" and "unknown" for some targets][78951] \* Refer to Rust's [platform support page][forge-platform-support] for more information on Rust's tiered platform support. Libraries ----------------------- - [`RangeInclusive` now checks for exhaustion when calling `contains` and indexing.][78109] - [`ToString::to_string` now no longer shrinks the internal buffer in the default implementation.][77997] - [`ops::{Index, IndexMut}` are now implemented for fixed sized arrays of any length.][74989] Stabilized APIs --------------- - [`slice::select_nth_unstable`] - [`slice::select_nth_unstable_by`] - [`slice::select_nth_unstable_by_key`] The following previously stable methods are now `const`. - [`Poll::is_ready`] - [`Poll::is_pending`] Cargo ----------------------- - [Building a crate with `cargo-package` should now be independently reproducible.][cargo/8864] - [`cargo-tree` now marks proc-macro crates.][cargo/8765] - [Added `CARGO_PRIMARY_PACKAGE` build-time environment variable.] [cargo/8758] This variable will be set if the crate being built is one the user selected to build, either with `-p` or through defaults. - [You can now use glob patterns when specifying packages & targets.][cargo/8752] Compatibility Notes ------------------- - [Demoted `i686-unknown-freebsd` from host tier 2 to target tier 2 support.][78746] - [Macros that end with a semi-colon are now treated as statements even if they expand to nothing.][78376] - [Rustc will now check for the validity of some built-in attributes on enum variants.][77015] Previously such invalid or unused attributes could be ignored. - Leading whitespace is stripped more uniformly in documentation comments, which may change behavior. You read [this post about the changes][rustdoc-ws-post] for more details. - [Trait bounds are no longer inferred for associated types.][79904] Internal Only ------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of rustc and related tools. - [rustc's internal crates are now compiled using the `initial-exec` Thread Local Storage model.][78201] - [Calculate visibilities once in resolve.][78077] - [Added `system` to the `llvm-libunwind` bootstrap config option.][77703] - [Added `--color` for configuring terminal color support to bootstrap.][79004] [75991]: rust-lang/rust#75991 [78951]: rust-lang/rust#78951 [78848]: rust-lang/rust#78848 [78746]: rust-lang/rust#78746 [78376]: rust-lang/rust#78376 [78228]: rust-lang/rust#78228 [78227]: rust-lang/rust#78227 [78201]: rust-lang/rust#78201 [78109]: rust-lang/rust#78109 [78077]: rust-lang/rust#78077 [77997]: rust-lang/rust#77997 [77703]: rust-lang/rust#77703 [77547]: rust-lang/rust#77547 [77015]: rust-lang/rust#77015 [76199]: rust-lang/rust#76199 [76119]: rust-lang/rust#76119 [75914]: rust-lang/rust#75914 [74989]: rust-lang/rust#74989 [79004]: rust-lang/rust#79004 [78676]: rust-lang/rust#78676 [79904]: rust-lang/rust#79904 [cargo/8864]: rust-lang/cargo#8864 [cargo/8765]: rust-lang/cargo#8765 [cargo/8758]: rust-lang/cargo#8758 [cargo/8752]: rust-lang/cargo#8752 [`slice::select_nth_unstable`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable [`slice::select_nth_unstable_by`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by [`slice::select_nth_unstable_by_key`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by_key [`hint::spin_loop`]: https://doc.rust-lang.org/stable/std/hint/fn.spin_loop.html [`Poll::is_ready`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_ready [`Poll::is_pending`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_pending [rustdoc-ws-post]: https://blog.guillaume-gomez.fr/articles/2020-11-11+New+doc+comment+handling+in+rustdoc
This PR modifies
check_attr
so that:check_expr_attributes
andcheck_stmt_attributes
are removed ascheck_attributes
can perform the same checks. This means that codegen attribute errors aren't shown if there are other errors first (e.g. from other attributes, as shown insrc/test/ui/macros/issue-68060.rs
changes below).