Stabilize #[diagnostic::on_unknown]

[weiznich]

Stabilization report

Summary

Remind us what this feature is and what value it provides. Tell the story of what led up to this stabilization

This PR seeks to stabilize the #[diagnostic::on_unknown] attribute in the #[diagnostic] namespace. This attribute let users change the error message emitted by rustc for the case that an referenced item is not found. This allows crate authors to provide custom error messages for specific use-cases to help their users to more easily understand how a certain feature works and why a specific code example doesn't compile.

Tracking:

• Tracking Issue for #[diagnostic::on_unknown] #152900

Reference PRs:

• Add some documentation around the #[diagnostic::on_unknown] attribute reference#2379

Not sure who to really ping on stabilization. The #[diagnostic] namespace RFC states:

Adding a new attribute or option to the #[diagnostic] namespace is for now a decision of the language team. The language team can delegate these decisions partially or completely to a different team without requiring a new RFC.

I'm not sure what's the current state of that delegation, I remember that it was planned to delegate it to the compiler team at some point.

What is stabilized

Describe each behavior being stabilized and give a short example of code that will now be accepted.

The #[diagnostic::on_unknown] attribute is an attribute in the #[diagnostic] tool namespace. It follows the general rules of that namespace, so it can only affect compiler diagnostics, not change actual behaviour. Any syntax error on any of the attributes parameters will be emitted as warning and otherwise ignored. There is no guarantee by the compiler that a certain output is emitted even with the attribute being accepted, so future changes (including ignoring it) to this attribute are possible.
The attribute should be
placed on use and module declarations as well as the crate root, though it is not an error to be located in other
positions.

Format parameters with the given named parameter will be replaced with the following text:

• {Unresolved} — The SimplePathSegment of the import path that could not be resolved.
• {This} — The name of the annotated item. On use statements this is identical to {Unresolved}.

The original error message will not be suppressed but is emitted as a note instead.

On use declarations

#[diagnostic::on_unknown(
    message = "`{Unresolved}` doesn't exist",
    label = "you did something silly here"
)]
use doesnt_exist;

This will result in the following error:

error[E0432]: `doesnt_exist` doesn't exist
 --> src/lib.rs:7:5
  |
7 | use doesnt_exist;
  |     ^^^^^^^^^^^^ you did something silly here
  |
  = note: unresolved import `doesnt_exist`

For more information about this error, try `rustc --explain E0432`.

On module declarations

#[diagnostic::on_unknown(
    message = "module `{This}` is empty, there is no `{Unresolved}` here",
    label = "can't import something from an empty module"
)]
mod empty {}

use empty::what;

This will result in the following error:

error[E0432]: module `empty` is empty, there is no `what` here
 --> src/lib.rs:9:5
  |
9 | use empty::what;
  |     ^^^^^^^----
  |            |
  |            can't import something from an empty module
  |
  = note: unresolved import `empty::what`

Example

Consider the following pair of macros, one which creates a hidden module with a constant and another that reads it.

#![feature(macro_metavar_expr, macro_attr)]

macro_rules! instrument {
    attr() { $vis:vis fn $fn_name:ident ($($arg_name:ident : $arg_ty:ty),*) $(-> $ret:ty)? $body:block } => {
        $vis fn $fn_name ($($arg_name:$arg_ty),*) $(-> $ret)? $body
        #[doc(hidden)]
        $vis mod $fn_name {
            pub const LEN: usize = ${ count($arg_name) };
        }
    }
}

macro_rules! count_args {
    ($name:ident) => {{
        $name::LEN
    }}
}

#[instrument]
fn add(a: u8, b: u8) -> u8 {
    a + b
}

fn main() {
    let n = count_args!(add);
    println!("`add` has {n} arguments");
}

If the #[instrument] macro is omitted it will emit this confusing error:

error[E0433]: cannot find module or crate `add` in this scope
  --> src/main.rs:25:25
   |
25 |     let n = count_args!(add);
   |                         ^^^ function `add` is not a crate or module

#[diagnostic::on_unknown] can be used to customize this error message:

macro_rules! count_args {
    ($name:ident) => {{
        #[diagnostic::on_unknown(
            message = "cannot count arguments of `{Unresolved}`",
            label = "`{Unresolved}` is not a function decorated \
                    with the `#[instrument]` macro"
        )]
        use $name::LEN as length;
        length
    }}
}

// #[instrument]
fn add(a: u8, b: u8) -> u8 {
  a + b
}

fn main() {
    let n = count_args!(add);
    println!("`add` has {n} arguments");
}

This produces:

