Feed ErrorGuaranteed from late lifetime resolution errors through to bound variable resolution

[eggyal]

If late lifetime resolution fails for whatever reason, forward to RBV the guarantee that an error was emitted - thereby eliminating the need for a "hack" to suppress subsequent/superfluous error diagnostics.

Fixes #152014
r? fmease

[rustbot]

Changes to the size of AST and/or HIR nodes.

cc @nnethercote

[rustbot]

fmease is currently at their maximum review capacity.
They may take a while to respond.

[fmease]

Thanks for working on this! I won't be able to review this tonight but I'd like to note that thanks to your changes we should now be able to get rid of the minor hack in HIR ty lowering as I've mentioned in the issue:

rust/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_trait.rs

Lines 454 to 464 in f60a0f1

	if let hir::Node::Ty(hir::Ty {
	kind: hir::TyKind::Ref(parent_lifetime, _),
	..
	}) = tcx.parent_hir_node(hir_id)
	&& tcx.named_bound_var(parent_lifetime.hir_id).is_none()
	{
	// Parent lifetime must have failed to resolve. Don't emit a redundant error.
	RegionInferReason::ExplicitObjectLifetime
	} else {
	RegionInferReason::ObjectLifetimeDefault
	}

(by replacing that whole snippet with just RegionInferReason::ObjectLifetimeDefault)

View changes since this review

[fmease]

Preliminary sanity perf check due to size changes.

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: e81e54f (e81e54fb4fbe70e3d11975443d97b3652269a38d, parent: 366a1b93e7f466ebe559477add05f064873d0c71)

[rust-timer]

Finished benchmarking commit (e81e54f): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.2%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 3.1%, secondary -0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.1%	[3.1%, 3.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.9%	[-0.9%, -0.8%]	3
All ❌✅ (primary)	3.1%	[3.1%, 3.1%]	1

Cycles

Results (secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Binary size

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

Bootstrap: 472.721s -> 472.345s (-0.08%)
Artifact size: 397.76 MiB -> 397.70 MiB (-0.02%)

[rustbot]

HIR ty lowering was modified

cc @fmease

[eggyal]

Whereas in 08ed1a1 (now fa5cccf) I was only feeding ErrorGuaranteed through to RBV in the specific case of undeclared named lifetimes, in a531436 (now 8e83a57) I've extended the logic to feed through such an ErrorGuaranteed for every early lifetime resolution error. This enables the aforementioned "hack" to be removed without giving rise to new instances of E0228 ("the lifetime bound for this object type cannot be deduced from context"), however it does also suppress a few other errors that previously were emitted—see updates to test outputs.

View changes since this review

I didn't squash (yet) in case you had begun reviewing and it was easier to see what has since changed. However, these commits really ought to be squashed (with commensurate updates to the PR description and commit comments) before merging, and review may be easier combined in any event.

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[eggyal]

I've responded to the rustdoc issue in that comment thread, but GitHub isn't rendering that (for me) on this page.

@rustbot ready

[fmease]

Thanks! I've just noticed that you've pushed another commit while I was in the midst of reviewing. As I've noted in this review comment you couldn't see yet, I'm actually fine with this "breaking" rustdoc change. My initial concern is unsubstantial.

I'll put it into the queue once you've dropped the RustdocSuppressedError change again (I'm sorry ^^'), impl'ed my suggestions and finally squashed the commits (as you've already offered :D).

View changes since this review

[eggyal]

Thank you for all the excellent feedback—it's a much better PR as a result! ❤️

@rustbot ready

[fmease]

Thanks for these kind works & thanks again for having worked on this!

@bors r+ rollup-

View changes since this review

[rust-bors]

📌 Commit c43a33e has been approved by fmease

It is now in the queue for this repository.