generalize: handle occurs check failure in aliases #117088

lcnr · 2023-10-23T14:50:32Z

mostly fixes #105787, except for the for<'a> fn(<<?x as OtherTrait>::Assoc as Trait<'a>>::Assoc) eq ?x case in rust-lang/trait-system-refactor-initiative#8.

r? @compiler-errors

lcnr · 2023-10-23T14:55:48Z

compiler/rustc_infer/src/infer/generalize.rs

+    /// incorrect errors when relating `?0` with `<?0 as Trait>::Assoc`.
+    in_alias: bool,


exists because of <() as Trait<<?0 as Trait<()>>::Assoc>>::Assoc == ?0 have to update comment. With Projection goals the behavior is still broken, but I want to change it to emit alias-relate in the new solver in a different PR, at which point this should work.

lcnr · 2023-10-23T14:57:24Z

compiler/rustc_infer/src/infer/generalize.rs

+                            if !infer_replacement_is_complete {
+                                warn!("incomplete generalization of an alias type: {t:?}");
+                            }


handling this correctly is HARD. We'd have to push the infer var to the place which introduced the binder and then have a nested Equate goal or sth.

idk how to handle this during coherence, ideally just succeed with equating and emit an Ambiguity predicate?

lcnr · 2023-10-23T15:01:51Z

compiler/rustc_infer/src/infer/nll_relate/mod.rs

+
+        let ty = value.may_be_infer();
+        if ty.is_ty_var() {
+            span_bug!(self.delegate.span(), "occurs check failure in MIR typeck");


potentially reachable with user type annotations? Probably not as the actual type inferred by HIR typeck also needs to contain an inference variable? 🤔

compiler/rustc_infer/src/infer/generalize.rs

compiler/rustc_infer/src/infer/combine.rs

compiler-errors · 2023-10-25T15:24:31Z

this is still in draft, so marking as author so i can clean up my queue :>

@rustbot author

lcnr · 2023-10-26T08:22:26Z

This PR mostly fixes generalization for aliases, fixing #105787. It is therefore a (theoretical) breaking change.

Equating ?x == Alias<?x> previously failed. This is incomplete (and therefore allows overlaps via the implicit negative overlap check in coherence). This PR instead delays equality in this case, emitting a Projection goal. This projection goal then stalls until the projection can be normalized to a rigid type, in which case the occurs check is correct, or the projection cannot be normalized and stays rigid (due to a ParamEnv candidate).

It is non-trivial whether we can get trait solver overflow (in the old solver) for rigid projections, e.g.

trait Unnormalizable<T> {
    type Assoc;
}

struct Inv<T>(*mut T);

fn unconstrained<T>() -> T {
    todo!()
}

fn create<T, U: Unnormalizable<T>>(
    x: &U,
) -> (Inv<T>, Inv<<U as Unnormalizable<T>>::Assoc>) {
    todo!()
}

fn foo<T: Unnormalizable<U> + Unnormalizable<V>, U, V>() {
    let q = unconstrained();
    let (mut x, y) = create::<_, _>(&q);
    x = y; // equate `?x` with `<T as Unnormalizable<?x>>::Assoc`
    drop::<T>(q);
}

However, projection goals only constrain the term to the projection itself if we were able to select a ParamEnv candidate. As we do not have non-lifetime binders, this only happens if the projection does not contain type inference variables.

I believe that you can get solver overflow with specialization and default associated items, but idc 😁

This only fixes problem case 2 of rust-lang/trait-system-refactor-initiative#8. However, this is by far the easiest way to get an incorrect occurs check failure (and the only one I bothered to write a coherence unsoundness test for). This only impacts the old solver for projections containing bound variables (as we otherwise eagerly normalize and replace the projection with an inference variable).

I consider this to 1) be a step in the right direction and 2) necessary for future work on the new trait solver.

lcnr · 2023-10-26T08:28:23Z

should have done perf + crater 😅

@bors try @rust-timer queue

rfcbot · 2023-12-03T17:55:22Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

compiler-errors · 2023-12-03T17:57:16Z

