Coalesce `rustc_on_unimplemented` attributes and lint malformed filters by qaijuang · Pull Request #160113 · rust-lang/rust

qaijuang · 2026-07-29T01:49:06Z

Discussion on Zulip.

Best reviewed in commit order.

r? @mejrs

rustbot · 2026-07-29T09:24:51Z

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred to diagnostic attributes.

cc @mejrs

Sign in to view

-//~^ ERROR invalid flag in `on`-clause
+//~^ WARN invalid flag in `on`-clause


Bryntet · 2026-07-29T16:03:29Z

+// the first matching filter provides each scalar option, while root options act as fallbacks.
+// Notes from every matching filter and every root directive remain in source order.
+
+//@ dont-require-annotations: NOTE


IMO we should have required note annotations in this file, in case we get new notes added, duplicate notes, etc, by accident

View changes since the review

I didn’t want to use it initially, but without it we’d have to annotate all notes unrelated to coalescing (as you can see in the stderr files), which I found to be noisy.

A better approach would be to model the test such that it won't require extra notes outside coalescing

It's important that the notes are annotated so that missing or extra notes actually fail the test. otherwise it is easy for people to just blindly bless the test and move on.

Bryntet · 2026-07-30T09:03:00Z

Also just to put context in here in the actual PR thread, this change is done to allow us to later migrate the whole #[rustc_on_unimplemented] to be #[diagnostic::on_unimplemented], correct?

Since we warn on malformed diagnostic attrs

mejrs · 2026-07-30T09:13:54Z

Also just to put context in here in the actual PR thread, this change is done to allow us to later migrate the whole #[rustc_on_unimplemented] to be #[diagnostic::on_unimplemented], correct?

Since we warn on malformed diagnostic attrs

not without modifications, but yes.

mejrs

Can you also add a test specifically combining a rustc_on_unimplemented with filters and a regular diagnostic attribute? That might up being (perhaps accidentally?) used in std and I want to be doubly sure it works.

like

#[rustc_on_unimplemented(
    on(
        ...,
        ....
    ),
    ....
)]
#[diagnostic::on_unimplemented( ...)
trait Trait { }

View changes since this review

mejrs · 2026-07-30T09:21:22Z

+        if let Some(parent_label) = parent_label {
+            if let Some(first_parent_label) = &first.parent_label {
+                cx.emit_lint(
+                    MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                    IgnoredDiagnosticOption {
+                        first_span: first_parent_label.span,
+                        later_span: parent_label.span,
+                        option_name: sym::parent_label,
+                    },
+                    parent_label.span,
+                );
+            } else {
+                first.parent_label = Some(parent_label);
+            }
        }


You can just call merge here instead? Like for message and label? Change parent_label to be of type Option<(Span, _)> instead if you need to.

Fair enough, I avoided touching the HIR code since the files involved were outside the scope of this refactor

mejrs · 2026-07-30T09:31:43Z

+        let Directive { is_rustc_attr, filters, message, label, notes, parent_label } = later.1;
+
+        // Evaluation visits every filter before the root directive. Appending filters and notes
+        // preserves their source order across separate attribute occurrences.


This comment seems maybe a little bit out of place. And it reads like it vaguely implies that the ordering of the operations below it matters which afaik doesn't matter.

If you want to document what merge_directives does that would be best as a doc comment on the function itself.

mejrs · 2026-07-30T09:56:46Z

 The `#[rustc_on_unimplemented]` attribute lets you specify a custom error
 message for when a particular trait isn't implemented on a type placed in a
 position that needs that trait. The attribute will let you filter on
 various types, with `on`:

-```compile_fail,E0232
+```ignore (error is no longer emitted)
 #![feature(rustc_attrs)]
 #![allow(internal_features)]


Can you edit this to say that now a lint is emitted instead?

Done!

@rustbot ready

Sorry, I mean like in the prose above the example. As it reads now it's just kinda weird and confusing (and it wasn't great). Feel free to remove all the prose and example and just replace it with smth like

this used to emitted on an invalid rustc_on_unimplemented filter, the `malformed_diagnostic_filters` lint is now emitted instead.

remember, good documentation > no documentation >>>> bad documentation.

Done. I was trying to mirror neighboring error code files.

