rework rigid alias handling #136863

lcnr · 2025-02-11T12:12:02Z

Necessary for #136824 if we treat coinductive cycles as errors as we otherwise don't emit an error for

trait Overflow {
    type Assoc;
}
impl<T> Overflow for T {
    type Assoc = <T as Overflow>::Assoc;
}

The important part is that we only add a RigidAlias candidate in cases where the alias is actually supposed to be rigid:

its trait bound has been proven via a ParamEnv or ItemBound candidate
it's one of the special builtin traits which have a blanket impl with a default assoc type

This means that we now more explicitly control which aliases should rigid to avoid accidentally accepting cyclic aliases. This requires changes to diagnostics as we no longer enter an explicit RigidAlias candidate for NormalizesTo goals whose trait bound doesn't hold.

To fix this I've modified the BestObligation visitor always ignore RigidAlias candidates and to instead manually check these requirements if there are no applicable candidates. I also removed the hack for handling structurally_normalize_ty failures. This fixes #134905 as we no longer continue to use the EvalCtxt even though a nested goal failed.

r? @compiler-errors

rustbot · 2025-02-12T10:44:46Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs

lcnr · 2025-02-12T10:51:39Z

tests/ui/impl-trait/ice-unexpected-param-type-whensubstituting-in-region-112823.rs

@@ -26,9 +26,11 @@ impl X for Y {
    //~^ ERROR type `LineStream` has 0 type parameters but its trait declaration has 1 type parameter
    type LineStreamFut<'a, Repr> = impl Future<Output = Self::LineStream<'a, Repr>>;
    fn line_stream<'a, Repr>(&'a self) -> Self::LineStreamFut<'a, Repr> {}


This error is a bit iffy, ideally it would point to the error when failing to prove AliasRelate(impl Future<Output = Self::LineStream<'a, Repr>>, ()), However, this is a nested goal with GoalSource::Misc of an NormalizesTo goal, so we currently do not recurse into it in the BestObligations visitor.

Think that we should probably look at Misc goals if all non-misc goals fail or something. This requires some experimentation so I would leave it for a separate PR.

I'd rather we figure out how to mark specific goals that are currently marked as misc today. Looking into all misc goals seems way too broad.

compiler-errors · 2025-02-12T20:10:51Z

compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs

+                match pred_kind.no_bound_vars() {
+                    Some(ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred))) => {
+                        self.detect_error_in_self_ty_normalization(goal, pred.self_ty())?;
+                    }
+                    Some(ty::PredicateKind::NormalizesTo(pred))
+                        if let ty::AliasTermKind::ProjectionTy
+                        | ty::AliasTermKind::ProjectionConst = pred.alias.kind(tcx) =>
+                    {
+                        self.detect_error_in_self_ty_normalization(goal, pred.alias.self_ty())?;
+                        // It is likely that `NormalizesTo` failed because the alias is not well-formed.
+                        // As we only enter `RigidAlias` candidates if the trait bound of the associated type
+                        // holds, we discard these candidates in `non_trivial_candidates` and always manually
+                        // check this here.
+                        let obligation = Obligation::new(
+                            tcx,
+                            self.obligation.cause.clone(),
+                            goal.goal().param_env,
+                            pred.alias.trait_ref(tcx),
+                        );
+                        self.with_derived_obligation(obligation, |this| {
+                            goal.infcx().visit_proof_tree_at_depth(
+                                goal.goal().with(tcx, pred.alias.trait_ref(tcx)),
+                                goal.depth() + 1,
+                                this,
+                            )
+                        })?;
+                    }
+                    Some(_) | None => {}
+                }

-        let [candidate] = candidates.as_slice() else {
-            return ControlFlow::Break(self.obligation.clone());
+                return ControlFlow::Break(self.obligation.clone());


pull this into a helper function

The whole highlighted thing

compiler-errors · 2025-02-12T20:27:41Z

r=me after nit

lcnr · 2025-02-13T09:48:48Z

@bors r=compiler-errors rollup

bors · 2025-02-13T09:48:52Z

📌 Commit f5ebd71 has been approved by compiler-errors

It is now in the queue for this repository.

compiler-errors · 2025-02-13T20:15:51Z

@bors r-

Sorry, I meant moving the whole "handle when there is no candidate" into a subfn.

compiler-errors · 2025-02-13T20:25:36Z

Pushed the change I wanted to see (pulling the nontrivial match on the maybe-no-bound-vars-predicate-kind), if it's ok w/ you then r=me, otherwise maybe rework it to how you'd like to see it and I'll re-review.

compiler-errors · 2025-02-13T20:38:14Z

compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs

+        })
+    }
+
+    /// If we have no candidates, then it's likely that we a non-well-formed alias


