Skip to content

do not unnecessarily leak auto traits in item bounds #139789

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

Merged
merged 3 commits into from
Apr 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions compiler/rustc_next_trait_solver/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ where
ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution> {
let cx = ecx.cx();
if goal.predicate.polarity != ty::PredicatePolarity::Positive {
return Err(NoSolution);
}
Expand All @@ -174,20 +175,37 @@ where

// Only consider auto impls of unsafe traits when there are no unsafe
// fields.
if ecx.cx().trait_is_unsafe(goal.predicate.def_id())
if cx.trait_is_unsafe(goal.predicate.def_id())
&& goal.predicate.self_ty().has_unsafe_fields()
{
return Err(NoSolution);
}

// We only look into opaque types during analysis for opaque types
// outside of their defining scope. Doing so for opaques in the
// defining scope may require calling `typeck` on the same item we're
// currently type checking, which will result in a fatal cycle that
// ideally we want to avoid, since we can make progress on this goal
// via an alias bound or a locally-inferred hidden type instead.
// We leak the implemented auto traits of opaques outside of their defining scope.
// This depends on `typeck` of the defining scope of that opaque, which may result in
// fatal query cycles.
//
// We only get to this point if we're outside of the defining scope as we'd otherwise
// be able to normalize the opaque type. We may also cycle in case `typeck` of a defining
// scope relies on the current context, e.g. either because it also leaks auto trait
// bounds of opaques defined in the current context or by evaluating the current item.
//
// To avoid this we don't try to leak auto trait bounds if they can also be proven via
// item bounds of the opaque. These bounds are always applicable as auto traits must not
// have any generic parameters. They would also get preferred over the impl candidate
// when merging candidates anyways.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe find some place to also explicitly mention that this is sound b/c we later separately check that the hidden type implements the RPIT's bounds after opaque type inference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels like a more general question about alias bound candidates of opaques 🤔 so we'd probably have to put it there. To be totally honest, I am not confident that it is fully sound in the first place as long as checking well-formedness for opaque types happens in analysis mode xx

I've kept #109387 in my github notifications for multiple months now

//
// See tests/ui/impl-trait/auto-trait-leakage/avoid-query-cycle-via-item-bound.rs.
if let ty::Alias(ty::Opaque, opaque_ty) = goal.predicate.self_ty().kind() {
debug_assert!(ecx.opaque_type_is_rigid(opaque_ty.def_id));
for item_bound in cx.item_self_bounds(opaque_ty.def_id).skip_binder() {
if item_bound
.as_trait_clause()
.is_some_and(|b| b.def_id() == goal.predicate.def_id())
{
return Err(NoSolution);
}
}
}

ecx.probe_and_evaluate_goal_for_constituent_tys(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//@ check-pass
//@ revisions: current next
//@[next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)

// When proving auto trait bounds, make sure that we depend on auto trait
// leakage if we can also prove it via an item bound.
fn is_send<T: Send>(_: T) {}

fn direct() -> impl Send {
is_send(check(false)); // leaks auto traits, depends on `check`
1u16
}

trait Indir: Send {}
impl Indir for u32 {}
fn indir() -> impl Indir {
is_send(check(false)); // leaks auto traits, depends on `check`
1u32
}

fn check(b: bool) -> impl Sized {
if b {
// must not leak auto traits, as we otherwise get a query cycle.
is_send(direct());
is_send(indir());
}
1u64
}

fn main() {
check(true);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
error[E0391]: cycle detected when computing type of `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::{anon_assoc#0}`
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
|
note: ...which requires comparing an impl and trait method signature, inferring any hidden `impl Trait` types in the process...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
= note: ...which requires evaluating trait selection obligation `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo::{opaque#0}: core::marker::Send`...
note: ...which requires computing type of opaque `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo::{opaque#0}`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
note: ...which requires borrow-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires promoting constants in MIR for `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires checking if `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo` contains FFI-unwind calls...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires match-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which again requires computing type of `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::{anon_assoc#0}`, completing the cycle
note: cycle used when checking that `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>` is well-formed
--> $DIR/method-compatability-via-leakage-cycle.rs:17:1
|
LL | impl Trait for u32 {
| ^^^^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0391`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
error[E0391]: cycle detected when computing type of `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::{anon_assoc#0}`
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
|
note: ...which requires comparing an impl and trait method signature, inferring any hidden `impl Trait` types in the process...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires computing type of `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo::{opaque#0}`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
note: ...which requires computing type of opaque `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo::{opaque#0}`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
note: ...which requires borrow-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires promoting constants in MIR for `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires checking if `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo` contains FFI-unwind calls...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires match-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which again requires computing type of `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::{anon_assoc#0}`, completing the cycle
note: cycle used when checking that `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>` is well-formed
--> $DIR/method-compatability-via-leakage-cycle.rs:17:1
|
LL | impl Trait for u32 {
| ^^^^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error[E0391]: cycle detected when computing type of `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::{anon_assoc#0}`
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
|
note: ...which requires comparing an impl and trait method signature, inferring any hidden `impl Trait` types in the process...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires computing type of `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo::{opaque#0}`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
note: ...which requires computing type of opaque `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo::{opaque#0}`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:24
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^
note: ...which requires borrow-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires promoting constants in MIR for `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires checking if `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo` contains FFI-unwind calls...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires match-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::foo`...
--> $DIR/method-compatability-via-leakage-cycle.rs:21:5
|
LL | fn foo(b: bool) -> impl Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which again requires computing type of `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>::{anon_assoc#0}`, completing the cycle
note: cycle used when checking that `<impl at $DIR/method-compatability-via-leakage-cycle.rs:17:1: 17:19>` is well-formed
--> $DIR/method-compatability-via-leakage-cycle.rs:17:1
|
LL | impl Trait for u32 {
| ^^^^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0391`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//@ revisions: current next
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be marked as a known-bug.

//@[next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)
//@ known-bug: #139788

// Recursively using the trait method inside of an impl in case checking
// method compatability relies on opaque type leakage currently causes a
// cycle error.

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
}
}
}

fn main() {}
14 changes: 14 additions & 0 deletions tests/ui/impl-trait/in-trait/method-compatability-via-leakage.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ check-pass
//@ revisions: current next
//@[next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)

trait Trait {
fn foo() -> impl Sized + Send;
}

impl Trait for u32 {
fn foo() -> impl Sized {}
}

fn main() {}
3 changes: 3 additions & 0 deletions tests/ui/impl-trait/in-trait/refine-cycle.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//@ check-pass
//@ revisions: current next
//@[next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)

// Make sure that refinement checking doesn't cause a cycle in `Instance::resolve`
// which calls `compare_impl_item`.
Expand Down
Loading