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.

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 return definite guidance if we find one solution and then flounder #331

Closed
wants to merge 1 commit 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
4 changes: 4 additions & 0 deletions chalk-solve/src/solve/slg/aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ impl<I: Interner> context::AggregateOps<SlgContext<I>> for SlgContextOps<'_, I>
break Guidance::Unknown;
}

if let AnswerResult::Floundered = answers.peek_answer(|| should_continue()) {
break Guidance::Unknown;
}

if !answers
.any_future_answer(|ref mut new_subst| new_subst.may_invalidate(interner, &subst))
{
Expand Down
57 changes: 57 additions & 0 deletions tests/test/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,63 @@ fn flounder() {
}
}

/// Don't return definite guidance if we flounder after finding one solution.
#[test]
fn flounder_ambiguous() {
test! {
program {
trait IntoIterator { }
#[non_enumerable]
trait OtherTrait { }

struct Ref<T> { }
struct A { }

impl IntoIterator for Ref<A> { }
impl<T> IntoIterator for Ref<T> where T: OtherTrait { }
}

goal {
exists<T> { Ref<T>: IntoIterator }
} yields {
"Ambiguous; no inference guidance"
}
}
}

/// Don't return definite guidance if we are able to merge two solutions and the
/// third one matches that as well (the fourth may not).
#[test]
fn normalize_ambiguous() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this test have anything to do with floundering?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so (except that this test only reproduces the problem with the fix for the other problem, I think because it calls peek_answer)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, curious.

test! {
program {
trait IntoIterator { type Item; }

struct Ref<T> { }
struct A { }
struct B { }
struct C { }

struct D { }

impl IntoIterator for Ref<A> { type Item = Ref<A>; }
impl IntoIterator for Ref<B> { type Item = Ref<B>; }
impl IntoIterator for Ref<C> { type Item = Ref<C>; }
impl IntoIterator for Ref<D> { type Item = D; }
}

goal {
exists<T, U> {
Normalize(<Ref<T> as IntoIterator>::Item -> U)
}
} yields {
// FIXME: this is wrong!
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably create a FIXME policy of creating open issues to track these...

"Ambiguous; definite substitution"
// "Ambiguous; no inference guidance"
}
}
}

// Test that, when solving `?T: Sized`, we only wind up pulling a few
// answers before we stop.
// Also tests that we search breadth-first.
Expand Down