[Sema] Improve handling of invalid protocols during conformance checking #910
Conversation
| @@ -241,7 +241,7 @@ void NormalProtocolConformance::setTypeWitness( | |||
| assert(getProtocol() == cast<ProtocolDecl>(assocType->getDeclContext()) && | |||
| "associated type in wrong protocol"); | |||
| assert(TypeWitnesses.count(assocType) == 0 && "Type witness already known"); | |||
| assert(!isComplete() && "Conformance already complete?"); | |||
| assert((!isComplete() || isInvalid()) && "Conformance already complete?"); | |||
There was a problem hiding this comment.
Note: there is precedent for this; a similar change was made in c52f920
There was a problem hiding this comment.
This assertion message could be a little bit more precise now, right?
There was a problem hiding this comment.
What would you suggest?
There was a problem hiding this comment.
You could say "Conformance already complete or invalid" but then one wouldn't know which condition actually failed, so maybe separating into 2 assertions would be better?
There was a problem hiding this comment.
"Well actually" (sorry 😬), it would have to be "Conformance already complete and not invalid". Which to me seems implied by the fact that the assertion is being hit. If it's invalid we don't care whether it's complete.
|
@jtbandes Nice stuff! 👍 Very happy to see these fuzzing cases getting fixed :-) |
|
@practicalswift I'm afraid I'll never catch up with your fuzzer 😄 |
|
@jtbandes No worries - the number of possible crash locations is finite, so given enough time you'll catch up eventually :-) |
|
But also — thanks for doing the fuzzing! It provides a great source of "intro" bugs for gaining some familiarity with the codebase :) |
[Sema] Improve handling of invalid protocols during conformance checking
Fixes 4 compiler_crashers.