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

[eggyal]

Undeclared lifetimes that appear in user code always give rise to an error (E0261) early in name resolution. This patch adds such undeclared lifetimes to the relevant context's bound vars, so that they resolve to regions with RegionKind::Error(ErrorGuaranteed), in order to suppress some ensuing diagnostics (that might be resolved if the lifetime becomes properly declared).

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]

This will unconditionally emit an error, whereas before it was suppressed in rustdoc; however I'm not sure that such suppression was correct here anyway—see the comments on Self::should_report_error, which don't appear to apply:

rust/compiler/rustc_resolve/src/late.rs

Lines 4718 to 4720 in a531436

	/// If we're actually rustdoc then avoid giving a name resolution error for `cfg()` items or
	// an invalid `use foo::*;` was found, which can cause unbounded amounts of "item not found"
	// errors. We silence them all.

In any event, this change doesn't appear to be breaking any tests...

[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.

[rust-bors]

☔ The latest upstream changes (presumably #152437) made this pull request unmergeable. Please resolve the merge conflicts.