Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alias-relate results in exponential blowup #68

Closed
lcnr opened this issue Oct 6, 2023 · 0 comments · Fixed by rust-lang/rust#117278
Closed

alias-relate results in exponential blowup #68

lcnr opened this issue Oct 6, 2023 · 0 comments · Fixed by rust-lang/rust#117278

Comments

@lcnr
Copy link

lcnr commented Oct 6, 2023

alias-relate currently assembles three candidates and then decides which candidate should be used:

  • lhs-normalizes-to-rhs: checks normalize(lhs) == rhs
  • rhs-normalizes-to-lhs: checks lhs = normalize(rhs)
  • outside of coherence bidirectional-normalizes-to: normalize(lhs) == normalize(rhs)
  • args-relate: if both aliases have the same DefId, relates their generic arguments

This results in a hang in the following example:

trait Identity {
    type Assoc: ?Sized;
}

impl<T: ?Sized> Identity for T {
    type Assoc = T;
}

type Id<T> = <T as Identity>::Assoc;

type Five<T> = Id<Id<Id<Id<Id<T>>>>>;
// hangs
type Ty<T> = Five<Five<Five<Five<Five<T>>>>>;
// ok
// type Ty<T> = Five<T>;

trait Trait<T> {}

impl<T> Trait<T> for Ty<T> {}
impl Trait<u32> for Ty<i32> {}

To see what's going on, consider Id<Id<?x>> == Id<Id<u32>>:

  • alias-relate(Id<Id<?x>>, Id<Id<u32>>)
    • lhs-normalizes-to-rhs ~> alias-relate(Id<?x>, Id<Id<u32>>)
      • lhs-normalizes-to-rhs ~> ?x constrained to Id<Id<u32>>
      • rhs-normalizes-to-lhs ~> alias-relate(Id<?x>, Id<u32>)
        • lhs-normalizes-to-rhs ~> ?x constrained to Id<u32>
        • rhs-normalizes-to-lhs ~> alias-relate(Id<?x>, u32)
          • lhs-normalizes-to-rhs ~> ?x constrained to u32
        • args-relate ~> ?x constrained to u32
        • candidites with different results ~> AMBIGUITY
      • args-relate ~> ?x constrained to Id<u32>
      • candidates with different results ~> AMBIGUITY
    • rhs-normalizes-to-lhs ~> alias-relate(Id<Id<?x>>, Id<u32>)
      • ...
    • args-relate ~> alias-relate(Id<?x>, Id<u32>)
      • ...

Even when ignoring cache hits, the blowup is still depth^2 while normalizing <alias as Identity>::Assoc also normalizes the self type when searching for candidates.

I would like to avoid this issue somehow, something like try_normalize_ty(lhs) == try_normalize_ty(rhs) would avoid this blowup but has other issues and is non-trivial.

bors added a commit to rust-lang/miri that referenced this issue Nov 21, 2023
new solver normalization improvements

cool beans

At the core of this PR is a `try_normalize_ty` which stops for rigid aliases by using `commit_if_ok`.

Reworks alias-relate to fully normalize both the lhs and rhs and then equate the resulting rigid (or inference) types. This fixes rust-lang/trait-system-refactor-initiative#68 by avoiding the exponential blowup. Also supersedes #116369 by only defining opaque types if the hidden type is rigid.

I removed the stability check in `EvalCtxt::evaluate_goal` due to rust-lang/trait-system-refactor-initiative#75. While I personally have opinions on how to fix it, that still requires further t-types/`@nikomatsakis` buy-in, so I removed that for now. Once we've decided on our approach there, we can revert this commit.

r? `@compiler-errors`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
new solver normalization improvements

cool beans

At the core of this PR is a `try_normalize_ty` which stops for rigid aliases by using `commit_if_ok`.

Reworks alias-relate to fully normalize both the lhs and rhs and then equate the resulting rigid (or inference) types. This fixes rust-lang/trait-system-refactor-initiative#68 by avoiding the exponential blowup. Also supersedes #116369 by only defining opaque types if the hidden type is rigid.

I removed the stability check in `EvalCtxt::evaluate_goal` due to rust-lang/trait-system-refactor-initiative#75. While I personally have opinions on how to fix it, that still requires further t-types/`@nikomatsakis` buy-in, so I removed that for now. Once we've decided on our approach there, we can revert this commit.

r? `@compiler-errors`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
new solver normalization improvements

cool beans

At the core of this PR is a `try_normalize_ty` which stops for rigid aliases by using `commit_if_ok`.

Reworks alias-relate to fully normalize both the lhs and rhs and then equate the resulting rigid (or inference) types. This fixes rust-lang/trait-system-refactor-initiative#68 by avoiding the exponential blowup. Also supersedes #116369 by only defining opaque types if the hidden type is rigid.

I removed the stability check in `EvalCtxt::evaluate_goal` due to rust-lang/trait-system-refactor-initiative#75. While I personally have opinions on how to fix it, that still requires further t-types/`@nikomatsakis` buy-in, so I removed that for now. Once we've decided on our approach there, we can revert this commit.

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant