rustc_macros: Make it possible to derive both Diagnostic and LintDiagnostic on the same type

[fmease]

In #117164 I had to copy the diagnostic structs TyParamFirstLocal and TyParamSome and define the separate structs TyParamFirstLocalLint and TyParamSomeLint to be able to use the translatable diagnostics hir_analysis_ty_param_first_local and hir_analysis_ty_param_some as both a Diagnostic and a LintDiagnostic:

rust/compiler/rustc_hir_analysis/src/errors.rs

Lines 1360 to 1406 in b21b74b

	#[derive(Diagnostic)]
	#[diag(hir_analysis_ty_param_first_local, code = E0210)]
	#[note]
	pub struct TyParamFirstLocal<'tcx> {
	#[primary_span]
	#[label]
	pub span: Span,
	#[note(hir_analysis_case_note)]
	pub note: (),
	pub param: Symbol,
	pub local_type: Ty<'tcx>,
	}
	#[derive(LintDiagnostic)]
	#[diag(hir_analysis_ty_param_first_local, code = E0210)]
	#[note]
	pub struct TyParamFirstLocalLint<'tcx> {
	#[label]
	pub span: Span,
	#[note(hir_analysis_case_note)]
	pub note: (),
	pub param: Symbol,
	pub local_type: Ty<'tcx>,
	}
	#[derive(Diagnostic)]
	#[diag(hir_analysis_ty_param_some, code = E0210)]
	#[note]
	pub struct TyParamSome {
	#[primary_span]
	#[label]
	pub span: Span,
	#[note(hir_analysis_only_note)]
	pub note: (),
	pub param: Symbol,
	}
	#[derive(LintDiagnostic)]
	#[diag(hir_analysis_ty_param_some, code = E0210)]
	#[note]
	pub struct TyParamSomeLint {
	#[label]
	pub span: Span,
	#[note(hir_analysis_only_note)]
	pub note: (),
	pub param: Symbol,
	}

In #116829, had to duplicate the lint diagnostic struct ReprConflicting and split it into the separate structs ReprConflicting and ReprConflictingLint to be able to use the translatable diagnostic passes_repr_conflicting as both a Diagnostic and a LintDiagnostic:

rust/compiler/rustc_passes/src/errors.rs

Lines 554 to 563 in b21b74b

	#[derive(Diagnostic)]
	#[diag(passes_repr_conflicting, code = E0566)]
	pub struct ReprConflicting {
	#[primary_span]
	pub hint_spans: Vec<Span>,
	}
	#[derive(LintDiagnostic)]
	#[diag(passes_repr_conflicting, code = E0566)]
	pub struct ReprConflictingLint;

In all three cases, I could've probably turned the derivation of LintDiagnostic into a manual impl but that doesn't seem very enticing either for various reasons.

For context, this situation arises whenever we want to emit a diagnostic for something but can't report a hard error unconditionally in all cases for backward compatibility and therefore have to emit the very same diagnostic at different “levels of severity” depending on a set of conditions. With level of severity I'm specifically referring to the set {hard error, lint} here (where lint has its own level of course).

---

More concretely, the reason why you can't just #[derive(Diagnostic, LintDiagnostic)] struct Ty/*...*/ is because of #[primary_span]: If the diagnostic struct contains a #[primary_span] (which it does most of the time) for the derive macro Diagnostic, then the derive macro LintDiagnostic will reject #[primary_span] rendering the two derives incompatible with one another.

Individually, LintDiagnostic rejecting #[primary_span] makes sense because the span(s) for a lint are to be provided to the function that emits the lint like emit_span_lint, emit_node_span_lint.

---

There are probably multiple ways to approach this but I'm not super familiar with internals of rustc's linting API. Anyways, I'd like to see this “just work” because it won't be the last time this case will occur.

[fmease]

I'll probably investigate getting rid of emit_span_lint, emit_node_span_lint and the like in favor of emit_lint etc. (no span / multi-span parameter) and storing the span(s) of LintDiagnostics in the lint diagnostic structs themselves via #[primary_span] which seems to be the most logical approach. However, it'll require updating all existing lint diagnostic structs.

cc @nnethercote

[compiler-errors]

Yep, I think that LintDiagnostics should be responsible for providing the primary_span to the underlying lint function, just like they do for their primary message. This should be pretty easy to do 👍

[fmease]

Ah, that's annoying. We can't simply leverage Diag::span (inside LintDiagnostic::decorate_lint) unlike Diagnostic because lint_level (lint_level_impl) needs access to the (multi) span before decorate'ing the Diag to suppress lints whose spans come from an external macro (and to disable suggestions):

rust/compiler/rustc_middle/src/lint.rs

Lines 333 to 351 in 4eedf73

	if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
	// Any suggestions made here are likely to be incorrect, so anything we
	// emit shouldn't be automatically fixed by rustfix.
	err.disable_suggestions();
	// If this is a future incompatible that is not an edition fixing lint
	// it'll become a hard error, so we have to emit *something*. Also,
	// if this lint occurs in the expansion of a macro from an external crate,
	// allow individual lints to opt-out from being reported.
	let incompatible = future_incompatible.is_some_and(|f| f.reason.edition().is_none());
	if !incompatible && !lint.report_in_external_macro {
	err.cancel();
	// Don't continue further, since we don't want to have
	// `diag_span_note_once` called for a diagnostic that isn't emitted.
	return;
	}
	}

By the way, I've already migrated all lint diagnostic structs to the new system.

The only solution I see is adding a span method to trait LintDiagnostic (similar to the existing msg method) which I really wanted to avoid because it makes the API more complicated and bloats the generated code.

[fmease]

Right now, my patched rustc fails to build std because of that >.<

[fmease]

fmease commented in #125208 (comment):

Yesterday I coincidentally stumbled upon this Zulip post from 2024-09-25 which essentially declares the current diagnostic translation effort (#100717) unfruitful and unmaintained. While I only read this post yesterday, I have been aware of this fact for some time.

Therefore I don't want to burden davidtwco with re-reviewing >1000 lines and I'm also fine with closing this PR.

Of course, not a lot has changed. It's essentially a rebase from a 5 month old master. I stalled the rebase until now because I knew that this approach (of making lint diagnostics responsible or providing their primary span) does not scale to early (buffered) lints. I came to realize that when reviewing PR #124417 (which made early lints translatable) and implementing PR #125410 (which won back some of the perf regressions caused by the former).

You say it doesn't scale to early (buffered) lints, so how did you succeed in rebasing then while keeping perf in check? Well, I've cheated a little bit. The statement "lints diagnostics are responsible for providing their primary span" now only holds for non-early lints. The early lint diagnostics don't provide a #[primary_span] / return None for LintDiagnostic::span(&self) -> Option<MultiSpan> with the hidden assumption/knowledge1 that it's supplied separately in opt_span_lint_with_diagnostics from BufferedEarlyLint.span.

For perf reasons, we can't eagerly "lower" BuiltinLintDiag to lint diagnostics ("impl LintDiagnostic") inside opt_span_lint_with_diagnostics via decorate_lint2. However, only if we lower eagerly, we can truly make early lint diagnostics responsible for providing their primary span – and that's off the table. Re. perf, the lowering step is expensive and therefore we delay this step as far back as possible. Namely we don't want to lower lint diagnostics that end up not getting emitted (due to lint levels). We achieve this by letting lint_level decide … which takes a Span that is used in its decision process! We need the primary span(s) before lowering to lint diagnostics, therefore lint diagnostics cannot possibly provide their primary span!

This creates a divide between early and late LintDiagnostics which is unacceptable in my opinion. I wanted to bring {,Lint}Diagnostic closer together for ease of understanding and use. This simply doesn't work if LintDiagnostic itself is divided. As such, I no longer believe in this PR.

Footnotes

1. Well, I did add comments in the LintDiagnostic::span cases but not in the #[derive(LintDiagnostic)] ones (there are simply too many). ↩
2. See the merged commit 37bf2d2 which reverted this. ↩