Skip to content
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.

Sign up for GitHub

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

don't succeed evaluate_obligation query if new opaque types were registered #98614

Merged
merged 6 commits into from
Jul 8, 2022
Merged

don't succeed evaluate_obligation query if new opaque types were registered #98614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9dbfcbc
pessimistically treat all function items as containing an opaque type
oli-obk Jun 29, 2022
ade2a96
use a method instead of manually doing what its body does
oli-obk Jun 30, 2022
84fc551
Make `evaluate_obligation` not succeed unconditionally if it register…
oli-obk Jun 30, 2022
58c08cd
Remove type flag based opaque type workaround
oli-obk Jul 1, 2022
0b863e0
not knowing about opaque types is worse than not knowing about region…
oli-obk Jul 7, 2022
d6b93eb
Only register hidden types for opaque types from the current crate, n…
oli-obk Jul 8, 2022
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
4 changes: 4 additions & 0 deletions compiler/rustc_infer/src/infer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
.region_constraints_added_in_snapshot(&snapshot.undo_snapshot)
}

pub fn opaque_types_added_in_snapshot(&self, snapshot: &CombinedSnapshot<'a, 'tcx>) -> bool {
self.inner.borrow().undo_log.opaque_types_in_snapshot(&snapshot.undo_snapshot)
}

