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 create impl candidates when obligation contains errors #73005

Merged
merged 3 commits into from
Jun 11, 2020
Merged

Don't create impl candidates when obligation contains errors #73005

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f7729d
Don't create impl candidates when obligation contains errors
Aaron1011 Jun 4, 2020
3295c26
Treat selection error as ambiguous when error type is present
Aaron1011 Jun 5, 2020
ae42c91
Fix typo
Aaron1011 Jun 5, 2020
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
10 changes: 10 additions & 0 deletions src/librustc_trait_selection/traits/select/candidate_assembly.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
) -> Result<(), SelectionError<'tcx>> {
debug!("assemble_candidates_from_impls(obligation={:?})", obligation);

// Essentially any user-written impl will match with an error type,
// so creating `ImplCandidates` isn't useful. However, we might
// end up finding a candidate elsewhere (e.g. a `BuiltinCandidate` for `Sized)
// This helps us avoid overflow: see issue #72839
// Since compilation is already guarnateed to fail, this is just
Aaron1011 marked this conversation as resolved.
Show resolved Hide resolved
// to try to show the 'nicest' possible errors to the user.
if obligation.references_error() {
return Ok(());
}

self.tcx().for_each_relevant_impl(
obligation.predicate.def_id(),
obligation.predicate.skip_binder().trait_ref.self_ty(),
Expand Down
8 changes: 8 additions & 0 deletions src/librustc_trait_selection/traits/select/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// who might care about this case, like coherence, should use
// that function).
if candidates.is_empty() {
// If there's an error type, 'downgrade' our result from
// `Err(Unimplemented)` to `Ok(None)`. This helps us avoid
// emitting additional spurious errors, since we're guaranteed
// to have emitted at least one.
if stack.obligation.references_error() {
debug!("no results for error type, treating as ambiguous");
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a delay_span_bug, just in case.

Copy link
Member Author

Choose a reason for hiding this comment

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

With #70551, seeing an error type guarantees that delay_span_bug has been called.

Copy link
Contributor

Choose a reason for hiding this comment

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

r=me when CI is green then :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you want to wait for #70551, or can I r+ this now?

return Ok(None);
}
return Err(Unimplemented);
}

Expand Down
2 changes: 0 additions & 2 deletions 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,8 +11,6 @@ fn main() {
// return type, which can't depend on the obligation.
fn cycle1() -> impl Clone {
//~^ ERROR cycle detected
//~| ERROR cycle detected
//~| ERROR cycle detected
send(cycle2().clone());
//~^ ERROR cannot be sent between threads safely

Expand Down
188 changes: 9 additions & 179 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 @@ -36,37 +36,37 @@ LL | fn cycle1() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`...
note: ...which requires computing type of `cycle2::{{opaque}}#0`...
--> $DIR/auto-trait-leak.rs:22:16
--> $DIR/auto-trait-leak.rs:20:16
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^
note: ...which requires borrow-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires unsafety-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
--> $DIR/auto-trait-leak.rs:20:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -84,178 +84,8 @@ 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
|
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 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`...
note: ...which requires computing type of `cycle2::{{opaque}}#0`...
--> $DIR/auto-trait-leak.rs:22:16
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^
note: ...which requires borrow-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires unsafety-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22: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
|
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 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which requires evaluating trait selection obligation `impl std::clone::Clone: std::marker::Send`...
note: ...which requires computing type of `cycle2::{{opaque}}#0`...
--> $DIR/auto-trait-leak.rs:22:16
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^
note: ...which requires borrow-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires unsafety-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires building MIR for `cycle2`...
--> $DIR/auto-trait-leak.rs:22:1
|
LL | fn cycle2() -> impl Clone {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires type-checking `cycle2`...
--> $DIR/auto-trait-leak.rs:22: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[E0277]: `std::rc::Rc<std::string::String>` cannot be sent between threads safely
--> $DIR/auto-trait-leak.rs:16:5
--> $DIR/auto-trait-leak.rs:14:5
|
LL | fn send<T: Send>(_: T) {}
| ---- required by this bound in `send`
Expand All @@ -269,7 +99,7 @@ LL | fn cycle2() -> impl Clone {
= help: within `impl std::clone::Clone`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<std::string::String>`
= note: required because it appears within the type `impl std::clone::Clone`

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

Some errors have detailed explanations: E0277, E0391.
For more information about an error, try `rustc --explain E0277`.
19 changes: 19 additions & 0 deletions src/test/ui/issues/issue-72839-error-overflow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression test for issue #72839
// Tests that we do not overflow during trait selection after
// a type error occurs
use std::ops::Rem;
trait Foo {}
struct MyStruct<T>(T);

impl<T, U> Rem<MyStruct<T>> for MyStruct<U> where MyStruct<U>: Rem<MyStruct<T>> {
type Output = u8;
fn rem(self, _: MyStruct<T>) -> Self::Output {
panic!()
}
}

fn main() {}

fn foo() {
if missing_var % 8 == 0 {} //~ ERROR cannot find
}
9 changes: 9 additions & 0 deletions src/test/ui/issues/issue-72839-error-overflow.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0425]: cannot find value `missing_var` in this scope
--> $DIR/issue-72839-error-overflow.rs:18:8
|
LL | if missing_var % 8 == 0 {}
| ^^^^^^^^^^^ not found in this scope

error: aborting due to previous error

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