Skip to content

Commit

Permalink
Auto merge of #80246 - matthewjasper:projection-cycle-caching, r=Mark…
Browse files Browse the repository at this point in the history
…-Simulacrum

Prevent caching normalization results with a cycle

When normalizing a projection which results in a cycle, we would cache the result of `project_type` without the nested obligations (because they're not needed for inference). This would result in the nested obligations only being handled once in fulfill, which would avoid the cycle error. `get_paranoid_cache_value_obligation` used to add an obligation that resulted in a cycle in this case previously, but was removed by #73905.

This PR makes the projection cache not cache the value of a projection if it was ever normalized in a cycle (except in a snapshot that's rolled back).

Fixes #79714.

r? `@nikomatsakis`
  • Loading branch information
bors committed Dec 26, 2020
2 parents 1d2730f + 2e92b13 commit 931aa27
Show file tree
Hide file tree
Showing 12 changed files with 159 additions and 24 deletions.
16 changes: 15 additions & 1 deletion compiler/rustc_infer/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ impl ProjectionCacheKey<'tcx> {
pub enum ProjectionCacheEntry<'tcx> {
InProgress,
Ambiguous,
Recur,
Error,
NormalizedTy(NormalizedTy<'tcx>),
}
Expand Down Expand Up @@ -143,7 +144,12 @@ impl<'tcx> ProjectionCache<'_, 'tcx> {
"ProjectionCacheEntry::insert_ty: adding cache entry: key={:?}, value={:?}",
key, value
);
let fresh_key = self.map().insert(key, ProjectionCacheEntry::NormalizedTy(value));
let mut map = self.map();
if let Some(ProjectionCacheEntry::Recur) = map.get(&key) {
debug!("Not overwriting Recur");
return;
}
let fresh_key = map.insert(key, ProjectionCacheEntry::NormalizedTy(value));
assert!(!fresh_key, "never started projecting `{:?}`", key);
}

Expand Down Expand Up @@ -197,6 +203,14 @@ impl<'tcx> ProjectionCache<'_, 'tcx> {
assert!(!fresh, "never started projecting `{:?}`", key);
}

/// Indicates that while trying to normalize `key`, `key` was required to
/// be normalized again. Selection or evaluation should eventually report
/// an error here.
pub fn recur(&mut self, key: ProjectionCacheKey<'tcx>) {
let fresh = self.map().insert(key, ProjectionCacheEntry::Recur);
assert!(!fresh, "never started projecting `{:?}`", key);
}

/// Indicates that trying to normalize `key` resulted in
/// error.
pub fn error(&mut self, key: ProjectionCacheKey<'tcx>) {
Expand Down
23 changes: 16 additions & 7 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,12 +496,6 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
return Ok(None);
}
Err(ProjectionCacheEntry::InProgress) => {
// If while normalized A::B, we are asked to normalize
// A::B, just return A::B itself. This is a conservative
// answer, in the sense that A::B *is* clearly equivalent
// to A::B, though there may be a better value we can
// find.

// Under lazy normalization, this can arise when
// bootstrapping. That is, imagine an environment with a
// where-clause like `A::B == u32`. Now, if we are asked
Expand All @@ -512,6 +506,14 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(

debug!("found cache entry: in-progress");

// Cache that normalizing this projection resulted in a cycle. This
// should ensure that, unless this happens within a snapshot that's
// rolled back, fulfillment or evaluation will notice the cycle.

infcx.inner.borrow_mut().projection_cache().recur(cache_key);
return Err(InProgress);
}
Err(ProjectionCacheEntry::Recur) => {
return Err(InProgress);
}
Err(ProjectionCacheEntry::NormalizedTy(ty)) => {
Expand Down Expand Up @@ -734,7 +736,14 @@ fn project_type<'cx, 'tcx>(

if !selcx.tcx().sess.recursion_limit().value_within_limit(obligation.recursion_depth) {
debug!("project: overflow!");
return Err(ProjectionTyError::TraitSelectionError(SelectionError::Overflow));
match selcx.query_mode() {
super::TraitQueryMode::Standard => {
selcx.infcx().report_overflow_error(&obligation, true);
}
super::TraitQueryMode::Canonical => {
return Err(ProjectionTyError::TraitSelectionError(SelectionError::Overflow));
}
}
}

let obligation_trait_ref = &obligation.predicate.trait_ref(selcx.tcx());
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self.infcx.tcx
}

pub(super) fn query_mode(&self) -> TraitQueryMode {
self.query_mode
}

///////////////////////////////////////////////////////////////////////////
// Selection
//
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/associated-types/defaults-cyclic-fail-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ impl Tr for u32 {
// ...but not in an impl that redefines one of the types.
impl Tr for bool {
type A = Box<Self::B>;
//~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
//~^ ERROR overflow evaluating the requirement `<bool as Tr>::B == _`
}
// (the error is shown twice for some reason)

impl Tr for usize {
type B = &'static Self::A;
//~^ ERROR type mismatch resolving `<usize as Tr>::A == _`
//~^ ERROR overflow evaluating the requirement `<usize as Tr>::A == _`
}

fn main() {
Expand Down
10 changes: 5 additions & 5 deletions src/test/ui/associated-types/defaults-cyclic-fail-1.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
error[E0271]: type mismatch resolving `<bool as Tr>::B == _`
error[E0275]: overflow evaluating the requirement `<bool as Tr>::B == _`
--> $DIR/defaults-cyclic-fail-1.rs:26:5
|
LL | type A = Box<Self::B>;
| ^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size
| ^^^^^^^^^^^^^^^^^^^^^^

error[E0271]: type mismatch resolving `<usize as Tr>::A == _`
error[E0275]: overflow evaluating the requirement `<usize as Tr>::A == _`
--> $DIR/defaults-cyclic-fail-1.rs:32:5
|
LL | type B = &'static Self::A;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0271`.
For more information about this error, try `rustc --explain E0275`.
4 changes: 2 additions & 2 deletions src/test/ui/associated-types/defaults-cyclic-fail-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ impl Tr for u32 {

impl Tr for bool {
type A = Box<Self::B>;
//~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
//~^ ERROR overflow evaluating the requirement `<bool as Tr>::B == _`
}
// (the error is shown twice for some reason)

impl Tr for usize {
type B = &'static Self::A;
//~^ ERROR type mismatch resolving `<usize as Tr>::A == _`
//~^ ERROR overflow evaluating the requirement `<usize as Tr>::A == _`
}

fn main() {
Expand Down
10 changes: 5 additions & 5 deletions src/test/ui/associated-types/defaults-cyclic-fail-2.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
error[E0271]: type mismatch resolving `<bool as Tr>::B == _`
error[E0275]: overflow evaluating the requirement `<bool as Tr>::B == _`
--> $DIR/defaults-cyclic-fail-2.rs:27:5
|
LL | type A = Box<Self::B>;
| ^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size
| ^^^^^^^^^^^^^^^^^^^^^^

error[E0271]: type mismatch resolving `<usize as Tr>::A == _`
error[E0275]: overflow evaluating the requirement `<usize as Tr>::A == _`
--> $DIR/defaults-cyclic-fail-2.rs:33:5
|
LL | type B = &'static Self::A;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0271`.
For more information about this error, try `rustc --explain E0275`.
29 changes: 29 additions & 0 deletions src/test/ui/associated-types/impl-wf-cycle-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Regression test for #79714

trait Baz {}
impl Baz for () {}
impl<T> Baz for (T,) {}

trait Fiz {}
impl Fiz for bool {}

trait Grault {
type A;
type B;
}

impl<T: Grault> Grault for (T,)
where
Self::A: Baz,
Self::B: Fiz,
{
type A = ();
//~^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _`
type B = bool;
//~^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _`
}
//~^^^^^^^^^^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _`

fn main() {
let x: <(_,) as Grault>::A = ();
}
39 changes: 39 additions & 0 deletions src/test/ui/associated-types/impl-wf-cycle-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
--> $DIR/impl-wf-cycle-1.rs:15:1
|
LL | / impl<T: Grault> Grault for (T,)
LL | | where
LL | | Self::A: Baz,
LL | | Self::B: Fiz,
... |
LL | |
LL | | }
| |_^
|
= note: required because of the requirements on the impl of `Grault` for `(T,)`
= note: 1 redundant requirements hidden
= note: required because of the requirements on the impl of `Grault` for `(T,)`

error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
--> $DIR/impl-wf-cycle-1.rs:20:5
|
LL | type A = ();
| ^^^^^^^^^^^^
|
= note: required because of the requirements on the impl of `Grault` for `(T,)`
= note: 1 redundant requirements hidden
= note: required because of the requirements on the impl of `Grault` for `(T,)`

error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
--> $DIR/impl-wf-cycle-1.rs:22:5
|
LL | type B = bool;
| ^^^^^^^^^^^^^^
|
= note: required because of the requirements on the impl of `Grault` for `(T,)`
= note: 1 redundant requirements hidden
= note: required because of the requirements on the impl of `Grault` for `(T,)`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0275`.
16 changes: 16 additions & 0 deletions src/test/ui/associated-types/impl-wf-cycle-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Regression test for #79714

trait Grault {
type A;
}

impl<T: Grault> Grault for (T,)
where
Self::A: Copy,
{
type A = ();
//~^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _`
}
//~^^^^^^^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _`

fn main() {}
25 changes: 25 additions & 0 deletions src/test/ui/associated-types/impl-wf-cycle-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
--> $DIR/impl-wf-cycle-2.rs:7:1
|
LL | / impl<T: Grault> Grault for (T,)
LL | | where
LL | | Self::A: Copy,
LL | | {
LL | | type A = ();
LL | |
LL | | }
| |_^
|
= note: required because of the requirements on the impl of `Grault` for `(T,)`

error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
--> $DIR/impl-wf-cycle-2.rs:11:5
|
LL | type A = ();
| ^^^^^^^^^^^^
|
= note: required because of the requirements on the impl of `Grault` for `(T,)`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0275`.
3 changes: 1 addition & 2 deletions src/test/ui/issues/issue-23122-2.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
error[E0275]: overflow evaluating the requirement `<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<T as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next: Sized`
error[E0275]: overflow evaluating the requirement `<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<T as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next`
--> $DIR/issue-23122-2.rs:9:5
|
LL | type Next = <GetNext<T::Next> as Next>::Next;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`issue_23122_2`)
= note: required because of the requirements on the impl of `Next` for `GetNext<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<T as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next>`

error: aborting due to previous error

Expand Down

0 comments on commit 931aa27

Please sign in to comment.