Skip to content

Don't try to prove opaque type bounds twice on the same types #114094

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

Closed
wants to merge 3 commits into from
Closed
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
37 changes: 23 additions & 14 deletions compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ impl<'tcx> InferCtxt<'tcx> {
) -> InferResult<'tcx, ()> {
let mut obligations = Vec::new();

self.insert_hidden_type(
let already_had_hidden_ty = self.insert_hidden_type(
opaque_type_key,
&cause,
param_env,
Expand All @@ -517,14 +517,21 @@ impl<'tcx> InferCtxt<'tcx> {
&mut obligations,
)?;

self.add_item_bounds_for_hidden_type(
opaque_type_key.def_id.to_def_id(),
opaque_type_key.args,
cause,
param_env,
hidden_ty,
&mut obligations,
);
// If we already had a hidden type, that hidden type will
// have already gone through `add_item_bounds_for_hidden_type`.
// Since all the bounds must already hold for the previous hidden type
// and the new hidden type must be equal to the previous hidden type,
// we can conclude that the bounds must already hold.
if !already_had_hidden_ty {
self.add_item_bounds_for_hidden_type(
opaque_type_key.def_id.to_def_id(),
opaque_type_key.args,
cause,
param_env,
hidden_ty,
&mut obligations,
);
}

Ok(InferOk { value: (), obligations })
}
Expand All @@ -544,7 +551,7 @@ impl<'tcx> InferCtxt<'tcx> {
hidden_ty: Ty<'tcx>,
a_is_expected: bool,
obligations: &mut Vec<PredicateObligation<'tcx>>,
) -> Result<(), TypeError<'tcx>> {
) -> Result<bool, TypeError<'tcx>> {
// Ideally, we'd get the span where *this specific `ty` came
// from*, but right now we just use the span from the overall
// value being folded. In simple cases like `-> impl Foo`,
Expand All @@ -560,7 +567,8 @@ impl<'tcx> InferCtxt<'tcx> {
cause.clone(),
param_env,
ty::PredicateKind::Ambiguous,
))
));
Ok(false)
} else {
let prev = self
.inner
Expand All @@ -573,10 +581,11 @@ impl<'tcx> InferCtxt<'tcx> {
.eq_exp(DefineOpaqueTypes::Yes, a_is_expected, prev, hidden_ty)?
.obligations,
);
Ok(true)
} else {
Ok(false)
}
};

Ok(())
}
}

