Non-ambigous answer that can replace an ambiguous answer may panic

[jackh726]

It's possible that a new answer may not be ambiguous, but an older answer was. In this case push_answer on Table does not insert a new answer, but still returns true.

chalk/chalk-engine/src/table.rs

Line 107 in 3ef2030

let added = match self.answers_hash.entry(answer.subst.clone()) {

However, if true is returned, then it's propagated up and eventually the AnswerIndex is incremented

chalk/chalk-engine/src/forest.rs

Line 231 in 3ef2030

self.answer.increment();

Then, in the next iteration of checking for an answer, the AnswerIndex is not the next index, so this assertion can fail:

chalk/chalk-engine/src/logic.rs

Line 194 in 3ef2030

assert_eq!(self.tables[table].next_answer_index(), answer);

Simply changing push_answer to panic whenever a new answer would replace an old one doesn't cause any tests to fail. So if this is a valid case, then a test should be added.

Relevant zulip: https://rust-lang.zulipchat.com/#narrow/stream/144729-wg-traits/topic/design.20meeting.202019.2E12.2E02/near/182380076

[lnicola]

This causes a panic when running rust-analyzer over itself, as seen in rust-lang/rust-analyzer#2675.

[jackh726]

@lnicola Thanks for the report. It would be nice to get min repro for this, but I'll start looking at a fix.

[jackh726]

This can also be reproduced with the existing test gat_unify_with_implied_wc with a max_size of 3.

[jackh726]

This was fixed in #433