Skip to content

Commit 165f658

Browse files
slavapestovslavapestov
authored
Merge pull request #19386 from slavapestov/validate-extension-cleanup
Clean up extension validation
2 parents 555ee21 + 4c7e889 commit 165f658

File tree

7 files changed

+100
-129
lines changed

7 files changed

+100
-129
lines changed

‎include/swift/AST/LazyResolver.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ class LazyResolver {
6363
/// Resolve the generic environment of the given protocol.
6464
virtual void resolveProtocolEnvironment(ProtocolDecl *proto) = 0;
6565

66-
/// Bind an extension to its extended type.
67-
virtual void bindExtension(ExtensionDecl *ext) = 0;
68-
6966
/// Resolve the type of an extension.
7067
///
7168
/// This can be called to ensure that the members of an extension can be

‎lib/AST/DeclContext.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,11 @@ GenericTypeParamType *DeclContext::getProtocolSelfType() const {
9292
const_cast<ProtocolDecl *>(proto)->createGenericParamsIfMissing();
9393
}
9494

95-
return getGenericParamsOfContext()->getParams().front()
95+
auto *genericParams = getGenericParamsOfContext();
96+
if (genericParams == nullptr)
97+
return nullptr;
98+
99+
return genericParams->getParams().front()
96100
->getDeclaredInterfaceType()
97101
->castTo<GenericTypeParamType>();
98102
}