pub fn add_given(&self, sub: ty::Region<'tcx>, sup: ty::RegionVid) {
self.inner.borrow_mut().unwrap_region_constraints().add_given(sub, sup);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/opaque_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
let (a, b) = if a_is_expected { (a, b) } else { (b, a) };
let process = |a: Ty<'tcx>, b: Ty<'tcx>| match *a.kind() {
ty::Opaque(def_id, substs) => {
ty::Opaque(def_id, substs) if def_id.is_local() => {
let origin = if self.defining_use_anchor.is_some() {
// Check that this is `impl Trait` type is
// declared by `parent_def_id` -- i.e., one whose
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_infer/src/infer/undo_log.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ impl<'tcx> InferCtxtUndoLogs<'tcx> {
})
}

pub(crate) fn opaque_types_in_snapshot(&self, s: &Snapshot<'tcx>) -> bool {
self.logs[s.undo_len..].iter().any(|log| matches!(log, UndoLog::OpaqueTypes(..)))
}

pub(crate) fn region_constraints(
&self,
) -> impl Iterator<Item = &'_ region_constraints::UndoLog<'tcx>> + Clone {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/traits/project.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ impl<'tcx> ProjectionCache<'_, 'tcx> {
Some(&ProjectionCacheEntry::NormalizedTy { ref ty, complete: _ }) => {
info!("ProjectionCacheEntry::complete({:?}) - completing {:?}", key, ty);
let mut ty = ty.clone();
if result == EvaluationResult::EvaluatedToOk {
if result.must_apply_considering_regions() {
ty.obligations = vec![];
}
map.insert(key, ProjectionCacheEntry::NormalizedTy { ty, complete: Some(result) });
Expand Down
18 changes: 14 additions & 4 deletions compiler/rustc_middle/src/traits/select.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ pub enum EvaluationResult {
EvaluatedToOk,
/// Evaluation successful, but there were unevaluated region obligations.
EvaluatedToOkModuloRegions,
/// Evaluation successful, but need to rerun because opaque types got
/// hidden types assigned without it being known whether the opaque types
/// are within their defining scope
EvaluatedToOkModuloOpaqueTypes,
Copy link
Member

Choose a reason for hiding this comment

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

Is must_apply_modulo_regions()'s implementation still correct? It looks like that will return true for EvaluatedToOkModuloOpaqueTypes; is that valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, it's not, but I also wasn't able to create a test that actually was a problem. Just to be safe, I moved the opaque types down

/// Evaluation is known to be ambiguous -- it *might* hold for some
/// assignment of inference variables, but it might not.
///
Expand Down Expand Up @@ -252,9 +256,11 @@ impl EvaluationResult {

pub fn may_apply(self) -> bool {
match self {
EvaluatedToOk | EvaluatedToOkModuloRegions | EvaluatedToAmbig | EvaluatedToUnknown => {
true
}
EvaluatedToOkModuloOpaqueTypes
| EvaluatedToOk
| EvaluatedToOkModuloRegions
| EvaluatedToAmbig
| EvaluatedToUnknown => true,

EvaluatedToErr | EvaluatedToRecur => false,
}
Expand All @@ -264,7 +270,11 @@ impl EvaluationResult {
match self {
EvaluatedToUnknown | EvaluatedToRecur => true,

EvaluatedToOk | EvaluatedToOkModuloRegions | EvaluatedToAmbig | EvaluatedToErr => false,
EvaluatedToOkModuloOpaqueTypes
| EvaluatedToOk
| EvaluatedToOkModuloRegions
| EvaluatedToAmbig
| EvaluatedToErr => false,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,7 @@ impl<'tcx> InstantiatedPredicates<'tcx> {
Lift
)]
pub struct OpaqueTypeKey<'tcx> {
// FIXME(oli-obk): make this a LocalDefId
pub def_id: DefId,
pub substs: SubstsRef<'tcx>,
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
Ok(
EvaluationResult::EvaluatedToOk
| EvaluationResult::EvaluatedToOkModuloRegions
| EvaluationResult::EvaluatedToOkModuloOpaqueTypes
| EvaluationResult::EvaluatedToAmbig,
) => {}
_ => return false,
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
Err(_) => return Ok(EvaluatedToErr),
}

if self.infcx.opaque_types_added_in_snapshot(snapshot) {
return Ok(result.max(EvaluatedToOkModuloOpaqueTypes));
}

match self.infcx.region_constraints_added_in_snapshot(snapshot) {
None => Ok(result),
Some(_) => Ok(result.max(EvaluatedToOkModuloRegions)),
Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_type_ir/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,6 @@ bitflags! {
| TypeFlags::HAS_CT_INFER.bits
| TypeFlags::HAS_TY_PLACEHOLDER.bits
| TypeFlags::HAS_CT_PLACEHOLDER.bits
// The `evaluate_obligation` query does not return further
// obligations. If it evaluates an obligation with an opaque
// type, that opaque type may get compared to another type,
// constraining it. We would lose this information.
// FIXME: differentiate between crate-local opaque types
// and opaque types from other crates, as only opaque types
// from the local crate can possibly be a local name
| TypeFlags::HAS_TY_OPAQUE.bits
// We consider 'freshened' types and constants
// to depend on a particular fn.
// The freshening process throws away information,
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/impl-trait/auto-trait-leak.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ fn main() {
// return type, which can't depend on the obligation.
fn cycle1() -> impl Clone {
//~^ ERROR cycle detected
//~| ERROR cycle detected
send(cycle2().clone());

Rc::new(Cell::new(5))
Expand Down
112 changes: 15 additions & 97 deletions src/test/ui/impl-trait/auto-trait-leak.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,129 +30,47 @@ note: ...which requires building MIR for `cycle1`...
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `cycle1`...
--> $DIR/auto-trait-leak.rs:12:1
|
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires computing type of `cycle2::{opaque#0}`...
--> $DIR/auto-trait-leak.rs:20:16
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^
note: ...which requires borrow-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires unsafety-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which again requires computing type of `cycle1::{opaque#0}`, completing the cycle
note: cycle used when checking item types in top-level module
--> $DIR/auto-trait-leak.rs:1:1
|
LL | / use std::cell::Cell;
LL | | use std::rc::Rc;
LL | |
LL | | fn send<T: Send>(_: T) {}
... |
LL | | Rc::new(String::from("foo"))
LL | | }
| |_^

error[E0391]: cycle detected when computing type of `cycle1::{opaque#0}`
--> $DIR/auto-trait-leak.rs:12:16
--> $DIR/auto-trait-leak.rs:14:5
|
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^
|
note: ...which requires borrow-checking `cycle1`...
--> $DIR/auto-trait-leak.rs:12:1
|
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing `cycle1`...
--> $DIR/auto-trait-leak.rs:12:1
|
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing MIR for `cycle1`...
--> $DIR/auto-trait-leak.rs:12:1
|
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires unsafety-checking `cycle1`...
--> $DIR/auto-trait-leak.rs:12:1
|
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `cycle1`...
--> $DIR/auto-trait-leak.rs:12:1
|
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `cycle1`...
--> $DIR/auto-trait-leak.rs:12:1
|
LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
LL | send(cycle2().clone());
| ^^^^
= note: ...which requires evaluating trait selection obligation `impl core::clone::Clone: core::marker::Send`...
note: ...which requires computing type of `cycle2::{opaque#0}`...
--> $DIR/auto-trait-leak.rs:20:16
--> $DIR/auto-trait-leak.rs:19:16
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^
note: ...which requires borrow-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
--> $DIR/auto-trait-leak.rs:19:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
--> $DIR/auto-trait-leak.rs:19:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
--> $DIR/auto-trait-leak.rs:19:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires unsafety-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
--> $DIR/auto-trait-leak.rs:19:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
--> $DIR/auto-trait-leak.rs:19:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:20:1
--> $DIR/auto-trait-leak.rs:20:5
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
LL | send(cycle1().clone());
| ^^^^
= note: ...which requires evaluating trait selection obligation `impl core::clone::Clone: core::marker::Send`...
= note: ...which again requires computing type of `cycle1::{opaque#0}`, completing the cycle
note: cycle used when checking item types in top-level module
--> $DIR/auto-trait-leak.rs:1:1
Expand All @@ -166,6 +84,6 @@ LL | | Rc::new(String::from("foo"))
LL | | }
| |_^

error: aborting due to 2 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
1 change: 0 additions & 1 deletion src/test/ui/type-alias-impl-trait/auto-trait-leakage3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
mod m {
type Foo = impl std::fmt::Debug;
//~^ ERROR: cycle detected when computing type of `m::Foo::{opaque#0}` [E0391]
//~| ERROR: cycle detected when computing type of `m::Foo::{opaque#0}` [E0391]

pub fn foo() -> Foo {
22_u32
Expand Down
27 changes: 5 additions & 22 deletions src/test/ui/type-alias-impl-trait/auto-trait-leakage3.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,18 @@ LL | type Foo = impl std::fmt::Debug;
| ^^^^^^^^^^^^^^^^^^^^
|
note: ...which requires type-checking `m::bar`...
--> $DIR/auto-trait-leakage3.rs:15:5
--> $DIR/auto-trait-leakage3.rs:15:9
|
LL | pub fn bar() {
| ^^^^^^^^^^^^
LL | is_send(foo());
| ^^^^^^^
= note: ...which requires evaluating trait selection obligation `m::Foo: core::marker::Send`...
= note: ...which again requires computing type of `m::Foo::{opaque#0}`, completing the cycle
note: cycle used when checking item types in module `m`
--> $DIR/auto-trait-leakage3.rs:6:1
|
LL | mod m {
| ^^^^^

error[E0391]: cycle detected when computing type of `m::Foo::{opaque#0}`
--> $DIR/auto-trait-leakage3.rs:7:16
|
LL | type Foo = impl std::fmt::Debug;
| ^^^^^^^^^^^^^^^^^^^^
|
note: ...which requires type-checking `m::bar`...
--> $DIR/auto-trait-leakage3.rs:15:5
|
LL | pub fn bar() {
| ^^^^^^^^^^^^
= note: ...which again requires computing type of `m::Foo::{opaque#0}`, completing the cycle
note: cycle used when checking item types in module `m`
--> $DIR/auto-trait-leakage3.rs:6:1
|
LL | mod m {
| ^^^^^

error: aborting due to 2 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
1 change: 0 additions & 1 deletion src/test/ui/type-alias-impl-trait/inference-cycle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
mod m {
type Foo = impl std::fmt::Debug;
//~^ ERROR cycle detected
//~| ERROR cycle detected

// Cycle: error today, but it'd be nice if it eventually worked

Expand Down
27 changes: 5 additions & 22 deletions src/test/ui/type-alias-impl-trait/inference-cycle.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,18 @@ LL | type Foo = impl std::fmt::Debug;
| ^^^^^^^^^^^^^^^^^^^^
|
note: ...which requires type-checking `m::bar`...
--> $DIR/inference-cycle.rs:15:5
--> $DIR/inference-cycle.rs:15:9
|
LL | pub fn bar() {
| ^^^^^^^^^^^^
LL | is_send(foo()); // Today: error
| ^^^^^^^
= note: ...which requires evaluating trait selection obligation `m::Foo: core::marker::Send`...
= note: ...which again requires computing type of `m::Foo::{opaque#0}`, completing the cycle
note: cycle used when checking item types in module `m`
--> $DIR/inference-cycle.rs:4:1
|
LL | mod m {
| ^^^^^

error[E0391]: cycle detected when computing type of `m::Foo::{opaque#0}`
--> $DIR/inference-cycle.rs:5:16
|
LL | type Foo = impl std::fmt::Debug;
| ^^^^^^^^^^^^^^^^^^^^
|
note: ...which requires type-checking `m::bar`...
--> $DIR/inference-cycle.rs:15:5
|
LL | pub fn bar() {
| ^^^^^^^^^^^^
= note: ...which again requires computing type of `m::Foo::{opaque#0}`, completing the cycle
note: cycle used when checking item types in module `m`
--> $DIR/inference-cycle.rs:4:1
|
LL | mod m {
| ^^^^^

error: aborting due to 2 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
13 changes: 13 additions & 0 deletions src/test/ui/type-alias-impl-trait/issue-98604.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// edition:2018

type AsyncFnPtr = Box<
dyn Fn() -> std::pin::Pin<Box<dyn std::future::Future<Output = ()>>>,
>;

async fn test() {}

#[allow(unused_must_use)]
fn main() {
Box::new(test) as AsyncFnPtr;
//~^ ERROR type mismatch
}
Loading