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

discard_impls_shadowed_by_env forces unnecessary ambiguity #96

Closed
lcnr opened this issue Feb 22, 2024 · 1 comment · Fixed by rust-lang/rust#133643
Closed

discard_impls_shadowed_by_env forces unnecessary ambiguity #96

lcnr opened this issue Feb 22, 2024 · 1 comment · Fixed by rust-lang/rust#133643

Comments

@lcnr
Copy link
Contributor

lcnr commented Feb 22, 2024

the following works with the old solver but breaks with new:

trait Outer {
    type Assoc;
}

trait Id<U: ?Sized> {
    type Id;
}
impl<T, U: ?Sized + Copy> Id<U> for T {
    type Id = T;
}

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

fn create<T: Outer, U: Copy>(
    u: U,
) -> <<T as Id<U>>::Id as Outer>::Assoc {
    todo!()
}

fn foo<T: Outer<Assoc = i32>>() {
    let u = unconstrained();
    let assoc = create::<T, _>(u);
    assoc.abs();
    let _: i32 = u;
}

The type of assoc is <<T as Id<?u>>::Id as Outer>::Assoc and we have a T: Outer<Assoc = i32> where bound. <T as Id<?u>>::Id uses the impl candidate and normalizes to T, doing so tries to prove U: Copy, resulting in Certainty::Ambiguity. We now use the ParamEnv candidate for <<T as Id<?u>>::Id as Outer>::Assoc (<T as Outer>::Assoc after normalizing the self type). In discard_impls_shadowed_by_env there exists an ambiguous ParamEnv trait candidate, causing us to discard all constraints.

We should somehow only limit "discard all constraints" to the case where there are multiple ParamEnv candidates and we're unable to choose, not if we have an applicable ParamEnv candidate which is ambiguous.

@lcnr
Copy link
Contributor Author

lcnr commented Dec 18, 2024

fixed by rust-lang/rust#133643

@lcnr lcnr closed this as completed Dec 18, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 18, 2024
…errors

-Znext-solver: modify candidate preference rules

This implements the design proposed in the FCP in rust-lang#132325 and matches the old solver behavior. I hope the inline comments are all sufficiently clear, I personally think this is a fairly clear improvement over the existing approach using `fn discard_impls_shadowed_by_env`. This fixes rust-lang/trait-system-refactor-initiative#96.

This also fixes rust-lang#133639 which encounters an ICE in negative coherence when evaluating the where-clause. Given the features required to trigger this ICE 🤷

r? `@compiler-errors`
jhpratt added a commit to jhpratt/rust that referenced this issue Dec 19, 2024
…errors

-Znext-solver: modify candidate preference rules

This implements the design proposed in the FCP in rust-lang#132325 and matches the old solver behavior. I hope the inline comments are all sufficiently clear, I personally think this is a fairly clear improvement over the existing approach using `fn discard_impls_shadowed_by_env`. This fixes rust-lang/trait-system-refactor-initiative#96.

This also fixes rust-lang#133639 which encounters an ICE in negative coherence when evaluating the where-clause. Given the features required to trigger this ICE 🤷

r? ``@compiler-errors``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 19, 2024
Rollup merge of rust-lang#133643 - lcnr:merge-candidates, r=compiler-errors

-Znext-solver: modify candidate preference rules

This implements the design proposed in the FCP in rust-lang#132325 and matches the old solver behavior. I hope the inline comments are all sufficiently clear, I personally think this is a fairly clear improvement over the existing approach using `fn discard_impls_shadowed_by_env`. This fixes rust-lang/trait-system-refactor-initiative#96.

This also fixes rust-lang#133639 which encounters an ICE in negative coherence when evaluating the where-clause. Given the features required to trigger this ICE 🤷

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