Skip to content

Cleanup unused diagnostic emission methods - part 2#153509

Open
GuillaumeGomez wants to merge 6 commits intorust-lang:mainfrom
GuillaumeGomez:migrate-diag2
Open

Cleanup unused diagnostic emission methods - part 2#153509
GuillaumeGomez wants to merge 6 commits intorust-lang:mainfrom
GuillaumeGomez:migrate-diag2

Conversation

@GuillaumeGomez
Copy link
Member

@rustbot
Copy link
Collaborator

rustbot commented Mar 6, 2026

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 6, 2026
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Mar 6, 2026
"cannot use constants which depend on generic parameters in types",
);
},
ConstDependingOnGenericParam,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we said we'd not convert more things to the struct error style?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I don't remember that. Where did we discuss it? Also, we are removing the node_span_lint method to remove the duplicated lint_level function, so there isn't much of a choice, unless we simply wrap the closure in a type.

Copy link
Contributor

@JonathanBrouwer JonathanBrouwer Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Afaik the consensus is that we can use whatever style is prettier. In this case, I think the struct style is quite pretty. I don't like the explicit impl Diagnostic ones in this PR tho, and would like to explore the closure approach for those

|lint| {
lint.primary_message(format!("feature `{}` is declared but not used", feature));
},
UnusedFeature { feature },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like how positive the diff of this PR is, the explicit Diagnostic implementation is a bit ugly. Could you:

  • For diagnostics like this one, use a struct diagnostic instead
  • For more complicated ones, see if you can invent a syntax similar to the old one with a closure to make the diagnostics a bit less verbose? If this turns out to be difficult I'm happy to discuss some approaches with you, but I feel like this approach is a bit too verbose

@rustbot 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 Mar 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants