Allow using bool instead of Option<()> in diagnostics

[clubby789]

Disallow the unit type for #[help], #[note] etc, instead using bool to express optional annotations without a span which I believe is more intuitive.

Test output ordering has changed in a few places, where a field was of type () and the annotation has been moved to the struct itself. If any of these changes are an issue, this can be restricted to allowing specifically (), and not Option<()>

Actual changes here: https://github.com/rust-lang/rust/pull/108402/files#diff-815b1d8debfc564112bd51093791d7c3f2ee288a37a8f5c0e89c11d1f609b4c0

Allows using bool in derive diagnostics to indicate an optional subdiagnostic without a span, where previously Option<()> had to be used

@rustbot label +A-diagnostics

[rustbot]

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Some changes occured in rustc_ty_utils::consts.rs

cc @BoxyUwU

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @TaKO8Ki

[compiler-errors]

Hm... I actually quite like () and Option<()>. They are good ways of indicating that the note will not be applied to a span, forming a parallel to Span and Option<Span>, and the overhead isn't that big when constructing them imo (for the latter, just appending .then_some(()))?

[clubby789]

What do you think about reducing this PR to just allowing bool, and not changing existing diagnostics?

[WaffleLapkin]

I was mildly annoyed by Option<()>s too (actually replacing them with bools was on my todo-list...), hm. Maybe we can add a special enum at least? Something like

pub enum ShowAnnotation { Show, Hide }

[davidtwco]

What do you think about reducing this PR to just allowing bool, and not changing existing diagnostics?

I think this is probably a good step to take. I don't have particularly strong feelings about this change. I liked the symmetry between ()/Option<()> and Span/Option<Span>, but appreciate that writing Some(()) when constructing a error type doesn't necessarily communicate intent very well.

[davidtwco]

@bors r+

[bors]

📌 Commit ef434f0 has been approved by davidtwco

It is now in the queue for this repository.

[bors]

⌛ Testing commit ef434f0 with merge f63ccaf...

[bors]

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing f63ccaf to master...

[rust-timer]

Finished benchmarking commit (f63ccaf): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.4%	[-2.4%, -2.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.4%	[-2.4%, -2.4%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.2%	[2.2%, 2.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0