Warn on codegen attributes on required trait methods#148756
Warn on codegen attributes on required trait methods#148756bors merged 2 commits intorust-lang:mainfrom
Conversation
|
Some changes occurred in compiler/rustc_attr_parsing |
|
r? jdonszelmann |
|
@rfcbot fcp merge lang |
|
Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
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! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
@rfcbot reviewed |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
So, I'd like to file a concern here that we should do a crater run on this before landing. Here's my reasoning: This is landing as a FCW, which means that the team wants to remove this behavior, In my mind, this means that the team should know what the barriers are for that. If, for example, crater says that nobody is relying on it, it may mean that we want to move rather fast on that. If crater shows that a bunch of crates are relying on this, we should know that to be able to plan (I think Niko mentioned e.g. making the change over an edition in that case would be a possibility). Essentially, the concern from me is that a FCW here indicates it's important enough for people to think about, and I think that the lang team should have the data about who it effects. And, not blocking landing this on that data runs the (high) risk of it falling through the cracks. |
|
@rfcbot concern jackh726-concern |
|
@JonathanBrouwer could you make a parallel PR that makes this a full error, then we crater that. Or at least I believe we wouldn't see this warning in a crate run right? |
… r=<try> Crater for #148756
|
Crater run will happen here: #149137 |
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
c49ff7a to
8fa10c0
Compare
|
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. |
|
@rustbot ready |
|
@bors r+ rollup |
…ts2, r=jdonszelmann Warn on codegen attributes on required trait methods This PR turns applying the following attributes on required trait methods (that is, trait methods **without** a default implementation) into a FCW: - `#[cold]` - `#[link_section]` - `#[linkage]` (unstable) - `#[rustc_allow_const_fn_unstable]` (internal attribute) These attributes already had no effect when applied to a required trait method, this PR only adds a warning. Furthermore, it adds a comment in the code that the following codegen attributes are *inherited* when applied to a required trait method: - `#[track_caller]` - `#[align]` (unstable) `@rustbot` labels +I-lang-nominated `@rust-lang/lang` Two questions for the lang team: - Is adding this warning ok? - Does the current behaviour of these attributes align with that you would expect them to be? Fixes rust-lang#147432
Rollup of 9 pull requests Successful merges: - #148756 (Warn on codegen attributes on required trait methods) - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf) - #149271 (feat: dlopen Enzyme) - #149354 (Bootstrap config: libgccjit libs dir) - #149459 (std: sys: fs: uefi: Implement set_times and set_perm) - #149950 (Simplify how inline asm handles `MaybeUninit`) - #150000 (Port `#[rustc_legacy_const_generics]` to use attribute parser ) - #150014 (Metadata loader cleanups) - #150021 (document that mpmc channels deliver an item to (at most) one receiver) r? `@ghost` `@rustbot` modify labels: rollup
…ts2, r=jdonszelmann Warn on codegen attributes on required trait methods This PR turns applying the following attributes on required trait methods (that is, trait methods **without** a default implementation) into a FCW: - `#[cold]` - `#[link_section]` - `#[linkage]` (unstable) - `#[rustc_allow_const_fn_unstable]` (internal attribute) These attributes already had no effect when applied to a required trait method, this PR only adds a warning. Furthermore, it adds a comment in the code that the following codegen attributes are *inherited* when applied to a required trait method: - `#[track_caller]` - `#[align]` (unstable) ``@rustbot`` labels +I-lang-nominated ``@rust-lang/lang`` Two questions for the lang team: - Is adding this warning ok? - Does the current behaviour of these attributes align with that you would expect them to be? Fixes rust-lang#147432
Rollup of 8 pull requests Successful merges: - #148756 (Warn on codegen attributes on required trait methods) - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf) - #149271 (feat: dlopen Enzyme) - #149459 (std: sys: fs: uefi: Implement set_times and set_perm) - #149950 (Simplify how inline asm handles `MaybeUninit`) - #150000 (Port `#[rustc_legacy_const_generics]` to use attribute parser ) - #150014 (Metadata loader cleanups) - #150021 (document that mpmc channels deliver an item to (at most) one receiver) r? `@ghost` `@rustbot` modify labels: rollup
…ts2, r=jdonszelmann Warn on codegen attributes on required trait methods This PR turns applying the following attributes on required trait methods (that is, trait methods **without** a default implementation) into a FCW: - `#[cold]` - `#[link_section]` - `#[linkage]` (unstable) - `#[rustc_allow_const_fn_unstable]` (internal attribute) These attributes already had no effect when applied to a required trait method, this PR only adds a warning. Furthermore, it adds a comment in the code that the following codegen attributes are *inherited* when applied to a required trait method: - `#[track_caller]` - `#[align]` (unstable) ```@rustbot``` labels +I-lang-nominated ```@rust-lang/lang``` Two questions for the lang team: - Is adding this warning ok? - Does the current behaviour of these attributes align with that you would expect them to be? Fixes rust-lang#147432
Rollup of 14 pull requests Successful merges: - #148756 (Warn on codegen attributes on required trait methods) - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf) - #149271 (feat: dlopen Enzyme) - #149459 (std: sys: fs: uefi: Implement set_times and set_perm) - #149771 (bootstrap readme: make easy to read when editor wrapping is not enabled) - #149856 (Provide an extended framework for type visit, for use in rust-analyzer) - #149950 (Simplify how inline asm handles `MaybeUninit`) - #150014 (Metadata loader cleanups) - #150021 (document that mpmc channels deliver an item to (at most) one receiver) - #150022 (Generate macro expansion for rust compiler crates docs) - #150029 (Update books) - #150031 (assert impossible branch is impossible) - #150034 (do not add `I-prioritize` when `F-*` labels are present) - #150036 (Use the embeddable filename for coverage artifacts) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 13 pull requests Successful merges: - #148756 (Warn on codegen attributes on required trait methods) - #148790 (Add new Tier-3 target: riscv64im-unknown-none-elf) - #149271 (feat: dlopen Enzyme) - #149459 (std: sys: fs: uefi: Implement set_times and set_perm) - #149771 (bootstrap readme: make easy to read when editor wrapping is not enabled) - #149856 (Provide an extended framework for type visit, for use in rust-analyzer) - #149950 (Simplify how inline asm handles `MaybeUninit`) - #150014 (Metadata loader cleanups) - #150021 (document that mpmc channels deliver an item to (at most) one receiver) - #150029 (Update books) - #150031 (assert impossible branch is impossible) - #150034 (do not add `I-prioritize` when `F-*` labels are present) - #150036 (Use the embeddable filename for coverage artifacts) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #148756 - JonathanBrouwer:link_section_targets2, r=jdonszelmann Warn on codegen attributes on required trait methods This PR turns applying the following attributes on required trait methods (that is, trait methods **without** a default implementation) into a FCW: - `#[cold]` - `#[link_section]` - `#[linkage]` (unstable) - `#[rustc_allow_const_fn_unstable]` (internal attribute) These attributes already had no effect when applied to a required trait method, this PR only adds a warning. Furthermore, it adds a comment in the code that the following codegen attributes are *inherited* when applied to a required trait method: - `#[track_caller]` - `#[align]` (unstable) ````@rustbot```` labels +I-lang-nominated ````@rust-lang/lang```` Two questions for the lang team: - Is adding this warning ok? - Does the current behaviour of these attributes align with that you would expect them to be? Fixes #147432
Rollup of 13 pull requests Successful merges: - rust-lang/rust#148756 (Warn on codegen attributes on required trait methods) - rust-lang/rust#148790 (Add new Tier-3 target: riscv64im-unknown-none-elf) - rust-lang/rust#149271 (feat: dlopen Enzyme) - rust-lang/rust#149459 (std: sys: fs: uefi: Implement set_times and set_perm) - rust-lang/rust#149771 (bootstrap readme: make easy to read when editor wrapping is not enabled) - rust-lang/rust#149856 (Provide an extended framework for type visit, for use in rust-analyzer) - rust-lang/rust#149950 (Simplify how inline asm handles `MaybeUninit`) - rust-lang/rust#150014 (Metadata loader cleanups) - rust-lang/rust#150021 (document that mpmc channels deliver an item to (at most) one receiver) - rust-lang/rust#150029 (Update books) - rust-lang/rust#150031 (assert impossible branch is impossible) - rust-lang/rust#150034 (do not add `I-prioritize` when `F-*` labels are present) - rust-lang/rust#150036 (Use the embeddable filename for coverage artifacts) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 13 pull requests Successful merges: - rust-lang/rust#148756 (Warn on codegen attributes on required trait methods) - rust-lang/rust#148790 (Add new Tier-3 target: riscv64im-unknown-none-elf) - rust-lang/rust#149271 (feat: dlopen Enzyme) - rust-lang/rust#149459 (std: sys: fs: uefi: Implement set_times and set_perm) - rust-lang/rust#149771 (bootstrap readme: make easy to read when editor wrapping is not enabled) - rust-lang/rust#149856 (Provide an extended framework for type visit, for use in rust-analyzer) - rust-lang/rust#149950 (Simplify how inline asm handles `MaybeUninit`) - rust-lang/rust#150014 (Metadata loader cleanups) - rust-lang/rust#150021 (document that mpmc channels deliver an item to (at most) one receiver) - rust-lang/rust#150029 (Update books) - rust-lang/rust#150031 (assert impossible branch is impossible) - rust-lang/rust#150034 (do not add `I-prioritize` when `F-*` labels are present) - rust-lang/rust#150036 (Use the embeddable filename for coverage artifacts) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 13 pull requests Successful merges: - rust-lang/rust#148756 (Warn on codegen attributes on required trait methods) - rust-lang/rust#148790 (Add new Tier-3 target: riscv64im-unknown-none-elf) - rust-lang/rust#149271 (feat: dlopen Enzyme) - rust-lang/rust#149459 (std: sys: fs: uefi: Implement set_times and set_perm) - rust-lang/rust#149771 (bootstrap readme: make easy to read when editor wrapping is not enabled) - rust-lang/rust#149856 (Provide an extended framework for type visit, for use in rust-analyzer) - rust-lang/rust#149950 (Simplify how inline asm handles `MaybeUninit`) - rust-lang/rust#150014 (Metadata loader cleanups) - rust-lang/rust#150021 (document that mpmc channels deliver an item to (at most) one receiver) - rust-lang/rust#150029 (Update books) - rust-lang/rust#150031 (assert impossible branch is impossible) - rust-lang/rust#150034 (do not add `I-prioritize` when `F-*` labels are present) - rust-lang/rust#150036 (Use the embeddable filename for coverage artifacts) r? `@ghost` `@rustbot` modify labels: rollup
Pkgsrc changes: * Update version & checksums. * Adapt patches to new vendored crates. This has so far just been verified to build on NetBSD/amd64. Upstream changes relative to 1.93.1: Version 1.94.0 (2026-03-05) ========================== Language -------- - [Impls and impl items inherit `dead_code` lint level of the corresponding traits and trait items] (rust-lang/rust#144113) - [Stabilize additional 29 RISC-V target features including large portions of the RVA22U64 / RVA23U64 profiles] (rust-lang/rust#145948) - [Add warn-by-default `unused_visibilities` lint for visibility on `const _` declarations] (rust-lang/rust#147136) - [Update to Unicode 17] (rust-lang/rust#148321) - [Avoid incorrect lifetime errors for closures] (rust-lang/rust#148329) Platform Support ---------------- - [Add `riscv64im-unknown-none-elf` as a tier 3 target] (rust-lang/rust#148790) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. [platform-support-doc]: https://doc.rust-lang.org/rustc/platform-support.html Libraries --------- - [Relax `T: Ord` bound for some `BinaryHeap<T>` methods.] (rust-lang/rust#149408) Stabilized APIs --------------- - [`<[T]>::array_windows`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.array_windows) - [`<[T]>::element_offset`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.element_offset) - [`LazyCell::get`] (https://doc.rust-lang.org/stable/std/cell/struct.LazyCell.html#method.get) - [`LazyCell::get_mut`] (https://doc.rust-lang.org/stable/std/cell/struct.LazyCell.html#method.get_mut) - [`LazyCell::force_mut`] (https://doc.rust-lang.org/stable/std/cell/struct.LazyCell.html#method.force_mut) - [`LazyLock::get`] (https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html#method.get) - [`LazyLock::get_mut`] (https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html#method.get_mut) - [`LazyLock::force_mut`] (https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html#method.force_mut) - [`impl TryFrom<char> for usize`] (https://doc.rust-lang.org/stable/std/convert/trait.TryFrom.html#impl-TryFrom%3Cchar%3E-for-usize) - [`std::iter::Peekable::next_if_map`] (https://doc.rust-lang.org/stable/std/iter/struct.Peekable.html#method.next_if_map) - [`std::iter::Peekable::next_if_map_mut`] (https://doc.rust-lang.org/stable/std/iter/struct.Peekable.html#method.next_if_map_mut) - [x86 `avx512fp16` intrinsics] (rust-lang/rust#127213) (excluding those that depend directly on the unstable `f16` type) - [AArch64 NEON fp16 intrinsics] (rust-lang/rust#136306) (excluding those that depend directly on the unstable `f16` type) - [`f32::consts::EULER_GAMMA`] (https://doc.rust-lang.org/stable/std/f32/consts/constant.EULER_GAMMA.html) - [`f64::consts::EULER_GAMMA`] (https://doc.rust-lang.org/stable/std/f64/consts/constant.EULER_GAMMA.html) - [`f32::consts::GOLDEN_RATIO`] (https://doc.rust-lang.org/stable/std/f32/consts/constant.GOLDEN_RATIO.html) - [`f64::consts::GOLDEN_RATIO`] (https://doc.rust-lang.org/stable/std/f64/consts/constant.GOLDEN_RATIO.html) These previously stable APIs are now stable in const contexts: - [`f32::mul_add`] (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.mul_add) - [`f64::mul_add`] (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.mul_add) Cargo ----- - Stabilize the config include key. The top-level include config key allows loading additional config files, enabling better organization, sharing, and management of Cargo configurations across projects and environments. [docs] (https://doc.rust-lang.org/nightly/cargo/reference/config.html#including-extra-configuration-files) [#16284] (rust-lang/cargo#16284) - Stabilize the pubtime field in registry index. This records when a crate version was published and enables time-based dependency resolution in the future. Note that crates.io will gradually backfill existing packages when a new version is published. Not all crates have pubtime yet. [#16369] (rust-lang/cargo#16369) [#16372] (rust-lang/cargo#16372) - Cargo now parses [TOML v1.1](https://toml.io/en/v1.1.0) for manifests and configuration files. Note that using these features in Cargo.toml will raise your development MSRV, but the published manifest remains compatible with older parsers. [#16415] (rust-lang/cargo#16415) - [Make `CARGO_BIN_EXE_<crate>` available at runtime ] (rust-lang/cargo#16421) Compatibility Notes ------------------- - [Forbid freely casting lifetime bounds of `dyn`-types] (rust-lang/rust#136776) - [Make closure capturing have consistent and correct behaviour around patterns] (rust-lang/rust#138961) Some finer details of how precise closure captures get affected by pattern matching have been changed. In some cases, this can cause a non-move closure that was previously capturing an entire variable by move, to now capture only part of that variable by move, and other parts by borrow. This can cause the borrow checker to complain where it previously didn't, or cause `Drop` to run at a different point in time. - [Standard library macros are now imported via prelude, not via injected ` #[macro_use]`] (rust-lang/rust#139493) This will raise an error if macros of the same name are glob imported. For example if a crate defines their own `matches` macro and then glob imports that, it's now ambiguous whether the custom or standard library `matches` is meant and an explicit import of the name is required to resolve the ambiguity. One exception is `core::panic` and `std::panic`, if their import is ambiguous a new warning ([`ambiguous_panic_imports`] (rust-lang/rust#147319)) is raised. This may raise a new warning ([`ambiguous_panic_imports`] (rust-lang/rust#147319)) on `#![no_std]` code glob importing the std crate. Both `core::panic!` and `std::panic!` are then in scope and which is used is ambiguous. - [Don't strip shebang in expression-context `include!(…)`s] (rust-lang/rust#146377) This can cause previously working includes to no longer compile if they included files which started with a shebang. - [Ambiguous glob reexports are now also visible cross-crate] (rust-lang/rust#147984) This unifies behavior between local and cross-crate errors on these exports, which may introduce new ambiguity errors. - [Don't normalize where-clauses before checking well-formedness] (rust-lang/rust#148477) - [Introduce a future compatibility warning on codegen attributes on body-free trait methods] (rust-lang/rust#148756) These attributes currently have no effect in this position. - [On Windows `std::time::SystemTime::checked_sub_duration` will return `None` for times before the Windows epoch (1/1/1601)] (rust-lang/rust#148825) - [Lifetime identifiers such as `'a` are now NFC normalized] (rust-lang/rust#149192). - [Overhaul filename handling for cross-compiler consistency] (rust-lang/rust#149709) Any paths emitted by compiler now always respect the relative-ness of the paths and `--remap-path-prefix` given originally. One side-effect of this change is that paths emitted for local crates in Cargo (path dependencies and workspace members) are no longer absolute but relative when emitted as part of a diagnostic in a downstream crate. Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Switch to `annotate-snippets` for error emission] (rust-lang/rust#150032) This should preserve mostly the same outputs in rustc error messages.
This PR turns applying the following attributes on required trait methods (that is, trait methods without a default implementation) into a FCW:
#[cold]#[link_section]#[linkage](unstable)#[rustc_allow_const_fn_unstable](internal attribute)These attributes already had no effect when applied to a required trait method, this PR only adds a warning.
Furthermore, it adds a comment in the code that the following codegen attributes are inherited when applied to a required trait method:
#[track_caller]#[align](unstable)@rustbot labels +I-lang-nominated
@rust-lang/lang
Two questions for the lang team:
Fixes #147432