r=me after rebase

lcnr · 2023-12-04T09:52:45Z

@bors r+ rollup=never

bors · 2023-12-04T09:52:48Z

📌 Commit cf8a2bd has been approved by lcnr

It is now in the queue for this repository.

compiler-errors · 2023-12-04T09:58:14Z

@bors r+

approver is wrong

bors · 2023-12-04T09:58:16Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: Change prefetch to avoid deadlock #118488

bors · 2023-12-04T09:58:17Z

📌 Commit cf8a2bd has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-12-04T10:29:46Z

⌛ Testing commit cf8a2bd with merge 90f22e7...

…rors generalize: handle occurs check failure in aliases mostly fixes rust-lang#105787, except for the `for<'a> fn(<<?x as OtherTrait>::Assoc as Trait<'a>>::Assoc) eq ?x` case in rust-lang/trait-system-refactor-initiative#8. r? `@compiler-errors`

lnicola · 2023-12-04T10:30:29Z

This is going to fail, CC #118597. ~~Maybe you can take a look, @compiler-errors, since you seem to be around.~~

Actually, @pietroalbini might be a better person to tag, sorry for the mix-up.

compiler-errors · 2023-12-04T10:37:09Z

@bors retry

kicking the job to avoid failure

rust-log-analyzer · 2023-12-04T11:09:39Z

The job x86_64-gnu-aux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

thread 'main' panicked at src/tools/cargotest/main.rs:148:13:
assertion failed: status.success()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Build completed unsuccessfully in 0:36:43
make: *** [Makefile:51: check-aux] Error 1
  network time: Mon, 04 Dec 2023 11:09:26 GMT
##[error]Process completed with exit code 2.
Post job cleanup.

bors · 2023-12-05T00:38:01Z

⌛ Testing commit cf8a2bd with merge 25dca40...

bors · 2023-12-05T02:35:43Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 25dca40 to master...

rust-timer · 2023-12-05T04:47:39Z