error[E0432]: cannot count arguments of `add`
  --> src/main.rs:33:25
   |
33 |     let n = count_args!(add);
   |                         ^^^ `add` is not a function decorated with
   |                              the `#[instrument]` macro
   |
   = note: unresolved import `add`

Edition differences

In the 2015 edition, use paths are relative to the crate root. For example, use empty will be resolved relative to the crate root and resolution will not encounter the mod empty {} declaration.

#![feature(diagnostic_on_unknown)]

mod foo {
   #[diagnostic::on_unknown(message = "oh oh")]
   mod empty {}

   use empty::what;
}

error[E0432]: unresolved import `empty`
 --> src/main.rs:9:8
  |
9 |    use empty::what;
  |        ^^^^^
  |

What isn't stabilized

Describe any parts of the feature not being stabilized. Talk about what we might want to do later and what doors are being left open for that. If what we're not stabilizing might lead to surprises for users, talk about that in particular.

Any usage that also requires another unstable feature is not stabilized in so far that the other feature is required to write the code using the attribute in that location. This mostly concerns the usage of #[diagnostic::on_unknown] at the crate root as inner attribute, which requires the custom_inner_attributes to define the inner attribute.

Design

Reference

What updates are needed to the Reference? Link to each PR. If the Reference is missing content needed for describing this feature, discuss that.

The reference needs a description of the attribute, basically what's removed from the unstable book and in the stabilization report above.

RFC history

What RFCs have been accepted for this feature?

This feature is part of the #[diagnostic] tool attribute namespace, which was defined in https://rust-lang.github.io/rfcs/3368-diagnostic-attribute-namespace.html. The attribute itself was not specified by an RFC as the RFC explicitly allows adding new attributes to that namespace without RFC.

Answers to unresolved questions

There are no unresolved questions I'm aware of

Post-RFC changes

What other user-visible changes have occurred since the RFC was accepted? Describe both changes that the lang team accepted (and link to those decisions) as well as changes that are being presented to the team for the first time in this stabilization report.

Given that there was no RFC for this particular attribute, there are no changes here to discuss

Key points

What decisions have been most difficult and what behaviors to be stabilized have proved most contentious? Summarize the major arguments on all sides and link to earlier documents and discussions.

The name of the attribute got some discussion. It started as #[diagnostic::on_unknown_item] and was then renamed to the current name. Otherwise I think @petrochenkov raised the question if we really want to allow customizing all the compiler error messages like that. Other than that there were no major arguments that I'm aware of.

Nightly extensions

Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those?

There are none

Doors closed

What doors does this stabilization close for later changes to the language? E.g., does this stabilization make any other RFCs, lang experiments, or known in-flight proposals more difficult or impossible to do later?

Feedback

Call for testing

Has a "call for testing" been done? If so, what feedback was received?

There was no call for testing as this is a rather specialized and somewhat simple feature

Nightly use

Do any known nightly users use this feature? Counting instances of #![feature(FEATURE_NAME)] on GitHub with grep might be informative.

The compiler itself uses this feature in rustc_span:

• rust/compiler/rustc_span/src/symbol.rs

  Lines 2986 to 2989 in a5c10c3

  	#[diagnostic::on_unknown(
  	label = "`{Unresolved}` is not a pre-interned symbol",
  	note = "consider adding `{Unresolved}` to the `symbols!` invocation in compiler/rustc_span/src/symbol.rs"
  	)]

I also did a small test implementation in Diesel, to demonstrate how this can be used to significantly improve error messages emitted by one of the derives diesel provides:

For the given code:

#[derive(AsChangeset)]
struct User {
    id: i32,
    name: String,
}

the following error message is emitted

error[E0432]: invalid table name `users` inferred
 --> tests/fail/derive/no_table.rs:9:8
  |