pub fn add_item_bounds_for_hidden_type(
Expand Down
23 changes: 17 additions & 6 deletions src/tools/clippy/tests/ui/new_ret_no_self_overflow.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
error[E0275]: overflow evaluating the requirement `<i32 as std::ops::Add>::Output == issue10041::X`
--> $DIR/new_ret_no_self_overflow.rs:20:25
error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self_overflow.rs:9:9
|
LL | pub fn new() -> X {
| ^
LL | / pub fn new() -> impl PartialOrd {
LL | | 0i32
LL | | }
| |_________^
|
= note: `-D clippy::new-ret-no-self` implied by `-D warnings`

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self_overflow.rs:20:9
|
LL | / pub fn new() -> X {
LL | | 0i32
LL | | }
| |_________^

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

For more information about this error, try `rustc --explain E0275`.
2 changes: 2 additions & 0 deletions tests/ui/async-await/suggest-missing-await.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ LL | take_u32(x.await)
error[E0308]: mismatched types
--> $DIR/suggest-missing-await.rs:22:5
|
LL | async fn suggest_await_in_async_fn_return() {
| - expected `()` because of return type
LL | dummy()
| ^^^^^^^ expected `()`, found future
|
Expand Down
13 changes: 10 additions & 3 deletions tests/ui/parser/expr-as-stmt.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,16 @@ error[E0308]: mismatched types
--> $DIR/expr-as-stmt.rs:64:7
|
LL | { foo() } || { true }
| ^^^^^- help: consider using a semicolon here: `;`
| |
| expected `()`, found `i32`
| ^^^^^ expected `()`, found `i32`
|
help: consider using a semicolon here
|
LL | { foo(); } || { true }
| +
help: you might have meant to return this value
|
LL | { return foo(); } || { true }
| ++++++ +

error[E0308]: mismatched types
--> $DIR/expr-as-stmt.rs:8:6
Expand Down
1 change: 0 additions & 1 deletion tests/ui/type-alias-impl-trait/issue-63279.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ fn c() -> Closure {
|| -> Closure { || () }
//~^ ERROR: mismatched types
//~| ERROR: mismatched types
//~| ERROR: expected a `FnOnce<()>` closure, found `()`
}

fn main() {}
14 changes: 4 additions & 10 deletions tests/ui/type-alias-impl-trait/issue-63279.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,6 @@ LL | fn c() -> Closure {
= help: the trait `FnOnce<()>` is not implemented for `()`
= note: wrap the `()` in a closure with no arguments: `|| { /* code */ }`

error[E0277]: expected a `FnOnce<()>` closure, found `()`
--> $DIR/issue-63279.rs:7:11
|
LL | || -> Closure { || () }
| ^^^^^^^ expected an `FnOnce<()>` closure, found `()`
|
= help: the trait `FnOnce<()>` is not implemented for `()`
= note: wrap the `()` in a closure with no arguments: `|| { /* code */ }`

error[E0308]: mismatched types
--> $DIR/issue-63279.rs:7:21
|
Expand All @@ -32,6 +23,9 @@ LL | || -> Closure { (|| ())() }
error[E0308]: mismatched types
--> $DIR/issue-63279.rs:7:5
|
LL | fn c() -> Closure {
| ------- expected `()` because of return type
LL |
LL | || -> Closure { || () }
| ^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found closure
|
Expand All @@ -42,7 +36,7 @@ help: use parentheses to call this closure
LL | (|| -> Closure { || () })()
| + +++

error: aborting due to 4 previous errors
error: aborting due to 3 previous errors

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: unconstrained opaque type
--> $DIR/self-referential-in-fn-trait.rs:13:16
|
LL | type Foo<'a> = impl Fn() -> Foo<'a>;
| ^^^^^^^^^^^^^^^^^^^^
|
= note: `Foo` must be used in combination with a concrete type within the same module

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: unconstrained opaque type
--> $DIR/self-referential-in-fn-trait.rs:13:16
|
LL | type Foo<'a> = impl Fn() -> Foo<'a>;
| ^^^^^^^^^^^^^^^^^^^^
|
= note: `Foo` must be used in combination with a concrete type within the same module

error: aborting due to previous error

25 changes: 25 additions & 0 deletions tests/ui/type-alias-impl-trait/self-referential-in-fn-trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//! This test checks that we do not
//! end up with an infinite recursion,
//! a cycle error or an overflow when
//! encountering an opaque type that has
//! an associated type that is just itself
//! again.

#![feature(type_alias_impl_trait)]
Copy link
Member

Choose a reason for hiding this comment

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

how does this behave on the new trait solver? I think the new trait solver does re-prove bounds even if it's unifying with an existing opaque:

ecx.eq(param_env, candidate_ty, ty)?;
ecx.add_item_bounds_for_hidden_type(
candidate_key.def_id.to_def_id(),
candidate_key.args,
param_env,
candidate_ty,
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it works (see new commit), but I haven't run any logs yet to see what it is doing, and whether anything is different.

Copy link
Member

Choose a reason for hiding this comment

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

Wonder why it's not cycling. I bet it's because of lazy norm. Anyways, I guess it shouldn't matter, unless we make sure we don't commit to something that breaks the new solver.

// revisions: next old working
//[next] compile-flags: -Ztrait-solver=next
//[working] check-pass

type Foo<'a> = impl Fn() -> Foo<'a>;
//[old,next]~^ ERROR: unconstrained opaque type

fn crash<'a>(_: &'a (), x: Foo<'a>) -> Foo<'a> {
x
}

#[cfg(working)]
fn foo<'a>() -> Foo<'a> {
foo
}

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@ error[E0308]: mismatched types
--> $DIR/issue-90027-async-fn-return-suggestion.rs:9:5
|
LL | async fn world() -> () {
| -- expected `()` because of return type
| --
| |
| expected `()` because of return type
| expected `()` because of return type
LL | 0
| ^ expected `()`, found integer

error[E0308]: mismatched types
--> $DIR/issue-90027-async-fn-return-suggestion.rs:14:5
|
LL | async fn suggest_await_in_async_fn_return() {
| - expected `()` because of return type
LL | hello()
| ^^^^^^^ expected `()`, found future
|
Expand Down