Skip to content

Commit

Permalink
Auto merge of rust-lang#126024 - oli-obk:candidate_key_caching_is_uns…
Browse files Browse the repository at this point in the history
…ound_yay, r=<try>

Do not use global cache for selection candidates if opaque types can be constrained

fixes rust-lang#105787

r? `@ghost`

This is certainly the crudest way to make the cache sound wrt opaque types, but if perf lets us get away with this, let's do it in the old solver and let the new solver fix this correctly once and for all.

If perf is prohibitively bad, I'll look into alternatives (using canonical queries, checking whether any opaque types got constrained or whether decisions based on the availability of opaque types were made, ..)

cc rust-lang#122192 (comment)
  • Loading branch information
bors committed Jun 5, 2024
2 parents a330e49 + 0714168 commit 22162a8
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 9 deletions.
13 changes: 13 additions & 0 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use rustc_middle::ty::{self, PolyProjectionPredicate, Upcast};
use rustc_middle::ty::{Ty, TyCtxt, TypeFoldable, TypeVisitableExt};
use rustc_span::symbol::sym;
use rustc_span::Symbol;
use rustc_type_ir::InferCtxtLike as _;

use std::cell::{Cell, RefCell};
use std::cmp;
Expand Down Expand Up @@ -1557,6 +1558,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
if self.is_intercrate() {
return None;
}

// Opaque types are not part of the global cache key, so we'd be caching and loading
// values, even though we don't have all the information that went into them.
if !self.infcx.defining_opaque_types().is_empty() {
return None;
}

let tcx = self.tcx();
let pred = cache_fresh_trait_pred.skip_binder();

Expand Down Expand Up @@ -1595,6 +1603,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
if self.is_intercrate() {
return false;
}
// Opaque types are not part of the global cache key, so we'd be caching and loading
// values, even though we don't have all the information that went into them.
if !self.infcx.defining_opaque_types().is_empty() {
return false;
}
match result {
Ok(Some(SelectionCandidate::ParamCandidate(trait_ref))) => !trait_ref.has_infer(),
_ => true,
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/coherence/occurs-check/opaques.next.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0119]: conflicting implementations of trait `Trait<_>`
--> $DIR/opaques.rs:30:1
--> $DIR/opaques.rs:28:1
|
LL | impl<T> Trait<T> for T {
| ---------------------- first implementation here
Expand All @@ -8,7 +8,7 @@ LL | impl<T> Trait<T> for defining_scope::Alias<T> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation

error[E0282]: type annotations needed
--> $DIR/opaques.rs:13:20
--> $DIR/opaques.rs:11:20
|
LL | pub fn cast<T>(x: Container<Alias<T>, T>) -> Container<T, T> {
| ^ cannot infer type
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/coherence/occurs-check/opaques.old.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0284]: type annotations needed
--> $DIR/opaques.rs:11:20
|
LL | pub fn cast<T>(x: Container<Alias<T>, T>) -> Container<T, T> {
| ^ cannot infer type
|
= note: cannot satisfy `<Alias<T> as Trait<T>>::Assoc == _`
note: required because it appears within the type `Container<Alias<T>, T>`
--> $DIR/opaques.rs:17:8
|
LL | struct Container<T: Trait<U>, U> {
| ^^^^^^^^^
= help: unsized fn params are gated as an unstable feature
help: function arguments must have a statically known size, borrowed types always have a known size
|
LL | pub fn cast<T>(x: &Container<Alias<T>, T>) -> Container<T, T> {
| +

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0284`.
4 changes: 1 addition & 3 deletions tests/ui/coherence/occurs-check/opaques.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@

// A regression test for #105787

//@[old] known-bug: #105787
//@[old] check-pass
#![feature(type_alias_impl_trait)]
mod defining_scope {
use super::*;
pub type Alias<T> = impl Sized;

pub fn cast<T>(x: Container<Alias<T>, T>) -> Container<T, T> {
//[next]~^ ERROR type annotations needed
//~^ ERROR type annotations needed
x
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/impl-trait/equality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn sum_to(n: u32) -> impl Foo {
0
} else {
n + sum_to(n - 1)
//~^ ERROR cannot satisfy `<u32 as Add<impl Foo>>::Output == i32`
//~^ ERROR cannot satisfy `<u32 as Add<impl Foo>>::Output == u32`
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/impl-trait/equality.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ help: change the type of the numeric literal from `u32` to `i32`
LL | 0_i32
| ~~~

error[E0284]: type annotations needed: cannot satisfy `<u32 as Add<impl Foo>>::Output == i32`
error[E0284]: type annotations needed: cannot satisfy `<u32 as Add<impl Foo>>::Output == u32`
--> $DIR/equality.rs:24:11
|
LL | n + sum_to(n - 1)
| ^ cannot satisfy `<u32 as Add<impl Foo>>::Output == i32`
| ^ cannot satisfy `<u32 as Add<impl Foo>>::Output == u32`

error: aborting due to 2 previous errors; 1 warning emitted

Expand Down
1 change: 1 addition & 0 deletions tests/ui/type-alias-impl-trait/escaping-bound-var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ trait Test<'a> {}

pub type Foo = impl for<'a> Trait<'a, Assoc = impl Test<'a>>;
//~^ ERROR `impl Trait` cannot capture higher-ranked lifetime from outer `impl Trait`
//~| ERROR unconstrained opaque type

impl Trait<'_> for () {
type Assoc = ();
Expand Down
10 changes: 9 additions & 1 deletion tests/ui/type-alias-impl-trait/escaping-bound-var.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ note: lifetime declared here
LL | pub type Foo = impl for<'a> Trait<'a, Assoc = impl Test<'a>>;
| ^^

error: aborting due to 1 previous error
error: unconstrained opaque type
--> $DIR/escaping-bound-var.rs:9:47
|
LL | pub type Foo = impl for<'a> Trait<'a, Assoc = impl Test<'a>>;
| ^^^^^^^^^^^^^
|
= note: `Foo` must be used in combination with a concrete type within the same module

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0657`.

0 comments on commit 22162a8

Please sign in to comment.