SessionDiagnostic derive does not consider #[label = "..."] on Span field valid

[Xiretza]

I tried this code:

#[derive(SessionDiagnostic)]
#[error(parser::if_expression_missing_condition)]
pub(crate) struct IfExpressionMissingCondition {
    #[primary_span]
    #[label = "condition_label"]
    pub if_span: Span,
    #[label = "block_label"]
    pub block_span: Span,
}

I expected to see this happen: As explained in the current rustc-dev-guide, the error message is fetched from parser_if_expression_missing_condition, and the attributes .condition_label and .block_label of this fluent resource are used to create two note subdiagnostics.

Instead, this happened:

error: `#[label = ...]` is not a valid attribute
   --> compiler/rustc_parse/src/parser/diagnostics.rs:568:5
    |
568 |     #[label = "condition_label"]
    |     ^

Working off current rustc master, 9c20b2a.

cc @davidtwco

[Xiretza]

@rustbot label +A-translation

[davidtwco]

That's just somewhere that the dev guide is out of date unfortunately. We used to refer to Fluent slugs as strings, like in these examples, but then we switched to always referring to them using the "typed identifiers" that we generate, as that provides a degree of compile-time validation.

So in these examples, #[label = "bar_foo_qux"] becomes #[label(bar::foo_qux)] and that expands to rustc_errors::fluent::bar::foo_qux.

If you want to submit a pull request to the dev guide with a fix for that then I'd happily approve it, otherwise I'll make a note to do this later today.

[davidtwco]

I'll close this because it's not really a rustc bug, just some out of date documentation in the dev guide.

[Xiretza]

So it's not possible to group multiple labels/notes/etc together with their respective diagnostic?

[davidtwco]

So it's not possible to group multiple labels/notes/etc together with their respective diagnostic?

It's still possible. #[label(typeck::definition_label)] expands to rustc_errors::fluent::typeck::definition_label which corresponds to <whatever-the-primary-slug-is>.definition_label. #[label]/#[help]/etc in SessionDiagnostic and LintDiagnostic are always relative to the primary slug.

[Xiretza]

Huh, that's slightly confusing, but good to know! So attributes really aren't attributes of slugs, but rather also part of the global namespace? In other words, you'd get a name clash if you had both typeck_foo.definition_label and typeck_bar.definition_label, because they both correspond to rustc_errors::fluent::typeck::definition_label?

[davidtwco]

Huh, that's slightly confusing, but good to know! So attributes really aren't attributes of slugs, but rather also part of the global namespace? In other words, you'd get a name clash if you had both typeck_foo.definition_label and typeck_bar.definition_label, because they both correspond to rustc_errors::fluent::typeck::definition_label?

Not quite - we cheat a little here. typeck_bar.definition_label and typeck_foo.definition_label are distinct in Fluent, and would both generate a rustc_errors::fluent::typeck::definition_label, so we just generate that once.

The trick is that rustc_errors::fluent::typeck::definition_label is a SubdiagnosticMessage type, which only contains the "definition_label" string. It is entirely disconnected from the primary slug that it was attached to, and then at diagnostic emission time, when we have the primary slug that it is being emitted relative to, we combine those to get the full Fluent path.

That means you could use a rustc_errors::fluent::typeck::definition_label with a diagnostic that doesn't have a definition_label, and we wouldn't detect that at compile-time, Fluent would just get confused that you're trying to use an attribute that doesn't exist when we go to actually perform the translation. It's not ideal, but it works well enough for now.

[Xiretza]

That's a little hacky, but also means that manually derived SessionSubdiagnostics can use their parent SessionDiagnostic's attributes, which is great (and something I hadn't realized until now)! Thanks a lot for the very fast and thorough explanations.

[davidtwco]

rust-lang/rustc-dev-guide#1436