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-bounds for normalizable alias + deeply normalize ~> MIR typeck ICE #77

Closed
lcnr opened this issue Dec 13, 2023 · 2 comments · Fixed by rust-lang/rust#119744
Closed

Comments

@lcnr
Copy link
Contributor

lcnr commented Dec 13, 2023

trait Trait {
    type Assoc: Clone;
}

fn is_clone<T: Clone>(_: *mut T) {}

fn foo<T: Trait<Assoc = U>, U>(x: *mut <T as Trait>::Assoc) {
    is_clone(x);
}

https://rust.godbolt.org/z/4WojT7KTW

This currently results in an ICE:

  • in HIR typeck <T as Trait>::Assoc: Clone holds due to the alias bound candidate
  • during writeback we deeply normalize, replacing <T as Trait>::Assoc with U
  • MIR typeck checks U: Clone which does not hold, resulting in an ICE
@lcnr
Copy link
Contributor Author

lcnr commented Dec 13, 2023

the easiest fix is to change assemble_candidates_after_normalizing_self_ty to only assemble candidates for a fully normalized type by switching it to use try_normalize_ty 🤔 this matches the behavior of the old solver and should avoid deeply normalizing from erasing relevant info

@compiler-errors
Copy link
Member

the easiest fix is to change assemble_candidates_after_normalizing_self_ty to only assemble candidates for a fully normalized type by switching it to use try_normalize_ty

:( incompleteness

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 26, 2024
always normalize `LoweredTy` in the new solver

I currently expect us to stop using alias bound candidates of normalizable aliases due to rust-lang/trait-system-refactor-initiative#77 by landing rust-lang#119744. At this point it mostly doesn't matter whether we eagerly normalize (and replace with infer vars in case of ambiguity). cc rust-lang#113473 previous attempt

The infer var replacement for ambiguous projections can in very rare cases:
- weaken inference rust-lang/trait-system-refactor-initiative#81
- strengthen inference rust-lang/trait-system-refactor-initiative#7

I do not expect this impact on inference to significantly affect real crates.

r? `@compiler-errors`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 26, 2024
always normalize `LoweredTy` in the new solver

I currently expect us to stop using alias bound candidates of normalizable aliases due to rust-lang/trait-system-refactor-initiative#77 by landing rust-lang#119744. At this point it mostly doesn't matter whether we eagerly normalize (and replace with infer vars in case of ambiguity). cc rust-lang#113473 previous attempt

The infer var replacement for ambiguous projections can in very rare cases:
- weaken inference rust-lang/trait-system-refactor-initiative#81
- strengthen inference rust-lang/trait-system-refactor-initiative#7

I do not expect this impact on inference to significantly affect real crates.

r? ``@compiler-errors``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 27, 2024
Rollup merge of rust-lang#120378 - lcnr:normalize-ast, r=compiler-errors

always normalize `LoweredTy` in the new solver

I currently expect us to stop using alias bound candidates of normalizable aliases due to rust-lang/trait-system-refactor-initiative#77 by landing rust-lang#119744. At this point it mostly doesn't matter whether we eagerly normalize (and replace with infer vars in case of ambiguity). cc rust-lang#113473 previous attempt

The infer var replacement for ambiguous projections can in very rare cases:
- weaken inference rust-lang/trait-system-refactor-initiative#81
- strengthen inference rust-lang/trait-system-refactor-initiative#7

I do not expect this impact on inference to significantly affect real crates.

r? ``@compiler-errors``
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 30, 2024
…-errors

only assemble alias bound candidates for rigid aliases

fixes rust-lang/trait-system-refactor-initiative#77

This also causes `<Wrapper<?0> as Trait>::Unwrap: Trait` to always be ambig, as we now normalize the self type before checking whether it is an inference variable.

I cannot think of an approach to the underlying issues here which does not require the "may-define means must-define" restriction for opaque types. Going to go ahead with this and added this restriction to the tracking issue for the new solver to make sure we don't stabilize it without getting types + lang signoff here.

r? `@compiler-errors`
@lcnr lcnr closed this as completed Apr 23, 2024
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.

2 participants