@rustbot ready

rustbot · 2026-07-30T10:15:33Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

rustbot · 2026-07-30T11:23:07Z

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred to diagnostic attributes.

cc @mejrs

mejrs

thanks, r=me after error code edit

@rustbot author

View changes since this review

mejrs · 2026-07-30T22:56:30Z

 The `#[rustc_on_unimplemented]` attribute lets you specify a custom error
 message for when a particular trait isn't implemented on a type placed in a
 position that needs that trait. The attribute will let you filter on
 various types, with `on`:

-```compile_fail,E0232
+```ignore (error is no longer emitted)
 #![feature(rustc_attrs)]
 #![allow(internal_features)]


Sorry, I mean like in the prose above the example. As it reads now it's just kinda weird and confusing (and it wasn't great). Feel free to remove all the prose and example and just replace it with smth like

this used to emitted on an invalid rustc_on_unimplemented filter, the `malformed_diagnostic_filters` lint is now emitted instead.

remember, good documentation > no documentation >>>> bad documentation.

mejrs

Thanks!

@bors r+ rollup

View changes since this review

rust-bors · 2026-07-31T09:31:06Z

📌 Commit 0a4168a has been approved by mejrs

It is now in the queue for this repository.

@mejrs

…, r=mejrs Coalesce `rustc_on_unimplemented` attributes and lint malformed filters Discussion on [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/attribute.20parsing.20rework/near/613179950). Best reviewed in commit order. r? @mejrs

@mejrs

…, r=mejrs Coalesce `rustc_on_unimplemented` attributes and lint malformed filters Discussion on [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/attribute.20parsing.20rework/near/613179950). Best reviewed in commit order. r? @mejrs

@mejrs

…, r=mejrs Coalesce `rustc_on_unimplemented` attributes and lint malformed filters Discussion on [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/attribute.20parsing.20rework/near/613179950). Best reviewed in commit order. r? @mejrs

…uwer Rollup of 21 pull requests Successful merges: - #160100 (Add "system" option to `override-allocator` directive) - #159675 (rustc: Support `--jobs` options for limiting parallelism in various parts of the compiler) - #159999 (Fix invalidation of stdlib in bootstrap when using non-LLVM codegen backends) - #160233 (Bubble bad path error while parsing field to avoid unecessary second error) - #160272 (rustc_metadata: Move native library search code to `rustc_codegen_ssa`) - #154202 (rustfmt: Format `cfg_select!`) - #158835 (rustc_passes: lint unused `#[path]` attributes on inline modules) - #159520 (Suggest `Vec<T>` instead of `[T]`) - #160034 (Move "macro only" check for `#[allow_internal_unsafe/unstable]` to attribute parser) - #160066 (rustc_middle: lint attribute cleanups) - #160085 (Remove various superfluous lint attributes) - #160113 (Coalesce `rustc_on_unimplemented` attributes and lint malformed filters) - #160119 (fix query cycle in `coroutine_hidden_types` for the next solver) - #160147 (tests: Remove `-Zthreads` options from tests in `ui/parallel-rustc`) - #160157 (Remove outdated comments from `va_list.rs`) - #160159 (More accurately check for interior mutability in `invalid_reference_casting` lint) - #160208 (rustdoc: Fix crash when trying to list attributes on an opaque type) - #160244 (Rename splat to rustc_splat in error messages) - #160246 (dont fire `unused_mut` on `&pin mut self`) - #160247 (Configure backport nominations for rustfmt) - #160274 (renovate: update lock files weekly)

