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, still using the cache for things that can't possibly constrain opaque types (probably sound, famous last words), ..)

cc rust-lang#122192 (comment)

* [ ] check if this actually fixes rust-lang#105787 or if it just fixes the opaque type reproducer
  • Loading branch information
bors committed Jun 5, 2024
2 parents db8aca4 + 319843a commit 56d23c2
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 12 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ use rustc_errors::ErrorGuaranteed;

use crate::ty;

use rustc_hir::def_id::DefId;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_macros::{HashStable, TypeVisitable};
use rustc_query_system::cache::Cache;

pub type SelectionCache<'tcx> = Cache<
// This cache does not use `ParamEnvAnd` in its keys because `ParamEnv::and` can replace
// caller bounds with an empty list if the `TraitPredicate` looks global, which may happen
// after erasing lifetimes from the predicate.
(ty::ParamEnv<'tcx>, ty::TraitPredicate<'tcx>),
(&'tcx ty::List<LocalDefId>, ty::ParamEnv<'tcx>, ty::TraitPredicate<'tcx>),
SelectionResult<'tcx, SelectionCandidate<'tcx>>,
>;

Expand Down
11 changes: 7 additions & 4 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 @@ -1559,13 +1560,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
let tcx = self.tcx();
let pred = cache_fresh_trait_pred.skip_binder();
let opaques = self.infcx.defining_opaque_types();

if self.can_use_global_caches(param_env) {
if let Some(res) = tcx.selection_cache.get(&(param_env, pred), tcx) {
if let Some(res) = tcx.selection_cache.get(&(opaques, param_env, pred), tcx) {
return Some(res);
}
}
self.infcx.selection_cache.get(&(param_env, pred), tcx)
self.infcx.selection_cache.get(&(opaques, param_env, pred), tcx)
}

/// Determines whether can we safely cache the result
Expand Down Expand Up @@ -1611,6 +1613,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
) {
let tcx = self.tcx();
let pred = cache_fresh_trait_pred.skip_binder();
let opaques = self.infcx.defining_opaque_types();

if !self.can_cache_candidate(&candidate) {
debug!(?pred, ?candidate, "insert_candidate_cache - candidate is not cacheable");
Expand All @@ -1624,14 +1627,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
if !candidate.has_infer() {
debug!(?pred, ?candidate, "insert_candidate_cache global");
// This may overwrite the cache with the same value.
tcx.selection_cache.insert((param_env, pred), dep_node, candidate);
tcx.selection_cache.insert((opaques, param_env, pred), dep_node, candidate);
return;
}
}
}

debug!(?pred, ?candidate, "insert_candidate_cache local");
self.infcx.selection_cache.insert((param_env, pred), dep_node, candidate);
self.infcx.selection_cache.insert((opaques, param_env, pred), dep_node, candidate);
}

/// Looks at the item bounds of the projection or opaque type.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//@ known-bug: #119272
//! used to ICE: #119272
#![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> {
//~^ ERROR: type annotations needed
x
}
}
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/auto-traits/opaque_type_candidate_selection.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0284]: type annotations needed
--> $DIR/opaque_type_candidate_selection.rs:7: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/opaque_type_candidate_selection.rs:13: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: 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

0 comments on commit 56d23c2

Please sign in to comment.