Finished benchmarking commit (25dca40): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

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)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	2.7%	[2.6%, 2.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.1%	[-1.1%, -1.1%]	1
All ❌✅ (primary)	1.7%	[1.7%, 1.7%]	1

Cycles

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

Binary size

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

Bootstrap: 672.674s -> 673.516s (0.13%)
Artifact size: 314.14 MiB -> 314.15 MiB (0.00%)

…3, r=BoxyUwU split `NormalizesTo` out of `Projection` 3 third attempt at rust-lang#112658. Rebasing rust-lang#116262 is very annoying, so I am doing it again from scratch. We should now be able to merge it without regressing anything as we handle occurs check failures involving aliases correctly since rust-lang#117088. see https://hackmd.io/ktEL8knTSYmtdfrMMnA-Hg fixes rust-lang/trait-system-refactor-initiative#1 r? `@compiler-errors`

rustbot assigned compiler-errors Oct 23, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 23, 2023

lcnr commented Oct 23, 2023

View reviewed changes

compiler-errors reviewed Oct 23, 2023

View reviewed changes

compiler/rustc_infer/src/infer/generalize.rs Outdated Show resolved Hide resolved

compiler-errors reviewed Oct 23, 2023

View reviewed changes

compiler/rustc_infer/src/infer/combine.rs Show resolved Hide resolved

lcnr force-pushed the generalize-alias branch from 96204ee to 6fdb8cb Compare October 23, 2023 16:09

lcnr added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Oct 23, 2023

This comment has been minimized.

Sign in to view

lcnr force-pushed the generalize-alias branch from 6fdb8cb to 768015f Compare October 23, 2023 18:13

This comment has been minimized.

Sign in to view

lcnr force-pushed the generalize-alias branch 2 times, most recently from 5840c9d to f817a32 Compare October 24, 2023 07:38

This comment has been minimized.

Sign in to view

lcnr force-pushed the generalize-alias branch from f817a32 to ea6e393 Compare October 24, 2023 07:49

This comment has been minimized.

Sign in to view

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 Oct 25, 2023

lcnr force-pushed the generalize-alias branch from ea6e393 to 6913673 Compare October 26, 2023 08:08

lcnr marked this pull request as ready for review October 26, 2023 08:08

lcnr closed this Oct 26, 2023

lcnr reopened this Oct 26, 2023

lcnr added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 26, 2023

This comment has been minimized.

Sign in to view

lcnr added 4 commits December 4, 2023 10:38

do not fetch variance info during generalization

2d0ec17

generalize: handle occurs check failure in aliases

f69d672

cleanup and comments

407c117

rebase

cf8a2bd

lcnr force-pushed the generalize-alias branch from 6913673 to cf8a2bd Compare December 4, 2023 09:49

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 4, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 5, 2023

bors merged commit 25dca40 into rust-lang:master Dec 5, 2023
12 checks passed

rustbot added this to the 1.76.0 milestone Dec 5, 2023

lcnr deleted the generalize-alias branch December 5, 2023 13:26

DaniPopes mentioned this pull request Dec 6, 2023

nightly docs: called Result::unwrap_err() on an Ok value #118670

Closed

lcnr mentioned this pull request Dec 7, 2023

split NormalizesTo out of Projection 3 #118725

Merged

lcnr mentioned this pull request Dec 11, 2023

occurs check with aliases results in error, not ambiguity #105787

Open

apiraino removed the to-announce Announce this issue on triage meeting label Dec 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

generalize: handle occurs check failure in aliases #117088

generalize: handle occurs check failure in aliases #117088

lcnr commented Oct 23, 2023 •

edited

Loading

lcnr Oct 23, 2023

lcnr Oct 23, 2023

lcnr Oct 23, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

compiler-errors commented Oct 25, 2023

lcnr commented Oct 26, 2023 •

edited

Loading

lcnr commented Oct 26, 2023 •

edited

Loading

This comment has been minimized.

rfcbot commented Dec 3, 2023

compiler-errors commented Dec 3, 2023

lcnr commented Dec 4, 2023

bors commented Dec 4, 2023

compiler-errors commented Dec 4, 2023

bors commented Dec 4, 2023

bors commented Dec 4, 2023

bors commented Dec 4, 2023

lnicola commented Dec 4, 2023 •

edited

Loading

compiler-errors commented Dec 4, 2023 •

edited

Loading

rust-log-analyzer commented Dec 4, 2023

bors commented Dec 5, 2023

bors commented Dec 5, 2023

rust-timer commented Dec 5, 2023

		/// incorrect errors when relating `?0` with `<?0 as Trait>::Assoc`.
		in_alias: bool,

generalize: handle occurs check failure in aliases #117088

generalize: handle occurs check failure in aliases #117088

Conversation

lcnr commented Oct 23, 2023 • edited Loading

lcnr Oct 23, 2023

Choose a reason for hiding this comment

lcnr Oct 23, 2023

Choose a reason for hiding this comment

lcnr Oct 23, 2023

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

compiler-errors commented Oct 25, 2023

lcnr commented Oct 26, 2023 • edited Loading

lcnr commented Oct 26, 2023 • edited Loading

This comment has been minimized.

rfcbot commented Dec 3, 2023

compiler-errors commented Dec 3, 2023

lcnr commented Dec 4, 2023

bors commented Dec 4, 2023

compiler-errors commented Dec 4, 2023

bors commented Dec 4, 2023

bors commented Dec 4, 2023

bors commented Dec 4, 2023

lnicola commented Dec 4, 2023 • edited Loading

compiler-errors commented Dec 4, 2023 • edited Loading

rust-log-analyzer commented Dec 4, 2023

bors commented Dec 5, 2023

bors commented Dec 5, 2023

rust-timer commented Dec 5, 2023

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

lcnr commented Oct 23, 2023 •

edited

Loading

lcnr commented Oct 26, 2023 •

edited

Loading

lcnr commented Oct 26, 2023 •

edited

Loading

lnicola commented Dec 4, 2023 •

edited

Loading

compiler-errors commented Dec 4, 2023 •

edited

Loading