[Sema] Handle invalid @_specialize generic signatures#89239
Conversation
Reject invalid @_specialize generic signatures before serialization can reconstruct interface types from inconsistent requirement-machine state. Add a reduced regression test for swiftlang#89154. Resolves swiftlang#89154
Jiaxu-Li
requested review from
hamishknight,
hborla,
slavapestov and
xedin
as code owners
slavapestov
left a comment
slavapestov
left a comment
There was a problem hiding this comment.
Can you spilt this up into multiple commits? It's a pretty involved set of changes and I'd like to go through them one by one.
| // not in protocol requirement signatures. | ||
| if (conformance.isConcrete() && | ||
| key.getRootProtocol() == nullptr) | ||
| inferConditionalRequirements(conformance.getConcrete(), substitutions); |
There was a problem hiding this comment.
It looks like you're plumbing through invalidTypeWitness to avoid a crash in inferConditionalRequirements()? Perhaps there's a better way to address the root cause.
| if (errors.contains(GenericSignatureErrorFlags::HasInvalidRequirements) || | ||
| errors.contains(GenericSignatureErrorFlags::CompletionFailed) || | ||
| errors.contains(GenericSignatureErrorFlags::CircularReference)) { | ||
| attr->setInvalid(); |
There was a problem hiding this comment.
If any of these conditions happened, we should have diagnosed an error in InferredGenericSignatureRequest. What does setting the invalid flag on top of that achieve?
| auto conformsTo = props->getConformsTo(); | ||
| if (std::find(conformsTo.begin(), conformsTo.end(), | ||
| symbol.getProtocol()) == conformsTo.end()) { | ||
| if (astCtx.hadError()) |
There was a problem hiding this comment.
Yep, we have a couple of open bugs here where it's possible to form a type term like T.[P:A].[Q:B] where T.[P:A] doesn't conform to Q.
Here is a crash in another spot that's related, the problem here is that Slice is malformed because Element doesn't conform to Collection:
protocol ElementProtocol {}
protocol MyCollection: RandomAccessCollection, MutableCollection, RangeReplaceableCollection where Index == Int, Element: ElementProtocol, SubSequence == Slice<Element> {
}
I'm not sure what the best fix is, though. Perhaps when expanding a concrete conformance, we should add any missing conformance rules and record the fact that we did.
|
|
||
| auto recordInvalidTypeWitness = [&] { | ||
| if (requirementKind == RequirementKind::SameType) | ||
| System.recordConflict(conformanceRuleID, concreteRuleID); |
There was a problem hiding this comment.
Does this diagnose an error in a case that wasn't previously being diagnosed? Do you have an example not involving @_specialize?
2 participants
Explanation:
Invalid
@_specializegeneric signatures are now rejected gracefully instead of crashing during interface type reconstruction.Scope:
Updates specialize attribute validation, printing, and requirement-machine recovery for invalid associated type witnesses.
Issues:
Resolves [Swift][Specialization] Assertion std::find(conformsTo) != conformsTo.end() in getTypeForSymbolRange when serializing @_specialize generic signature with ambiguous type used in associated type chain #89154
Risk:
Low. The change is limited to invalid-signature recovery after diagnostics have already been emitted.
Testing:
Added a reduced crash regression test on macOS.