Skip to content

Diagnostic assumes that braced unresolved identifiers are formatting arguments #141350

@fmease

Description

@fmease
Member

Uplifted from #141213 (comment). Given

struct Type { field: i32 }

impl Type {
    fn method(&self) { {field} }
}

the compiler outputs

error[E0425]: cannot find value `field` in this scope
 --> file.rs:4:25
  |
4 |     fn method(&self) { {field} }
  |                         ^^^^^
  |
  = help: you might have meant to use the available field in a format string: `"{}", self.field`

warning: unnecessary braces around block return value
 --> file.rs:4:24
  |
4 |     fn method(&self) { {field} }
  |                        ^     ^
  |
  = note: `#[warn(unused_braces)]` on by default
help: remove these braces
  |
4 -     fn method(&self) { {field} }
4 +     fn method(&self) { field }
  |

error: aborting due to 1 previous error; 1 warning emitted

Notice the (unstructured) suggestion you might have meant to use the available field in a format string: `"{}", self.field` which is obviously incorrect, there's no format string in sight and fn method(&self) { "{}", self.field; } is butchered.

For context, this regressed in PR #141213.

Version Information

rustc 1.89.0-nightly (c43786c9b 2025-05-21)
binary: rustc
commit-hash: c43786c9b7b8d8dcc3f9c604e0e3074c16ed69d3
commit-date: 2025-05-21
host: x86_64-unknown-linux-gnu
release: 1.89.0-nightly
LLVM version: 20.1.5

Activity

added
A-diagnosticsArea: Messages for errors, warnings, and lints
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
D-papercutDiagnostics: An error or lint that needs small tweaks.
D-incorrectDiagnostics: A diagnostic that is giving misleading or incorrect information.
on May 21, 2025
fmease

fmease commented on May 21, 2025

@fmease
MemberAuthor

We might be able to fix this by marking the span with DesugaringKind::FormatArgs (yet to be defined) by using mark_with_reason during the lowering of ast::FormatArgs (no surface syntax) (not to be confused with the expansion of format_args!(…)) and querying it at the relevant site.

However, even then we'd need to be wary of cases like format!("{}", {field}) (we don't want to (essentially) suggest format!("{}", "{}", self.field)).

added
A-resolveArea: Name/path resolution done by `rustc_resolve` specifically
on May 21, 2025
changed the title [-]Diagnostic assumes that braced unresolved identifiers are formatting argument[/-] [+]Diagnostic assumes that braced unresolved identifiers are formatting arguments[/+] on May 21, 2025
xizheyin

xizheyin commented on May 21, 2025

@xizheyin
Contributor

Great case, I'll look at adding DesugaringKind::FormatArgs in the near future if you're not going to do that.

xizheyin

xizheyin commented on May 21, 2025

@xizheyin
Contributor

@rustbot claim

fmease

fmease commented on May 21, 2025

@fmease
MemberAuthor

Also CC @mejrs to whom I recently also suggested to create DesugaringKind::FormatArgs & to mark_with_reason during the lowering of ast::FormatArgs for an unrelated diagnostic improvement. Pinging them to avoid duplicate efforts.

mejrs

mejrs commented on May 21, 2025

@mejrs
Contributor

I did not get that far with the implementation of the desugaring aspect of it, that turned out to be more complicated than I thought. Feel free to take it if you want.

Just some thought:

  • For example the format specifiers do not get a span when the literal is first parsed, so changes need to happen there as well.
  • I found it more useful to just have the desugaring pointing to the literal itself, not the positional or indexed arguments so I called it DesugaringKind::FormatLiteral. Open to ideas.
  • the span coming from the expansion also allows some internal features, so needs additional care
  • this should be its own PR imo.
xizheyin

xizheyin commented on May 25, 2025

@xizheyin
Contributor

Here are some notes as I'm going along, and feel free to make some suggestions.

I've found that marking span at ast_lowering doesn't solve the problem because the diagnostic occurs at rustc_resolve, which comes before ast_lowering.

I also tried to mark FormatArgs as soon as the span is generated in rustc_builtin_macros, but found that tcx is missing, and I may have to add a rustc_middle dependency, which I'm worried will interfere with compilation efficiency.

I need to do some more investigation to see if I can mark elsewhere.

mejrs

mejrs commented on May 25, 2025

@mejrs
Contributor

In rustc_resolve we should at least be able to tell if we're in an expansion of some particular macros, using the outer* methods on Span and that the format macros are diagnostic items.

Another way would be to revert #141213 and instead just tell the user to write let x = self.x; or let x = &self.x; in the surrounding scope. This also gets around issues like "what if the macro already has positional arguments".

7 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-diagnosticsArea: Messages for errors, warnings, and lintsA-resolveArea: Name/path resolution done by `rustc_resolve` specificallyD-incorrectDiagnostics: A diagnostic that is giving misleading or incorrect information.D-papercutDiagnostics: An error or lint that needs small tweaks.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Participants

    @fmease@mejrs@xizheyin

    Issue actions

      Diagnostic assumes that braced unresolved identifiers are formatting arguments · Issue #141350 · rust-lang/rust