typo xd

we have

lcnr · 2025-02-13T22:49:23Z

@bors r=compiler-errors

bors · 2025-02-13T22:49:26Z

📌 Commit 059288e has been approved by compiler-errors

It is now in the queue for this repository.

…rors rework rigid alias handling Necessary for rust-lang#136824 if we treat coinductive cycles as errors as we otherwise don't emit an error for ```rust trait Overflow { type Assoc; } impl<T> Overflow for T { type Assoc = <T as Overflow>::Assoc; } ``` The important part is that we only add a `RigidAlias` candidate in cases where the alias is actually supposed to be rigid: - its trait bound has been proven via a `ParamEnv` or `ItemBound` candidate - it's one of the special builtin traits which have a blanket impl with a `default` assoc type This means that we now more explicitly control which aliases should rigid to avoid accidentally accepting cyclic aliases. This requires changes to diagnostics as we no longer enter an explicit `RigidAlias` candidate for `NormalizesTo` goals whose trait bound doesn't hold. To fix this I've modified the `BestObligation` visitor always ignore `RigidAlias` candidates and to instead manually check these requirements if there are no applicable candidates. I also removed the hack for handling `structurally_normalize_ty` failures. This fixes rust-lang#134905 as we no longer continue to use the `EvalCtxt` even though a nested goal failed. r? `@compiler-errors`

…kingjubilee Rollup of 11 pull requests Successful merges: - rust-lang#136863 (rework rigid alias handling ) - rust-lang#136869 (Fix diagnostic when using = instead of : in let binding) - rust-lang#136895 (debuginfo: Set bitwidth appropriately in enum variant tags) - rust-lang#136928 (eagerly prove WF when resolving fully qualified paths) - rust-lang#136941 (Move `llvm.ccache` to `build.ccache`) - rust-lang#136950 (rustdoc: use better, consistent SVG icons for scraped examples) - rust-lang#136957 (coverage: Eliminate more counters by giving them to unreachable nodes) - rust-lang#136960 (Compiletest should not inherit all host RUSTFLAGS) - rust-lang#136962 (unify LLVM version finding logic) - rust-lang#136970 (ci: move `x86_64-gnu-debug` job to the free runner) - rust-lang#136973 (Fix `x test --stage 1 ui-fulldeps` on macOS (until the next beta bump)) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#136863 - lcnr:treat-as-rigid, r=compiler-errors rework rigid alias handling Necessary for rust-lang#136824 if we treat coinductive cycles as errors as we otherwise don't emit an error for ```rust trait Overflow { type Assoc; } impl<T> Overflow for T { type Assoc = <T as Overflow>::Assoc; } ``` The important part is that we only add a `RigidAlias` candidate in cases where the alias is actually supposed to be rigid: - its trait bound has been proven via a `ParamEnv` or `ItemBound` candidate - it's one of the special builtin traits which have a blanket impl with a `default` assoc type This means that we now more explicitly control which aliases should rigid to avoid accidentally accepting cyclic aliases. This requires changes to diagnostics as we no longer enter an explicit `RigidAlias` candidate for `NormalizesTo` goals whose trait bound doesn't hold. To fix this I've modified the `BestObligation` visitor always ignore `RigidAlias` candidates and to instead manually check these requirements if there are no applicable candidates. I also removed the hack for handling `structurally_normalize_ty` failures. This fixes rust-lang#134905 as we no longer continue to use the `EvalCtxt` even though a nested goal failed. r? ``@compiler-errors``

…tem-bounds, r=lcnr Deeply normalize item bounds in new solver Built on rust-lang#136863. Fixes rust-lang/trait-system-refactor-initiative#142. Fixes rust-lang/trait-system-refactor-initiative#151. cc rust-lang/trait-system-refactor-initiative#116 First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env. Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out. r? lcnr

Kobzol · 2025-02-18T08:47:54Z

@rust-timer build dc6c67c

…tem-bounds, r=lcnr Deeply normalize item bounds in new solver Built on rust-lang#136863. Fixes rust-lang/trait-system-refactor-initiative#142. Fixes rust-lang/trait-system-refactor-initiative#151. cc rust-lang/trait-system-refactor-initiative#116 First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env. Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out. r? lcnr

rust-timer · 2025-02-18T14:03:48Z

Finished benchmarking commit (dc6c67c): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

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

Max RSS (memory usage)

