Skip to content

opaque type leakage and RPITIT normalization #139788

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

Open
lcnr opened this issue Apr 14, 2025 · 3 comments
Open

opaque type leakage and RPITIT normalization #139788

lcnr opened this issue Apr 14, 2025 · 3 comments
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Comments

@lcnr
Copy link
Contributor

lcnr commented Apr 14, 2025

related to rust-lang/trait-system-refactor-initiative#173, see added tests in #139789

trait Trait {
    // desugars to
    // type Assoc: Sized + Send;
    // fn foo(b: bool) -> Self::Assoc;
    fn foo(b: bool) -> impl Sized + Send;
}

impl Trait for u32 {
    // desugars to
    // type Assoc = impl_rpit::<Self>;
    // fn foo(b: bool) -> Self::Assoc { .. }
    fn foo(b: bool) -> impl Sized {
        if b {
            u32::foo(false)
        } else {
            1u32
        }
    }
}

This currently results in a query cycle:

  • type_of(impl::Assoc)
  • collect_return_position_impl_trait_in_trait_tys
  • impl_rpit: Send
  • type_of(impl_rpit) // auto trait leakage
  • typeck(impl::foo)
  • normalize(<u32 as Trait>::Assoc)
  • type_of(impl::Assoc)

I believe that this query cycle should not be there and can be avoided.

collect_return_position_impl_trait_in_trait_tys currently adds the item bounds of the RPITIT when replacing it with fresh infer vars. I believe this is not necessary to guide inference as the method signature is fully concrete.

We could therefore split this in two:

  • collect_return_position_impl_trait_in_trait_tys instantiates RPITIT with infer vars but does not check the item bounds of the RPITIT trait assoc type
  • compare_type_predicate_entailment (or a separate query, idk and idc :3) then uses collect_return_position_impl_trait_in_trait_tys and actually checks the item bounds

This means normalizing impl::Assoc no longer has to prove the item bounds of the RPITIT, allowing the above example to compile and fixing rust-lang/trait-system-refactor-initiative#173

cc @compiler-errors

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 14, 2025
@lcnr lcnr added A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 14, 2025
@compiler-errors
Copy link
Member

I'm confused here: we use the item bounds not just to enforce that the hidden types implement their bounds, but also in order to collect the hidden type of inner RPITITs, for example, so we at least need to keep around projection bounds, unless I'm missing something from the write-up above.

I believe the example I use throughout the documentation to motivate this is something like an RPITIT impl Deref<Assoc = impl Sized> being equated against an impl signature of &String so that we infer impl Sized := String.

I'm not certain I see how this scheme manages to compute the type of the inner impl Sized -- can you elaborate?

@lcnr
Copy link
Contributor Author

lcnr commented Apr 14, 2025

I'm not certain I see how this scheme manages to compute the type of the inner impl Sized -- can you elaborate?

It does not '^^ I did not consider that we need this for nested RPITIT. We could filter the required item bounds to only prove bounds which may constrain nested RPITITs. But with this requirement changing the approach feels less obviously desirable to me 🤔 these projection bounds could still require leaking auto traits:

trait Project {
    type Assoc;
}
impl<T: Send> Project for Vec<T> {
    type Assoc = u32;
}

trait Trait {
    fn foo(b: bool) -> impl Project<Assoc = impl Sized>;
}
impl Trait for u32 {
    fn foo(b: bool) -> Vec<impl Sized> {
        if b {
            u32::foo(false)
        } else {
            vec![1u32]
        }
    }
}

otoh, we could add yet another TypingMode::ComputeSyntheticAssocItems which treats all impl Trait: AutoTrait bounds as trivially true (so it's unsound by itself) which we use to in collect_return_position_impl_trait_in_trait_tys and then recheck in TypingMode::Analysis proper while leaking auto traits as normal

@compiler-errors
Copy link
Member

otoh, we could add yet another TypingMode::ComputeSyntheticAssocItems which treats all impl Trait: AutoTrait bounds as trivially true (so it's unsound by itself) which we use to in collect_return_position_impl_trait_in_trait_tys and then recheck in TypingMode::Analysis proper while leaking auto traits as normal

Yes, though stylistically it feels a bit different than other candidate assembly special casing because it would need to happen before we try to normalize the self type of the trait bound.

I'm personally not very keen to fix this cycle, since to me it feels qualitiatively different than the one I'm trying to fix in rust-lang/trait-system-refactor-initiative#173, which can be justified much more simply by "rigid projections should never be projected in the first place".

Zalathar added a commit to Zalathar/rust that referenced this issue Apr 15, 2025
…=compiler-errors

do not unnecessarily leak auto traits in item bounds

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

Not a fix for rust-lang/trait-system-refactor-initiative#173 as you may have realized/tried yourself, cc rust-lang#139788. However, fixing this feels desirable regardless and I don't see any reason not to.

r? `@compiler-errors`
jieyouxu added a commit to jieyouxu/rust that referenced this issue Apr 15, 2025
…=compiler-errors

do not unnecessarily leak auto traits in item bounds

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

Not a fix for rust-lang/trait-system-refactor-initiative#173 as you may have realized/tried yourself, cc rust-lang#139788. However, fixing this feels desirable regardless and I don't see any reason not to.

r? ``@compiler-errors``
Zalathar added a commit to Zalathar/rust that referenced this issue Apr 15, 2025
…=compiler-errors

do not unnecessarily leak auto traits in item bounds

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

Not a fix for rust-lang/trait-system-refactor-initiative#173 as you may have realized/tried yourself, cc rust-lang#139788. However, fixing this feels desirable regardless and I don't see any reason not to.

r? ```@compiler-errors```
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 15, 2025
Rollup merge of rust-lang#139789 - lcnr:opaques-auto-trait-leakage, r=compiler-errors

do not unnecessarily leak auto traits in item bounds

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

Not a fix for rust-lang/trait-system-refactor-initiative#173 as you may have realized/tried yourself, cc rust-lang#139788. However, fixing this feels desirable regardless and I don't see any reason not to.

r? ```@compiler-errors```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

No branches or pull requests

3 participants