Skip to content

Don't ICE on pending obligations from deep normalization in a loop #140021

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+159 −14
Open

Don't ICE on pending obligations from deep normalization in a loop #140021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion compiler/rustc_trait_selection/src/traits/normalize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ impl<'tcx> At<'_, 'tcx> {
let value = self
.normalize(value)
.into_value_registering_obligations(self.infcx, &mut *fulfill_cx);
let errors = fulfill_cx.select_all_or_error(self.infcx);
let mut errors = fulfill_cx.select_all_or_error(self.infcx);
// Drain pending obligations too, since deep normalization may happen
// in a loop and we don't want to trigger the assertion on the next
// iteration due to pending obligations we've left over.
errors.extend(fulfill_cx.collect_remaining_errors(self.infcx));
Comment on lines +78 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, fn select_all_or_error is currently implemented as

    fn select_all_or_error(&mut self, infcx: &InferCtxt<'tcx>) -> Vec<E> {
        let errors = self.select_where_possible(infcx);
        if !errors.is_empty() {
            return errors;
        }

        self.collect_remaining_errors(infcx)
    }

it feels a bit footgunny to not add the overflowing obligations if there are other errors. I think changing the if errors.is_empty() branch to also add the remaining errors would be better 🤔

Copy link
Contributor

@lcnr lcnr Apr 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

or no, in case select_where_possible returns an error, we want to ignore overflow and ambiguity errors. I think we also want to ignore these ambiguity errors here, so maybe change this to

            let errors = fulfill_cx.select_all_or_error(self.infcx);
            if errors.is_empty() {
                Ok(self.infcx.resolve_vars_if_possible(value))
            } else {
                // In case normalization resulted in an error, we want to
                // drain any still ambiguous goal to avoid triggering the
                // assertion on the next iteration.
                let _ = fulfill_cx.collect_remaining_errors(self.infcx));
                Err(errors)
            }

Copy link
Contributor

@lcnr lcnr Apr 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

alternatively we could drop the remaining obligations in the !errors.is_empty() branch in select_all_or_error itself? 🤔

let value = self.infcx.resolve_vars_if_possible(value);
if errors.is_empty() { Ok(value) } else { Err(errors) }
}
Expand Down
13 changes: 0 additions & 13 deletions tests/crashes/133868.rs
View file
Open in desktop

This file was deleted.

24 changes: 24 additions & 0 deletions tests/ui/traits/deep-norm-pending.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
trait Foo {
type Assoc;
}

trait Bar {
fn method() -> impl Sized;
//~^ ERROR the trait bound `T: Foo` is not satisfied
}
impl<T> Bar for T
//~^ ERROR the trait bound `T: Foo` is not satisfied
//~| ERROR the trait bound `T: Foo` is not satisfied
where
<T as Foo>::Assoc: Sized,
{
fn method() {}
//~^ ERROR the trait bound `T: Foo` is not satisfied
//~| ERROR the trait bound `T: Foo` is not satisfied
//~| ERROR the trait bound `T: Foo` is not satisfied
//~| ERROR the trait bound `T: Foo` is not satisfied
//~| ERROR the trait bound `T: Foo` is not satisfied
//~| ERROR the trait bound `T: Foo` is not satisfied
}

fn main() {}
130 changes: 130 additions & 0 deletions tests/ui/traits/deep-norm-pending.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:15:5
|
LL | fn method() {}
| ^^^^^^^^^^^ the trait `Foo` is not implemented for `T`
|
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:9:1
|
LL | / impl<T> Bar for T
LL | |
LL | |
LL | | where
LL | | <T as Foo>::Assoc: Sized,
| |_____________________________^ the trait `Foo` is not implemented for `T`
|
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:9:1
|
LL | / impl<T> Bar for T
LL | |
LL | |
LL | | where
... |
LL | | }
| |_^ the trait `Foo` is not implemented for `T`
|
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:15:5
|
LL | fn method() {}
| ^^^^^^^^^^^ the trait `Foo` is not implemented for `T`
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:15:5
|
LL | fn method() {}
| ^^^^^^^^^^^ the trait `Foo` is not implemented for `T`
|
note: required for `T` to implement `Bar`
--> $DIR/deep-norm-pending.rs:9:9
|
LL | impl<T> Bar for T
| ^^^ ^
...
LL | <T as Foo>::Assoc: Sized,
| ----- unsatisfied trait bound introduced here
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:15:5
|
LL | fn method() {}
| ^^^^^^^^^^^ the trait `Foo` is not implemented for `T`
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:6:20
|
LL | fn method() -> impl Sized;
| ^^^^^^^^^^ the trait `Foo` is not implemented for `T`
|
note: required for `T` to implement `Bar`
--> $DIR/deep-norm-pending.rs:9:9
|
LL | impl<T> Bar for T
| ^^^ ^
...
LL | <T as Foo>::Assoc: Sized,
| ----- unsatisfied trait bound introduced here
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:15:5
|
LL | fn method() {}
| ^^^^^^^^^^^ the trait `Foo` is not implemented for `T`
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error[E0277]: the trait bound `T: Foo` is not satisfied
--> $DIR/deep-norm-pending.rs:15:8
|
LL | fn method() {}
| ^^^^^^ the trait `Foo` is not implemented for `T`
|
help: consider further restricting type parameter `T` with trait `Foo`
|
LL | <T as Foo>::Assoc: Sized, T: Foo
| ++++++

error: aborting due to 9 previous errors

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