…uwer Rollup of 22 pull requests Successful merges: - #160100 (Add "system" option to `override-allocator` directive) - #160220 (Refactor: shrink region ext traits) - #159675 (rustc: Support `--jobs` options for limiting parallelism in various parts of the compiler) - #159999 (Fix invalidation of stdlib in bootstrap when using non-LLVM codegen backends) - #160233 (Bubble bad path error while parsing field to avoid unecessary second error) - #160272 (rustc_metadata: Move native library search code to `rustc_codegen_ssa`) - #154202 (rustfmt: Format `cfg_select!`) - #159520 (Suggest `Vec<T>` instead of `[T]`) - #159710 (Add rustdoc/cargo PGO profiles to reproducible artifacts) - #160034 (Move "macro only" check for `#[allow_internal_unsafe/unstable]` to attribute parser) - #160066 (rustc_middle: lint attribute cleanups) - #160085 (Remove various superfluous lint attributes) - #160113 (Coalesce `rustc_on_unimplemented` attributes and lint malformed filters) - #160119 (fix query cycle in `coroutine_hidden_types` for the next solver) - #160147 (tests: Remove `-Zthreads` options from tests in `ui/parallel-rustc`) - #160157 (Remove outdated comments from `va_list.rs`) - #160159 (More accurately check for interior mutability in `invalid_reference_casting` lint) - #160208 (rustdoc: Fix crash when trying to list attributes on an opaque type) - #160244 (Rename splat to rustc_splat in error messages) - #160246 (dont fire `unused_mut` on `&pin mut self`) - #160247 (Configure backport nominations for rustfmt) - #160274 (renovate: update lock files weekly)

@mejrs

Rollup merge of #160113 - qaijuang:coalesce_on_unimplemented, r=mejrs Coalesce `rustc_on_unimplemented` attributes and lint malformed filters Discussion on [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/attribute.20parsing.20rework/near/613179950). Best reviewed in commit order. r? @mejrs

rustbot assigned mejrs Jul 29, 2026

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 29, 2026

qaijuang force-pushed the coalesce_on_unimplemented branch from d048c27 to 0e36f43 Compare July 29, 2026 01:56

qaijuang marked this pull request as ready for review July 29, 2026 09:24

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 29, 2026

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 29, 2026

Bryntet reviewed Jul 29, 2026

View reviewed changes

qaijuang added 3 commits July 30, 2026 02:23

test rejection of repeated rustc_on_unimplemented attributes

7f4e706

coalesce repeated on_unimplemented attributes

f21b64f

lint malformed rustc_on_unimplemented filters

9628fa5

qaijuang force-pushed the coalesce_on_unimplemented branch from 0e36f43 to 9628fa5 Compare July 30, 2026 06:45

mejrs requested changes Jul 30, 2026

View reviewed changes

rustbot added S-waiting-on-author 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 Jul 30, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2026

mejrs reviewed Jul 30, 2026

View reviewed changes

rustbot added S-waiting-on-author 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 Jul 30, 2026

address follow up suggestions

0a4168a

qaijuang force-pushed the coalesce_on_unimplemented branch from e194aa6 to 0a4168a Compare July 31, 2026 00:31

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 31, 2026

mejrs approved these changes Jul 31, 2026

View reviewed changes

rust-bors Bot 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 Jul 31, 2026

JonathanBrouwer mentioned this pull request Jul 31, 2026

Rollup of 11 pull requests #160273

Closed

JonathanBrouwer mentioned this pull request Jul 31, 2026

Rollup of 17 pull requests #160278

Closed

JonathanBrouwer mentioned this pull request Jul 31, 2026

Rollup of 21 pull requests #160283

Closed

JonathanBrouwer mentioned this pull request Jul 31, 2026

Rollup of 22 pull requests #160284

Merged

rust-bors Bot merged commit 212af25 into rust-lang:main Jul 31, 2026
13 checks passed

rustbot added this to the 1.99.0 milestone Jul 31, 2026

qaijuang deleted the coalesce_on_unimplemented branch July 31, 2026 22:41

		//~^ ERROR invalid flag in `on`-clause
		//~^ WARN invalid flag in `on`-clause

Uh oh!

Uh oh!

Conversation

qaijuang commented Jul 29, 2026

Uh oh!

rustbot commented Jul 29, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

Bryntet Jul 29, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bryntet commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mejrs commented Jul 30, 2026

Uh oh!

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qaijuang Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jul 30, 2026

Uh oh!

rustbot commented Jul 30, 2026

Uh oh!

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Jul 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Bryntet Jul 29, 2026 •

edited by rustbot

Loading

mejrs Jul 30, 2026 •

edited

Loading

Bryntet commented Jul 30, 2026 •

edited

Loading

mejrs left a comment •

edited by rustbot

Loading

qaijuang Jul 30, 2026 •

edited

Loading

mejrs left a comment •

edited by rustbot

Loading

mejrs left a comment •

edited by rustbot

Loading