Implement `#[diagnostic::on_unknown]` for modules. by mejrs · Pull Request #157926 · rust-lang/rust

mejrs · 2026-06-15T17:50:16Z

This lets you customize the error message if an item can't be found in a module. Primarily my motivation for this is that I was trying to dogfood diagnostic::on_unknown in my own crates and realized I was writing a lot of duplicate attributes and it'd be simpler if it just applied to things at a module level.

For example:

#[diagnostic::on_unknown(message = "oh no, `{Unresolved}` is not in module `{This}`")]
pub mod x {
    pub struct Foo;
}

pub use x::Bar;

error[E0432]: oh no, `Bar` is not in module `x`
  --> $DIR/on_module.rs:9:9
   |
LL | pub use x::Bar;
   |         ^^^---
   |            |
   |            no `Bar` in `x`
   |
   = note: unresolved import `x::Bar`

Also, as shown, this implements {Unresolved} to reference whatever couldn't be found.

Inner attribute syntax (#![diagnostic::on_unknown]) is supported, however it (still) is an error to use inner tool attributes without #![feature(custom_inner_attributes)].

cc @estebank @weiznich

rustbot · 2026-06-15T17:50:19Z

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-06-15T17:50:21Z

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 19 candidates

petrochenkov · 2026-06-16T11:29:28Z

The implementation looks ok, but I don't like the general direction in which it goes.
Yeah, in principle we can allow users to customize all diagnostic messages, and drown the compiler in technical debt in process.
Would it be a total win? I don't think so. Does this particular customization (including #152901) pull its weight? I also don't think so.
At least it's unstable now and can be removed in the future.

Could you move the throw_unresolved_import_error stuff into diagnostics.rs at least?
r=me after that
@rustbot author

rustbot · 2026-06-16T11:29:32Z

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

petrochenkov · 2026-06-16T11:33:31Z

Let's benchmark this as well.
Similarly to #152901 here we are doing more work on a good path even when we don't report any diagnostics.
@bors try @rust-timer queue

Implement `#[diagnostic::on_unknown]` for modules.

petrochenkov · 2026-06-16T11:38:26Z

Similarly to #152901 here we are doing more work on a good path even when we don't report any diagnostics.

At least in cross-crate cases the OnUnknownData can be loaded on demand only if errors actually occur.
Ideally, I'd like to get the on_unknown_attr fields removed from both import and module data, and always load them on demand, but for the local case it requires building a bit of new infra (which may happen naturally though as incremental name resolution progresses).

weiznich · 2026-06-16T12:34:55Z

Would it be a total win? I don't think so. Does this particular customization (including #152901) pull its weight? I also don't think so.

I would just like to raise the concern that this is a rather general comment without any in depth explanation. That makes it hard for anyone to address your concerns there. Given that I would like to ask you to add at least a little explanation why you believe my presented use-case is not valid. As currently written it reads like the improvements and therefore the reduction in support work done the error messages to crates maintained by me are not worth the effort you/the compiler team need to have. I would like to polity disagree there as past improvements did significantly reduce the support load on my end and I don't believe passing a few more information trough the relevant code up to the error message construction is really a lot more work in the long run.

At least it's unstable now and can be removed in the future.

Anything in the #[diagnostic] namespace can be removed at a later point, if it turns out to be not sustainable or a problem in any other way. So I really don't see the argument you are trying to make here. This is therefore also not a real blocker for "stabilizing" the #[diagnostic::on_unknown] attribute.

mejrs · 2026-06-16T13:39:15Z

Yeah, in principle we can allow users to customize all diagnostic messages, and drown the compiler in technical debt in process.

It goes both ways, diagnostic attributes also let us remove error reporting code in the compiler. No need to have specialized code error reporting code just to give a custom error message if we can just use an attribute instead. They can remove a lot of special casing in general.

Does this particular customization (including #152901) pull its weight? I also don't think so.

An example of something that I want to do is to put it on rustc_span's sym module, so that if you use sym::not_yet_predefined, it gives you a helpful error. This is something that new clippy contributors run into fairly often and it'd be nice to take away some friction there.

You can also imagine this being used as a tombstone mechanism:

#[diagnostic::on_unknown(note = "`Foo` was renamed to `Bar` in the great bikeshed war")]
mod stuff { ... }

// elsewhere
use stuff::Foo;

Similarly to #152901 here we are doing more work on a good path even when we don't report any diagnostics.

At least in cross-crate cases the OnUnknownData can be loaded on demand only if errors actually occur. Ideally, I'd like to get the on_unknown_attr fields removed from both import and module data, and always load them on demand, but for the local case it requires building a bit of new infra (which may happen naturally though as incremental name resolution progresses).

+1. I looked into lazily loading them at first but I couldn't figure out how to get from a NodeId an ast::Attribute (or ast::Item). Threading a &'a [Ast::Attribute] down the call stack seemed messy (and probably impossible) as well. Maybe that's the best approach once we can put more ast structs on arena.

I would just like to raise the concern that this is a rather general comment without any in depth explanation.

@weiznich note that the comment was towards me and I'm aware-ish of the context, so it does not need in depth explanation.

But the tl;dr is that name resolution and macro expansion, moreso than other things in the compiler, is rather "hacky":

there are quite a few places where things are/were allowed without thinking of how they fit in the big picture
bugs or underspecced behavior must be preserved to not break everyone
the process of doing refactoring/fcwing/etc to wean the ecosystem off these things is long and painful

As a result rustc_resolve is quite sensitive to adding complexity, especially if it stands in the way of doing refactoring later down the line. So the concern is not ungrounded. I don't think it applies here though.

weiznich · 2026-06-16T13:47:29Z

@mejrs Thanks for providing the context. That's helpful to know ❤️

(And yes I can understand what you mean by the code being hacky, it was rather hard to get the initial implementation in there)

rust-bors · 2026-06-16T17:34:22Z

💥 Test timed out after 21600s

mejrs · 2026-06-16T18:41:38Z

@bors try @rust-timer queue

Implement `#[diagnostic::on_unknown]` for modules.

rust-bors · 2026-06-16T20:56:48Z

☀️ Try build successful (CI)
Build commit: 45c9d49 (45c9d49ab4c812f251da4c0f347a9b1e5e9a547e, parent: 89a99936d9e76a50e8df622e7242190841fd871b)

mejrs · 2026-06-16T21:56:26Z

Could you move the throw_unresolved_import_error stuff into diagnostics.rs at least?

Done. Fwiw, lint_reexports and report_cannot_reexport also feel like they belong in /diagnostics. Let me know if you'd appreciate a standalone PR moving those.

r=me after that

I'll wait for the perf run to finish (if it finishes).

rust-timer · 2026-06-17T12:52:53Z

Finished benchmarking commit (d954550): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 2.4%, secondary 3.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.4%	[2.4%, 2.4%]	1
Regressions ❌ (secondary)	3.7%	[2.8%, 4.3%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.4%	[2.4%, 2.4%]	1

Cycles

Results (secondary 2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 523.739s -> 521.175s (-0.49%)
Artifact size: 401.37 MiB -> 401.87 MiB (0.12%)

mejrs · 2026-06-18T14:39:47Z

@bors try @rust-timer queue

Implement `#[diagnostic::on_unknown]` for modules.

rust-bors · 2026-06-18T16:50:56Z

☀️ Try build successful (CI)
Build commit: 99947af (99947af72ff5d091036432e9b0abdf4cbde83461)
Base parent: c55fad5 (c55fad5a9048ad0f7cff2a4867a297647af9392b)

rust-timer · 2026-06-18T17:31:07Z

Finished benchmarking commit (99947af): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary -3.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.4%	[-3.4%, -3.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.4%	[-3.4%, -3.4%]	1

Cycles

Results (secondary -2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	[-2.8%, -2.8%]	1
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.0%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	15
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	15

Bootstrap: 481.592s -> 480.903s (-0.14%)
Artifact size: 391.21 MiB -> 390.75 MiB (-0.12%)

mejrs · 2026-06-18T17:35:40Z

@rustbot ready

petrochenkov · 2026-06-19T10:28:34Z

@bors r+

rust-bors · 2026-06-19T10:28:37Z

📌 Commit 443c0ba has been approved by petrochenkov

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

JonathanBrouwer · 2026-06-19T10:46:11Z

@bors rollup

@estebank

…enkov Implement `#[diagnostic::on_unknown]` for modules. This lets you customize the error message if an item can't be found in a module. Primarily my motivation for this is that I was trying to dogfood `diagnostic::on_unknown` in my own crates and realized I was writing a lot of duplicate attributes and it'd be simpler if it just applied to things at a module level. For example: ```rust #[diagnostic::on_unknown(message = "oh no, `{Unresolved}` is not in module `{This}`")] pub mod x { pub struct Foo; } pub use x::Bar; ``` ``` error[E0432]: oh no, `Bar` is not in module `x` --> $DIR/on_module.rs:9:9 | LL | pub use x::Bar; | ^^^--- | | | no `Bar` in `x` | = note: unresolved import `x::Bar` ``` Also, as shown, this implements `{Unresolved}` to reference whatever couldn't be found. Inner attribute syntax (`#![diagnostic::on_unknown]`) is supported, however it (still) is an error to use inner tool attributes without `#![feature(custom_inner_attributes)]`. cc @estebank @weiznich

…uwer Rollup of 4 pull requests Successful merges: - #157926 (Implement `#[diagnostic::on_unknown]` for modules.) - #158084 (`-Znext-solver` Emit error instead of ICE when combining {int, float} var with alias) - #158128 (std: use correct low surrogate range in Windows standard I/O code) - #158132 (std: correctly report file size on UWP)

@estebank

…enkov Implement `#[diagnostic::on_unknown]` for modules. This lets you customize the error message if an item can't be found in a module. Primarily my motivation for this is that I was trying to dogfood `diagnostic::on_unknown` in my own crates and realized I was writing a lot of duplicate attributes and it'd be simpler if it just applied to things at a module level. For example: ```rust #[diagnostic::on_unknown(message = "oh no, `{Unresolved}` is not in module `{This}`")] pub mod x { pub struct Foo; } pub use x::Bar; ``` ``` error[E0432]: oh no, `Bar` is not in module `x` --> $DIR/on_module.rs:9:9 | LL | pub use x::Bar; | ^^^--- | | | no `Bar` in `x` | = note: unresolved import `x::Bar` ``` Also, as shown, this implements `{Unresolved}` to reference whatever couldn't be found. Inner attribute syntax (`#![diagnostic::on_unknown]`) is supported, however it (still) is an error to use inner tool attributes without `#![feature(custom_inner_attributes)]`. cc @estebank @weiznich

…uwer Rollup of 8 pull requests Successful merges: - #158129 (ensure the new solver bootstraps on CI) - #158134 (Rename `lint-rust-version` to `hint-msrv`) - #157926 (Implement `#[diagnostic::on_unknown]` for modules.) - #158075 (Point to the unstable segment of an import path instead of to the whole path) - #158084 (`-Znext-solver` Emit error instead of ICE when combining {int, float} var with alias) - #158128 (std: use correct low surrogate range in Windows standard I/O code) - #158132 (std: correctly report file size on UWP) - #158138 (Remove redundant check for `#[loop_match]` and `#[const_continue]`)

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 15, 2026

rustbot assigned petrochenkov Jun 15, 2026

mejrs changed the title ~~Implement #[diagnostic_on_unknown] for modules.~~ Implement #[diagnostic::on_unknown] for modules. Jun 15, 2026

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 Jun 16, 2026