LL | struct User {
  |        ^^^^ cannot find the default table name `users` in scope
  |
  = note: unresolved import `users`
  = note: without a `#[diesel(table_name = _)]` attribute looks for a `users` item in scope. Add a import to your schema module like `use crate::schema::users; or use the `#[diesel(table_name = _)]` to explicitly specify a different table name
  = help: you might be missing a crate named `users`

the underlying problem here is that the proc-macro derive needs access to another user defined module. By default it assumes a name based on the struct name, users can customize that name via an attribute. In the past users often struggled with discovering that this attribute existed, so it is really helpful to be able to point to the attribute as part of the error message.

See the full change here:

weiznich/diesel@15d6442

Implementation

Major parts

Summarize the major parts of the implementation and provide links into the code and to relevant PRs.

See, e.g., this breakdown of the major parts of async closures:

• https://rustc-dev-guide.rust-lang.org/coroutine-closures.html

This feature was implemented in the following PR's:

• Introduce a #[diagnostic::on_unknown] attribute #152901
• Implement #[diagnostic::on_unknown] for modules. #157926

Coverage

Summarize the test coverage of this feature.

Consider what the "edges" of this feature are. We're particularly interested in seeing tests that assure us about exactly what nearby things we're not stabilizing. Tests should of course comprehensively demonstrate that the feature works. Think too about demonstrating the diagnostics seen when common mistakes are made and the feature is used incorrectly.

Within each test, include a comment at the top describing the purpose of the test and what set of invariants it intends to demonstrate. This is a great help to our review.

Describe any known or intentional gaps in test coverage.

Contextualize and link to test folders and individual tests.

There is a good test coverage in the UI test suite:

https://github.com/rust-lang/rust/tree/a5c10c39125788787370a1ab11c954b9dff37859/tests/ui/diagnostic_namespace/on_unknown

Outstanding bugs

None that I'm aware of

Outstanding FIXMEs

None that I'm aware of

Tool changes

What changes must be made to our other tools to support this feature. Has this work been done? Link to any relevant PRs and issues.

• rustfmt
  • No changes required as far as I'm aware
• rust-analyzer
  • Might need to add the new attribute to their built-in completion list
• rustdoc (both JSON and HTML)
  • No changes required as far as I'm aware
• cargo
  • No changes required as far as I'm aware
• clippy
  • No changes required as far as I'm aware
• rustup
  • No changes required as far as I'm aware
• docs.rs
  • No changes required as far as I'm aware

Breaking changes

If this stabilization represents a known breaking change, link to the crater report, the analysis of the crater report, and to all PRs we've made to ecosystem projects affected by this breakage. Discuss any limitations of what we're able to know about or to fix.

No breaking change required

Type system, opsem

Compile-time checks

What compilation-time checks are done that are needed to prevent undefined behavior?

Link to tests demonstrating that these checks are being done.

It's not possible to use this to trigger undefined behaviour, so no compile time checks preventing that exit

Type system rules

What type system rules are enforced for this feature and what is the purpose of each?

None, as this feature doesn't affect the type system

Sound by default?

Does the feature's implementation need specific checks to prevent UB, or is it sound by default and need specific opt-in to perform the dangerous/unsafe operations? If it is not sound by default, what is the rationale?

No, it cannot be used to introduce undefined behaviour, it only changes compiler diagnostics

Breaks the AM?

Can users use this feature to introduce undefined behavior, or use this feature to break the abstraction of Rust and expose the underlying assembly-level implementation? Describe this if so.

No, it cannot be used to introduce undefined behaviour, it only changes compiler diagnostics

Common interactions

Temporaries

Does this feature introduce new expressions that can produce temporaries? What are the scopes of those temporaries?

This feature doesn't introduce new expressions

Drop order

Does this feature raise questions about the order in which we should drop values? Talk about the decisions made here and how they're consistent with our earlier decisions.

This feature doesn't change the generated code, so it cannot change the drop order

Pre-expansion / post-expansion

Does this feature raise questions about what should be accepted pre-expansion (e.g. in code covered by #[cfg(false)]) versus what should be accepted post-expansion? What decisions were made about this?

This feature doesn't change any expansion results, it just changes diagnostics.

Edition hygiene

If this feature is gated on an edition, how do we decide, in the context of the edition hygiene of tokens, whether to accept or reject code. E.g., what token do we use to decide?

It's not gated on an edition

SemVer implications

Does this feature create any new ways in which library authors must take care to prevent breaking downstreams when making minor-version releases? Describe these. Are these new hazards "major" or "minor" according to RFC 1105?

No, the #[diagnostic] namespace even allows to use that feature without bumping the minimal supported rust version as it ignores (with a warning) unknown diagnostic attributes by default

Exposing other features

Are there any other unstable features whose behavior may be exposed by this feature in any way? What features present the highest risk of that?

None that I'm aware of

History

List issues and PRs that are important for understanding how we got here.

• Diagnostic namespace #111780
• Stabilize the #[diagnostic] namespace and #[diagnostic::on_unimplemented] attribute #119888
• Introduce a #[diagnostic::on_unknown] attribute #152901
• Implement #[diagnostic::on_unknown] for modules. #157926

Acknowledgments

Summarize contributors to the feature by name for recognition and so that those people are notified about the stabilization. Does anyone who worked on this not think it should be stabilized right now? We'd like to hear about that if so.

@mejrs helped a lot to implement various features here and to guide the initial implementation. @estebank motivated the original work

Open items

List any known items that have not yet been completed and that should be before this is stabilized.

None that I'm aware of

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred to diagnostic attributes.

cc @mejrs

[rustbot]

r? @chenyukang

rustbot has assigned @chenyukang.
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 77 candidates
• Random selection from 20 candidates

[mejrs]

r? me

There are some outdated comments wrt the feature gating of this attribute, which need deleting

rust/compiler/rustc_resolve/src/imports.rs

Line 209 in 4ddbc06

/// This is `None` if the feature flag for `diagnostic::on_unknown` is disabled.

rust/compiler/rustc_resolve/src/diagnostics/impls.rs

Lines 180 to 181 in 4ddbc06

	// We don't need to check feature gates here; that happens on initialization of the
	// `on_unknown_attr` fields.

What isn't stabilized

Describe any parts of the feature not being stabilized. Talk about what we might want to do later and what doors are being left open for that. If what we're not stabilizing might lead to surprises for users, talk about that in particular.

Any usage that also requires another unstable feature is not stabilized in so far that the other feature is required to write the code using the attribute in that location

Can you clarify that this applies to the crate root, where the attribute has meaning but it remains gated by custom_inner_attributes?

View changes since this review

[weiznich]

@rustbot ready

[traviscross]

Overall, this sounds great to me. Thanks to @weiznich and @mejrs here. One thing:

Format parameters with the given named parameter will be replaced with the following text:

• {Unresolved} — The SimplePathSegment of the import path that could not be resolved.
• {This} — The name of the annotated item. On use statements this is identical to {Unresolved}.

The names and capitalization convention of these placeholders don't seem right to me.¹ As best I can tell, we haven't yet stabilized any similar placeholders², so we'll be setting precedent here.³

As background, some of the diagnostic attributes, including the stable diagnostic::on_unimplemented, support interpolating the argument types to type parameters. E.g.:

#[diagnostic::on_unimplemented(
    message = "my message that `Tr<{T}>` is not implemented for `{Self}`",
)]
trait Tr<T> {}