Results (primary 2.1%)

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)	2.1%	[2.1%, 2.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.1%	[2.1%, 2.1%]	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: 789.427s -> 787.705s (-0.22%)
Artifact size: 347.75 MiB -> 347.75 MiB (0.00%)

…tem-bounds, r=lcnr Deeply normalize item bounds in new solver Built on rust-lang#136863. Fixes rust-lang/trait-system-refactor-initiative#142. Fixes rust-lang/trait-system-refactor-initiative#151. cc rust-lang/trait-system-refactor-initiative#116 First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env. Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out. r? lcnr

Rollup merge of rust-lang#137000 - compiler-errors:deeply-normalize-item-bounds, r=lcnr Deeply normalize item bounds in new solver Built on rust-lang#136863. Fixes rust-lang/trait-system-refactor-initiative#142. Fixes rust-lang/trait-system-refactor-initiative#151. cc rust-lang/trait-system-refactor-initiative#116 First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env. Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out. r? lcnr

rustbot assigned compiler-errors Feb 11, 2025

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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Feb 11, 2025

This comment has been minimized.

Sign in to view

lcnr mentioned this pull request Feb 11, 2025

[DO NOT MERGE] bootstrap with -Znext-solver=globally #133502

Draft

This comment has been minimized.

Sign in to view

lcnr force-pushed the treat-as-rigid branch from 259fa93 to d2c0c50 Compare February 12, 2025 08:09

This comment has been minimized.

Sign in to view

lcnr force-pushed the treat-as-rigid branch 2 times, most recently from cf2cf0e to 374b121 Compare February 12, 2025 09:14

This comment has been minimized.

Sign in to view

lcnr force-pushed the treat-as-rigid branch from 374b121 to 55255ca Compare February 12, 2025 10:42

lcnr marked this pull request as ready for review February 12, 2025 10:44

lcnr force-pushed the treat-as-rigid branch from 55255ca to 45fee10 Compare February 12, 2025 10:46

lcnr commented Feb 12, 2025

View reviewed changes

compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs Show resolved Hide resolved

lcnr commented Feb 12, 2025

View reviewed changes

compiler-errors reviewed Feb 12, 2025

View reviewed changes

lcnr force-pushed the treat-as-rigid branch 2 times, most recently from 1f133ae to f5ebd71 Compare February 13, 2025 09:48

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2025

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 13, 2025

normalizes-to rework rigid alias handling

de273e4

rework pointee handling for the new rigid alias approach

05bd5ce

compiler-errors force-pushed the treat-as-rigid branch from f5ebd71 to 42641c5 Compare February 13, 2025 20:24

compiler-errors reviewed Feb 13, 2025

View reviewed changes

adjust derive_error

059288e

lcnr force-pushed the treat-as-rigid branch from 42641c5 to 059288e Compare February 13, 2025 22:49

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 13, 2025

workingjubilee mentioned this pull request Feb 14, 2025

Rollup of 12 pull requests #136999

Closed

compiler-errors mentioned this pull request Feb 14, 2025

Deeply normalize item bounds in new solver #137000

Merged

workingjubilee mentioned this pull request Feb 14, 2025

Rollup of 11 pull requests #137001

Merged

bors merged commit e2ee9f7 into rust-lang:master Feb 14, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Feb 14, 2025

This comment has been minimized.

Sign in to view

lcnr deleted the treat-as-rigid branch February 18, 2025 08:50

matthiaskrgr mentioned this pull request Apr 5, 2025

ICE: next solver: Failed to normalize Alias(Projection, AliasTy #139408

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rework rigid alias handling #136863

rework rigid alias handling #136863

lcnr commented Feb 11, 2025 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Feb 12, 2025

lcnr Feb 12, 2025

compiler-errors Feb 12, 2025

compiler-errors Feb 12, 2025

compiler-errors Feb 13, 2025

compiler-errors commented Feb 12, 2025

lcnr commented Feb 13, 2025

bors commented Feb 13, 2025

compiler-errors commented Feb 13, 2025

compiler-errors commented Feb 13, 2025

compiler-errors Feb 13, 2025

lcnr commented Feb 13, 2025

bors commented Feb 13, 2025

Kobzol commented Feb 18, 2025

This comment has been minimized.

rust-timer commented Feb 18, 2025

rework rigid alias handling #136863

rework rigid alias handling #136863

Conversation

lcnr commented Feb 11, 2025 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Feb 12, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors commented Feb 12, 2025

lcnr commented Feb 13, 2025

bors commented Feb 13, 2025

compiler-errors commented Feb 13, 2025

compiler-errors commented Feb 13, 2025

Choose a reason for hiding this comment

lcnr commented Feb 13, 2025

bors commented Feb 13, 2025

Kobzol commented Feb 18, 2025

This comment has been minimized.

rust-timer commented Feb 18, 2025

Overall result: no relevant changes - no action needed

lcnr commented Feb 11, 2025 •

edited

Loading