Kill validateExtension #27279
Kill validateExtension #27279
Conversation
|
@swift-ci please smoke test |
|
No idea what the LLDB failure is. @swift-ci please smoke test macOS platform |
|
And because moving cycles changes adopters for awful reasons: @swift-ci please test source compatibility |
lib/AST/Type.cpp
Outdated
| if (auto valueDecl = dyn_cast<ValueDecl>(decl)) | ||
| resolver->resolveDeclSignature(valueDecl); | ||
| if (auto valueDecl = decl->getAsGenericContext()) | ||
| (void)valueDecl->getGenericSignature(); |
There was a problem hiding this comment.
As a follow-up, we can use getGenericContextDepth() directly to compute the depth here.
lib/Sema/TypeCheckDecl.cpp
Outdated
| // Validate the nominal type declaration being extended. | ||
| TC.validateDecl(nominal); | ||
| // Don't bother computing the generic signature if the extended nominal | ||
| // type didn't pass validation so we don't crash. |
There was a problem hiding this comment.
Do we have tests that crash without the isInvalid() check? It's unfortunate. We should fix that in a follow-up too.
| tc.validateDecl(extendedNominal); | ||
| if (extendedNominal->isInvalid()) | ||
| return true; | ||
|
|
||
| // Assume unconstrained concrete extensions we found witnesses in are | ||
| // always viable. | ||
| if (!isa<ProtocolDecl>(extendedNominal)) | ||
| return !extension->isConstrainedExtension(); |
There was a problem hiding this comment.
isConstrainedExtension() calls getGenericSignature(). It could probably just be a syntactic property instead, by checking for the presence of a parsed where clause.
| // Build a generic signature. | ||
| tc.validateExtension(extension); | ||
|
|
||
|
|
||
| // FIXME: The extension may not have a generic signature set up yet as | ||
| // resolving signatures may trigger associated type inference. This cycle | ||
| // is now detectable and we should look into untangling it | ||
| // - see rdar://55263708 | ||
| if (!extension->hasComputedGenericSignature()) |
There was a problem hiding this comment.
Given the above isConstrainedExtension() check, this is only going to be hit for protocol extensions...
lib/Sema/TypeChecker.cpp
Outdated
| if (auto ext = dyn_cast<ExtensionDecl>(D)) { | ||
| if (auto *nominal = ext->getExtendedNominal()) { | ||
| // Validate the nominal type declaration being extended. | ||
| TC.validateDecl(nominal); |
There was a problem hiding this comment.
This is probably unnecessary
lib/Sema/TypeChecker.h
Outdated
| @@ -1017,10 +1017,6 @@ class TypeChecker final : public LazyResolver { | |||
| validateDecl(proto); | |||
| } | |||
|
|
|||
| virtual void resolveExtension(ExtensionDecl *ext) override { | |||
If it's going to validate the declaration, it wants to know if there's an interface type. Computing the generic signature can be done on demand.
The general class of cycle here is when validation asks for the generic signature which triggers requirement checking which forces us to ask for the generic signature of the extension again. We should look into a more principled solution. See rdar://55263708
CodaFi
force-pushed
the
eggs-tension-service
branch
from
1218e16 to
e7fccde
Compare
|
@swift-ci please smoke test |
|
⛵️ |
Discharge the work the callers of
validateExtensionactually wanted done into those callers. This patch is bittersweet: It kills one dead cycle check, but adds another unrelated one.