fn f(_: impl Tr<u8>) {}

fn main() {
    f(0u16);
}

Prints:

error[E0277]: my message that `Tr<u8>` is not implemented for `u16`
  --> src/main.rs
   |
   |     f(0u16);
   |     - ^^^^ the trait `Tr<u8>` is not implemented for `u16`
   |     |
   |     required by a bound introduced by this call
   |

Others, such as this one (on_unknown), want to interpolate things that no identifier refers to in the source code, such as the name of the module on which the attribute is placed or the name of the item that could not be resolved.

And others, such as diagnostic::on_type_error, want to interpolate both arguments to type parameters named in the source code and things where the compiler needs to provide the placeholder name (such as for the expected type).

Due to this last category, I don't think these names should be in CamelCase. The compiler-provided names shadow names present in the source code. So if we introduce a placeholder named This, then there'd be no way to refer to the generic parameter when applying the attribute to struct S<This>(…), and people do write such things.

Further, giving these CamelCase names just looks wrong to me.⁴ They suggest, to me, that I should be looking for matching type or type parameter names in the source code. And they feel particularly wrong here, as the placeholders refer to item names rather than to types.

I'd suggest that we name these placeholders in snake_case. Due to the convention for naming types and type parameters, this will avoid shadowing those in practice.

Regarding the names of the placeholders themselves, I'm a bit skeptical of the name this. I'm not sure how we'd use that name consistently across different diagnostic attributes. Even within the current implementation of on_unknown, it varies: on use it's the failing segment; on a module it's the module's name for imports, but for paths in expressions and type paths it's the prefix as written. Could we instead use {module} to refer to the name of the module? (Otherwise, maybe {item}?) And could we only support {module} when applied to modules (and not to use declarations)⁵ and have it always evaluate to just the module name?⁶ Would that make sense?

Regarding the placeholder to use for the SimplePathSegment of the path that couldn't be resolved, my read of the history is that on_unresolved and on_unresolved_import were considered for the name of the attribute, and on_unknown was chosen instead for various reasons. So it might seem odd, in that context, to then mix the two and use unresolved for the placeholder name with the on_unknown attribute. Would it make sense to simply use {name} for this, as in "can't find {name} in {module}"? (Otherwise, maybe {unknown}?)

With these changes, it'd look like this:

#[diagnostic::on_unknown(
    message = "module `{module}` is empty: no `{name}` found",
)]
mod empty {}

use empty::thing;

