resolve paths in rustc_on_unimplemented by mejrs · Pull Request #156505 · rust-lang/rust

mejrs · 2026-05-12T15:35:58Z

This creates a new (unstable) #[diagnostic::rustc_on_unimplemented] attribute that compares types by defid, rather than stringly at error reporting time. This is necessary to finally fix issues like #112923. This is the first step in our (@estebank and mine) plans to eventually stabilize a subset of rustc_on_unimplemented's filtering api.

It does this by acting as an attribute macro which passes the attribute through ast and resolving and then emits it as an attribute during ast lowering.

For simplicity the functionality is rather limited at the time, that's to be expanded in follow up prs:

can only resolve identifiers; allowing multi-segment paths and args will require pulling in/writing a parser
can only resolve adts (not primitives, tuples, slices, pointers etc) atm
this emits errors, these will have to be lints eventually
the attribute is lost during token roundstrips, we'll need something like Fix ICE when combining #[eii] with #[core::contracts::ensures] #153796
PrintAttribute and ast_pretty impls need improvement

r? @petrochenkov
@JonathanBrouwer you might be interested in reviewing as well
cc @estebank @weiznich
@BarronKane you might be interested in looking at the implementation

rustbot · 2026-05-12T15:36:02Z

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

mejrs · 2026-05-12T15:41:38Z

+    Path {
+        #[visitable(ignore)]
+        name: Name,
+        id: NodeId,
+        value: Path,
+    },
+    DefId {
+        #[visitable(ignore)]
+        name: Name,
+        #[visitable(ignore)]
+        def_id: Option<DefId>,
+    },


These are switched during ast lowering, so we never have a Path variant inside a hir::Attribute. An alternative is to make NameValue generic over these two but that introduces a lot of generics everywhere. I tried that first and it was quite messy.

View changes since the review

mejrs · 2026-05-12T15:42:48Z

    /// This is never nested more than once, i.e. the directives in this
    /// thinvec have no filters of their own.
    pub filters: ThinVec<(Filter, Directive)>,
+    #[visitable(ignore)]


I'm not sure about #[visitable(ignore)] here and elsewhere. Can I just put these on everything that doesn't have something "ast-y" like NodeId in it?

View changes since the review

mejrs · 2026-05-12T15:48:35Z

+    /// An AST-based "inert" attribute.
+    ///
+    /// These represent otherwise inert attributes. Instead of creating or modifying items like
+    /// a macro would, they instead attach state to these items which is carried through
+    /// ast/hir lowering and then emitted like an actual inert attribute.
+    InertAttr(
+        /// An expander with signature (&mut AST).
+        Arc<dyn AstAnnotate + sync::DynSync + sync::DynSend>,
+    ),


I decided on a new variant for several reasons:

I wanted to use ArgParser so I can reuse the implementation in rustc_attr_parsing, keeping that and a MetaItem impl at the same time is too error prone.

I would like to experiment with making this masquerade like an actual inert attribute so this can be stabilized without it being observable by other macros.

View changes since the review

rustbot · 2026-05-12T16:21:39Z

The Clippy subtree was changed

cc @rust-lang/clippy

rustbot · 2026-05-12T17:45:54Z

The Rustfmt subtree was changed

cc @rust-lang/rustfmt

rustbot · 2026-05-13T09:37:20Z

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.

…h ast lowering

rustbot assigned petrochenkov May 12, 2026