‎lib/AST/NameLookup.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,9 @@ SelfBoundsFromWhereClauseRequest::evaluate(Evaluator &evaluator,
564564

565565
ASTContext &ctx = proto->getASTContext();
566566
TinyPtrVector<NominalTypeDecl *> result;
567+
if (!ext->getGenericParams())
568+
return result;
569+
567570
for (const auto &req : ext->getGenericParams()->getTrailingRequirements()) {
568571
// We only care about type constraints.
569572
if (req.getKind() != RequirementReprKind::TypeConstraint)

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 81 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4689,7 +4689,25 @@ void TypeChecker::finalizeDecl(ValueDecl *decl) {
46894689
(void)decl->isDynamic();
46904690
}
46914691

4692-
bool swift::isPassThroughTypealias(TypeAliasDecl *typealias) {
4692+
/// Determine whether this is a "pass-through" typealias, which has the
4693+
/// same type parameters as the nominal type it references and specializes
4694+
/// the underlying nominal type with exactly those type parameters.
4695+
/// For example, the following typealias \c GX is a pass-through typealias:
4696+
///
4697+
/// \code
4698+
/// struct X<T, U> { }
4699+
/// typealias GX<A, B> = X<A, B>
4700+
/// \endcode
4701+
///
4702+
/// whereas \c GX2 and \c GX3 are not pass-through because \c GX2 has
4703+
/// different type parameters and \c GX3 doesn't pass its type parameters
4704+
/// directly through.
4705+
///
4706+
/// \code
4707+
/// typealias GX2<A> = X<A, A>
4708+
/// typealias GX3<A, B> = X<B, A>
4709+
/// \endcode
4710+
static bool isPassThroughTypealias(TypeAliasDecl *typealias) {
46934711
// Pass-through only makes sense when the typealias refers to a nominal
46944712
// type.
46954713
Type underlyingType = typealias->getUnderlyingTypeLoc().getType();
@@ -4877,31 +4895,78 @@ void TypeChecker::validateExtension(ExtensionDecl *ext) {
48774895
if (ext->hasValidationStarted())
48784896
return;
48794897

4880-
bindExtension(ext);
4881-
48824898
DeclValidationRAII IBV(ext);
48834899

4884-
// If the extension is already known to be invalid, we're done.
4885-
if (ext->isInvalid())
4900+
auto dc = ext->getDeclContext();
4901+
4902+
// If we didn't parse a type, fill in an error type and bail out.
4903+
if (!ext->getExtendedTypeLoc().getTypeRepr()) {
4904+
ext->setInvalid();
4905+
ext->getExtendedTypeLoc().setInvalidType(Context);
4906+
return;
4907+
}
4908+
4909+
// Validate the extended type.
4910+
TypeResolutionOptions options(TypeResolverContext::ExtensionBinding);
4911+
options |= TypeResolutionFlags::AllowUnboundGenerics;
4912+
if (validateType(ext->getExtendedTypeLoc(),
4913+
TypeResolution::forContextual(dc), options)) {
4914+
ext->setInvalid();
4915+
ext->getExtendedTypeLoc().setInvalidType(Context);
48864916
return;
4917+
}
48874918

4888-
// FIXME: We need to check whether anything is specialized, because
4889-
// the innermost extended type might itself be a non-generic type
4890-
// within a generic type.
4919+
// Dig out the extended type.
48914920
auto extendedType = ext->getExtendedType();
48924921

4893-
if (extendedType.isNull() || extendedType->hasError())
4922+
// Hack to allow extending a generic typealias.
4923+
if (auto *unboundGeneric = extendedType->getAs<UnboundGenericType>()) {
4924+
if (auto *aliasDecl = dyn_cast<TypeAliasDecl>(unboundGeneric->getDecl())) {
4925+
auto extendedNominal = aliasDecl->getDeclaredInterfaceType()->getAnyNominal();
4926+
if (extendedNominal) {
4927+
extendedType = extendedNominal->getDeclaredType();
4928+
if (!isPassThroughTypealias(aliasDecl))
4929+
ext->getExtendedTypeLoc().setType(extendedType);
4930+
}
4931+
}
4932+
}
4933+
4934+
// Cannot extend a metatype.
4935+
if (extendedType->is<AnyMetatypeType>()) {
4936+
diagnose(ext->getLoc(), diag::extension_metatype, extendedType)
4937+
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4938+
ext->setInvalid();
4939+
ext->getExtendedTypeLoc().setInvalidType(Context);
48944940
return;
4941+
}
48954942

4896-
// Validate the nominal type declaration being extended.
4897-
NominalTypeDecl *nominal = extendedType->getAnyNominal();
4898-
if (!nominal) {
4899-
auto unbound = cast<UnboundGenericType>(extendedType.getPointer());
4900-
auto typealias = cast<TypeAliasDecl>(unbound->getDecl());
4901-
validateDecl(typealias);
4943+
// Cannot extend a bound generic type.
4944+
if (extendedType->isSpecialized()) {
4945+
diagnose(ext->getLoc(), diag::extension_specialization,
4946+
extendedType->getAnyNominal()->getName())
4947+
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4948+
ext->setInvalid();
4949+
ext->getExtendedTypeLoc().setInvalidType(Context);
4950+
return;
4951+
}
49024952

4903-
nominal = typealias->getUnderlyingTypeLoc().getType()->getAnyNominal();
4953+
auto *nominal = extendedType->getAnyNominal();
4954+
4955+
// Cannot extend function types, tuple types, etc.
4956+
if (nominal == nullptr) {
4957+
diagnose(ext->getLoc(), diag::non_nominal_extension, extendedType)
4958+
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4959+
ext->setInvalid();
4960+
ext->getExtendedTypeLoc().setInvalidType(Context);
4961+
return;
49044962
}
4963+
4964+
// Extensions nested inside other declarations are invalid and we
4965+
// do not bind them.
4966+
if (!isa<SourceFile>(dc))
4967+
return;
4968+
4969+
// Validate the nominal type declaration being extended.
49054970
validateDecl(nominal);
49064971

49074972
if (nominal->getGenericParamsOfContext()) {

‎lib/Sema/TypeCheckProtocolInference.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,15 +183,19 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
183183
if (extension == conformanceExtension)
184184
return true;
185185

186-
tc.bindExtension(extension);
186+
auto *extendedNominal = extension->getExtendedNominal();
187+
188+
// Invalid case.
189+
if (extendedNominal == nullptr)
190+
return true;
187191

188192
// Assume unconstrained concrete extensions we found witnesses in are
189193
// always viable.
190-
if (!extension->getExtendedType()->isAnyExistentialType()) {
191-
// TODO: When constrained extensions are a thing, we'll need an "is
192-
// as specialized as" kind of check here.
194+
if (!isa<ProtocolDecl>(extendedNominal))
193195
return !extension->isConstrainedExtension();
194-
}
196+
197+
// Build a generic signature.
198+
tc.validateExtension(extension);
195199

196200
// The extension may not have a generic signature set up yet, as a
197201
// recursion breaker, in which case we can't yet confidently reject its

‎lib/Sema/TypeChecker.cpp

Lines changed: 2 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -346,80 +346,6 @@ static void bindExtensionToNominal(ExtensionDecl *ext,
346346
nominal->addExtension(ext);
347347
}
348348

349-
static void bindExtensionDecl(ExtensionDecl *ED, TypeChecker &TC) {
350-
if (ED->getExtendedType())
351-
return;
352-
353-
// If we didn't parse a type, fill in an error type and bail out.
354-
if (!ED->getExtendedTypeLoc().getTypeRepr()) {
355-
ED->setInvalid();
356-
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
357-
return;
358-
}
359-
360-
auto dc = ED->getDeclContext();
361-
362-
// Validate the representation.
363-
// FIXME: Perform some kind of "shallow" validation here?
364-
TypeResolutionOptions options(TypeResolverContext::ExtensionBinding);
365-
options |= TypeResolutionFlags::AllowUnboundGenerics;
366-
if (TC.validateType(ED->getExtendedTypeLoc(),
367-
TypeResolution::forContextual(dc), options)) {
368-
ED->setInvalid();
369-
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
370-
return;
371-
}
372-
373-
// Dig out the extended type.
374-
auto extendedType = ED->getExtendedType();
375-
376-
// Hack to allow extending a generic typealias.
377-
if (auto *unboundGeneric = extendedType->getAs<UnboundGenericType>()) {
378-
if (auto *aliasDecl = dyn_cast<TypeAliasDecl>(unboundGeneric->getDecl())) {
379-
auto extendedNominal = aliasDecl->getDeclaredInterfaceType()->getAnyNominal();
380-
if (extendedNominal) {
381-
extendedType = extendedNominal->getDeclaredType();
382-
if (!isPassThroughTypealias(aliasDecl))
383-
ED->getExtendedTypeLoc().setType(extendedType);
384-
}
385-
}
386-
}
387-
388-
// Handle easy cases.
389-
390-
// Cannot extend a metatype.
391-
if (extendedType->is<AnyMetatypeType>()) {
392-
TC.diagnose(ED->getLoc(), diag::extension_metatype, extendedType)
393-
.highlight(ED->getExtendedTypeLoc().getSourceRange());
394-
ED->setInvalid();
395-
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
396-
return;
397-
}
398-
399-
// Cannot extend a bound generic type.
400-
if (extendedType->isSpecialized()) {
401-
TC.diagnose(ED->getLoc(), diag::extension_specialization,
402-
extendedType->getAnyNominal()->getName())
403-
.highlight(ED->getExtendedTypeLoc().getSourceRange());
404-
ED->setInvalid();
405-
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
406-
return;
407-
}
408-
409-
// Dig out the nominal type being extended.
410-
NominalTypeDecl *extendedNominal = extendedType->getAnyNominal();
411-
if (!extendedNominal) {
412-
TC.diagnose(ED->getLoc(), diag::non_nominal_extension, extendedType)
413-
.highlight(ED->getExtendedTypeLoc().getSourceRange());
414-
ED->setInvalid();
415-
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
416-
return;
417-
}
418-
assert(extendedNominal && "Should have the nominal type being extended");
419-
420-
bindExtensionToNominal(ED, extendedNominal);
421-
}
422-
423349
static void bindExtensions(SourceFile &SF, TypeChecker &TC) {
424350
// Utility function to try and resolve the extended type without diagnosing.
425351
// If we succeed, we go ahead and bind the extension. Otherwise, return false.
@@ -467,14 +393,8 @@ static void bindExtensions(SourceFile &SF, TypeChecker &TC) {
467393
}
468394
} while(changed);
469395

470-
// Phase 3 - anything that remains on the worklist cannot be resolved, which
471-
// means its invalid. Diagnose.
472-
for (auto *ext : worklist)
473-
bindExtensionDecl(ext, TC);
474-
}
475-
476-
void TypeChecker::bindExtension(ExtensionDecl *ext) {
477-
::bindExtensionDecl(ext, *this);
396+
// Any remaining extensions are invalid. They will be diagnosed later by
397+
// typeCheckDecl().
478398
}
479399

480400
static void typeCheckFunctionsAndExternalDecls(SourceFile &SF, TypeChecker &TC) {

‎lib/Sema/TypeChecker.h

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,8 +1106,6 @@ class TypeChecker final : public LazyResolver {
11061106
validateDecl(proto);
11071107
}
11081108

1109-
virtual void bindExtension(ExtensionDecl *ext) override;
1110-
11111109
virtual void resolveExtension(ExtensionDecl *ext) override {
11121110
validateExtension(ext);
11131111
}
@@ -2173,26 +2171,6 @@ bool isAcceptableDynamicMemberLookupSubscript(SubscriptDecl *decl,
21732171
DeclContext *DC,
21742172
TypeChecker &TC);
21752173

2176-
/// Determine whether this is a "pass-through" typealias, which has the
2177-
/// same type parameters as the nominal type it references and specializes
2178-
/// the underlying nominal type with exactly those type parameters.
2179-
/// For example, the following typealias \c GX is a pass-through typealias:
2180-
///
2181-
/// \code
2182-
/// struct X<T, U> { }
2183-
/// typealias GX<A, B> = X<A, B>
2184-
/// \endcode
2185-
///
2186-
/// whereas \c GX2 and \c GX3 are not pass-through because \c GX2 has
2187-
/// different type parameters and \c GX3 doesn't pass its type parameters
2188-
/// directly through.
2189-
///
2190-
/// \code
2191-
/// typealias GX2<A> = X<A, A>
2192-
/// typealias GX3<A, B> = X<B, A>
2193-
/// \endcode
2194-
bool isPassThroughTypealias(TypeAliasDecl *typealias);
2195-
21962174
/// Whether an overriding declaration requires the 'override' keyword.
21972175
enum class OverrideRequiresKeyword {
21982176
/// The keyword is never required.

0 commit comments

Comments
 (0)