That looks better to me, and it would seem a better and more useful precedent going forward.

@mejrs, @weiznich: What do you think?

With this resolved, I'd be happy to propose FCP merge.

Footnotes

1. In the lang triage meeting where we discussed this PR briefly, @steffahn also mentioned these seemed a bit weird and asked whether there had been a discussion about choosing these names anywhere. I had a look, after the meeting, and I couldn't find one. ↩

2. That is, where we're picking a name rather than using a name available in the source, such as generic parameters on the item (including Self). ↩

3. To answer the question of "should we even bikeshed these?", I'd respond as follows. While the diagnostic namespace is designed such that we can drop support for an attribute later without breaking any code, people will still be relying on what we stabilize. These attributes, with the placeholders we chose, will appear in source code throughout the ecosystem. Crate authors will expect the things that they write to work. They'll expect that we won't remove and replace these capriciously later, causing churn. That's what I'd expect too, so it seems worth putting in some time and effort to make the best decisions we can upfront. This attribute isn't stable yet, so the cost of changing it is low, relative to that of changing it later. ↩

4. We want people to think of these like format strings, but rarely in format strings do we see this CamelCase convention. Most things in format strings are lowercase locals or screaming-case constants. Elsewhere in the attributes, such as for message, note, and label, we're using lowercase. And within rustc — which I cite just for what seemed idiomatic to one group of Rust programmers — the diagnostic templates use lowercase, as in {$found} and {$expected}. ↩

5. H/t to @steffahn, who pointed out in the lang triage meeting that both placeholders mean the same thing in one case, and that this seemed weird. ↩

6. When I think about how people are likely to use this, with messages such as "table {module} has no column {name}", it doesn't seem as though printing the leading segments of the path will make sense. ↩

[mejrs]

Thanks for that writeup. I find snake_case most compelling.

As best I can tell, we haven't yet stabilized any similar placeholders, so we'll be setting precedent here.

That's correct, and this discussion hasn't happened before either. I first found the CamelCase precedent in #62570 (comment) (when rustc_on_unimplemented's ItemContext was suggested), I cannot find any discussion about the convention specifically.

Regarding the names of the placeholders themselves, I'm a bit skeptical of the name this. I'm not sure how we'd use that name consistently across different diagnostic attributes. Even within the current implementation of on_unknown, it varies: on use it's the failing segment; on a module it's the module's name for imports, but for paths in expressions and type paths it's the prefix as written. Could we instead use {module} to refer to the name of the module? (Otherwise, maybe {item}?) And could we only support {module} when applied to modules (and not to use declarations) and have it always evaluate to just the module name? Would that make sense?

I think there's value in having one way, for all diagnostic attributes, to name the thing a diagnostic attribute applies to.

Some thoughts:

• item is inconsistent, because attributes can be applied on things that aren't items. No diagnostic attributes have meaning on any non-items today but they plausibly could in the future.
• module for diagnostic::on_unknown implies that other attributes use trait, struct, macro etc. Does that imply on_type_error (which goes on adts) must use either struct, enum or union depending on the item? Or would we use type to cover all of these? But then would we also use type for foreign types and type aliases? If we choose names for all these things, even if we document it well, I worry it could feel arbitrary and/or ambiguous.
• There's a case for not one general thing. Imagine #[diagnostic::attr(note = "{bikeshed_this}")] impl Foo for Bar {}, does this print Foo or Bar?
• In programming generally, even if it's not an actual keyword, this is widely understood to refer to the current object/context, so it seems appropriate here.

for paths in expressions and type paths it's the prefix as written

What do you mean with this? The attribute is not supported on expressions and paths - if it somehow works then that's a bug.

[traviscross]

on a module it's the module's name for imports, but for paths in expressions and type paths it's the prefix as written

What do you mean with this? The attribute is not supported on expressions and paths - if it somehow works then that's a bug.

#![feature(diagnostic_on_unknown)]

mod a {
    // On a module...
    #[diagnostic::on_unknown(
        message = "This={This}, Unresolved={Unresolved}",
    )]
    pub mod b {}
}

mod c {
    pub use super::a::b as d;
}

// ...it's the module's name for imports...
use c::d::x;
// Prints: "This=b, Unresolved=x"

fn main() {
    // ...but for paths in expressions and type paths, it's the prefix
    // as written.
    let _ = c::d::X();
    // Prints: "This=c::d, Unresolved=X"
    let _: c::d::X;
    // Prints: "This=c::d, Unresolved=X"
}

Playground link

[mejrs]

oh, I'll see about a fix for that

edit: #164178