Skip to content

Clean up extension validation #19386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 19, 2018
Merged

Clean up extension validation #19386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7eed6c4
AST: Remove LazyResolver::bindExtension()
slavapestov Sep 19, 2018
f466aeb
Sema: Don't call bindExtension() in inferTypeWitnessesViaValueWitness…
slavapestov Sep 19, 2018
26fddb7
Sema: bindExtensions() doesn't need to diagnose extensions right away
slavapestov Sep 19, 2018
ef82cd7
Sema: Merge bindExtensionDecl() into validateExtension()
slavapestov Sep 19, 2018
4c7e889
Sema: isPassThroughTypealias() is only used in one file
slavapestov Sep 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions include/swift/AST/LazyResolver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ class LazyResolver {
/// Resolve the generic environment of the given protocol.
virtual void resolveProtocolEnvironment(ProtocolDecl *proto) = 0;

/// Bind an extension to its extended type.
virtual void bindExtension(ExtensionDecl *ext) = 0;

/// Resolve the type of an extension.
///
/// This can be called to ensure that the members of an extension can be
Expand Down
6 changes: 5 additions & 1 deletion lib/AST/DeclContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,11 @@ GenericTypeParamType *DeclContext::getProtocolSelfType() const {
const_cast<ProtocolDecl *>(proto)->createGenericParamsIfMissing();
}

return getGenericParamsOfContext()->getParams().front()
auto *genericParams = getGenericParamsOfContext();
if (genericParams == nullptr)
return nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this come up for a protocol extension?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid code, like when an extension is inside of another decl, we don't build the generic parameter list for it anymore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we still bind the underlying type? Okay.


return genericParams->getParams().front()
->getDeclaredInterfaceType()
->castTo<GenericTypeParamType>();
}
Expand Down
3 changes: 3 additions & 0 deletions lib/AST/NameLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,9 @@ SelfBoundsFromWhereClauseRequest::evaluate(Evaluator &evaluator,

ASTContext &ctx = proto->getASTContext();
TinyPtrVector<NominalTypeDecl *> result;
if (!ext->getGenericParams())
return result;

for (const auto &req : ext->getGenericParams()->getTrailingRequirements()) {
// We only care about type constraints.
if (req.getKind() != RequirementReprKind::TypeConstraint)
Expand Down
97 changes: 81 additions & 16 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4689,7 +4689,25 @@ void TypeChecker::finalizeDecl(ValueDecl *decl) {
(void)decl->isDynamic();
}

bool swift::isPassThroughTypealias(TypeAliasDecl *typealias) {
/// Determine whether this is a "pass-through" typealias, which has the
/// same type parameters as the nominal type it references and specializes
/// the underlying nominal type with exactly those type parameters.
/// For example, the following typealias \c GX is a pass-through typealias:
///
/// \code
/// struct X<T, U> { }
/// typealias GX<A, B> = X<A, B>
/// \endcode
///
/// whereas \c GX2 and \c GX3 are not pass-through because \c GX2 has
/// different type parameters and \c GX3 doesn't pass its type parameters
/// directly through.
///
/// \code
/// typealias GX2<A> = X<A, A>
/// typealias GX3<A, B> = X<B, A>
/// \endcode
static bool isPassThroughTypealias(TypeAliasDecl *typealias) {
// Pass-through only makes sense when the typealias refers to a nominal
// type.
Type underlyingType = typealias->getUnderlyingTypeLoc().getType();
Expand Down Expand Up @@ -4877,31 +4895,78 @@ void TypeChecker::validateExtension(ExtensionDecl *ext) {
if (ext->hasValidationStarted())
return;

bindExtension(ext);

DeclValidationRAII IBV(ext);

// If the extension is already known to be invalid, we're done.
if (ext->isInvalid())
auto dc = ext->getDeclContext();

// If we didn't parse a type, fill in an error type and bail out.
if (!ext->getExtendedTypeLoc().getTypeRepr()) {
ext->setInvalid();
ext->getExtendedTypeLoc().setInvalidType(Context);
return;
}

// Validate the extended type.
TypeResolutionOptions options(TypeResolverContext::ExtensionBinding);
options |= TypeResolutionFlags::AllowUnboundGenerics;
if (validateType(ext->getExtendedTypeLoc(),
TypeResolution::forContextual(dc), options)) {
ext->setInvalid();
ext->getExtendedTypeLoc().setInvalidType(Context);
return;
}

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

if (extendedType.isNull() || extendedType->hasError())
// Hack to allow extending a generic typealias.
if (auto *unboundGeneric = extendedType->getAs<UnboundGenericType>()) {
if (auto *aliasDecl = dyn_cast<TypeAliasDecl>(unboundGeneric->getDecl())) {
auto extendedNominal = aliasDecl->getDeclaredInterfaceType()->getAnyNominal();
if (extendedNominal) {
extendedType = extendedNominal->getDeclaredType();
if (!isPassThroughTypealias(aliasDecl))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this is where we should diagnose non-passthrough typealias declarations that will turn into extensions of the underlying nominal type (and not become parameterized extensions magically).

ext->getExtendedTypeLoc().setType(extendedType);
}
}
}

// Cannot extend a metatype.
if (extendedType->is<AnyMetatypeType>()) {
diagnose(ext->getLoc(), diag::extension_metatype, extendedType)
.highlight(ext->getExtendedTypeLoc().getSourceRange());
ext->setInvalid();
ext->getExtendedTypeLoc().setInvalidType(Context);
return;
}

// Validate the nominal type declaration being extended.
NominalTypeDecl *nominal = extendedType->getAnyNominal();
if (!nominal) {
auto unbound = cast<UnboundGenericType>(extendedType.getPointer());
auto typealias = cast<TypeAliasDecl>(unbound->getDecl());
validateDecl(typealias);
// Cannot extend a bound generic type.
if (extendedType->isSpecialized()) {
diagnose(ext->getLoc(), diag::extension_specialization,
extendedType->getAnyNominal()->getName())
.highlight(ext->getExtendedTypeLoc().getSourceRange());
ext->setInvalid();
ext->getExtendedTypeLoc().setInvalidType(Context);
return;
}

nominal = typealias->getUnderlyingTypeLoc().getType()->getAnyNominal();
auto *nominal = extendedType->getAnyNominal();

// Cannot extend function types, tuple types, etc.
if (nominal == nullptr) {
diagnose(ext->getLoc(), diag::non_nominal_extension, extendedType)
.highlight(ext->getExtendedTypeLoc().getSourceRange());
ext->setInvalid();
ext->getExtendedTypeLoc().setInvalidType(Context);
return;
}

// Extensions nested inside other declarations are invalid and we
// do not bind them.
if (!isa<SourceFile>(dc))
return;

// Validate the nominal type declaration being extended.
validateDecl(nominal);

if (nominal->getGenericParamsOfContext()) {
Expand Down
14 changes: 9 additions & 5 deletions lib/Sema/TypeCheckProtocolInference.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,15 +183,19 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses(
if (extension == conformanceExtension)
return true;

tc.bindExtension(extension);
auto *extendedNominal = extension->getExtendedNominal();

// Invalid case.
if (extendedNominal == nullptr)
return true;

// Assume unconstrained concrete extensions we found witnesses in are
// always viable.
if (!extension->getExtendedType()->isAnyExistentialType()) {
// TODO: When constrained extensions are a thing, we'll need an "is
// as specialized as" kind of check here.
if (!isa<ProtocolDecl>(extendedNominal))
return !extension->isConstrainedExtension();
}

// Build a generic signature.
tc.validateExtension(extension);

// The extension may not have a generic signature set up yet, as a
// recursion breaker, in which case we can't yet confidently reject its
Expand Down
84 changes: 2 additions & 82 deletions lib/Sema/TypeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,80 +346,6 @@ static void bindExtensionToNominal(ExtensionDecl *ext,
nominal->addExtension(ext);
}

static void bindExtensionDecl(ExtensionDecl *ED, TypeChecker &TC) {
if (ED->getExtendedType())
return;

// If we didn't parse a type, fill in an error type and bail out.
if (!ED->getExtendedTypeLoc().getTypeRepr()) {
ED->setInvalid();
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
return;
}

auto dc = ED->getDeclContext();

// Validate the representation.
// FIXME: Perform some kind of "shallow" validation here?
TypeResolutionOptions options(TypeResolverContext::ExtensionBinding);
options |= TypeResolutionFlags::AllowUnboundGenerics;
if (TC.validateType(ED->getExtendedTypeLoc(),
TypeResolution::forContextual(dc), options)) {
ED->setInvalid();
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
return;
}

// Dig out the extended type.
auto extendedType = ED->getExtendedType();

// Hack to allow extending a generic typealias.
if (auto *unboundGeneric = extendedType->getAs<UnboundGenericType>()) {
if (auto *aliasDecl = dyn_cast<TypeAliasDecl>(unboundGeneric->getDecl())) {
auto extendedNominal = aliasDecl->getDeclaredInterfaceType()->getAnyNominal();
if (extendedNominal) {
extendedType = extendedNominal->getDeclaredType();
if (!isPassThroughTypealias(aliasDecl))
ED->getExtendedTypeLoc().setType(extendedType);
}
}
}

// Handle easy cases.

// Cannot extend a metatype.
if (extendedType->is<AnyMetatypeType>()) {
TC.diagnose(ED->getLoc(), diag::extension_metatype, extendedType)
.highlight(ED->getExtendedTypeLoc().getSourceRange());
ED->setInvalid();
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
return;
}

// Cannot extend a bound generic type.
if (extendedType->isSpecialized()) {
TC.diagnose(ED->getLoc(), diag::extension_specialization,
extendedType->getAnyNominal()->getName())
.highlight(ED->getExtendedTypeLoc().getSourceRange());
ED->setInvalid();
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
return;
}

// Dig out the nominal type being extended.
NominalTypeDecl *extendedNominal = extendedType->getAnyNominal();
if (!extendedNominal) {
TC.diagnose(ED->getLoc(), diag::non_nominal_extension, extendedType)
.highlight(ED->getExtendedTypeLoc().getSourceRange());
ED->setInvalid();
ED->getExtendedTypeLoc().setInvalidType(TC.Context);
return;
}
assert(extendedNominal && "Should have the nominal type being extended");

bindExtensionToNominal(ED, extendedNominal);
}

static void bindExtensions(SourceFile &SF, TypeChecker &TC) {
// Utility function to try and resolve the extended type without diagnosing.
// If we succeed, we go ahead and bind the extension. Otherwise, return false.
Expand Down Expand Up @@ -467,14 +393,8 @@ static void bindExtensions(SourceFile &SF, TypeChecker &TC) {
}
} while(changed);

// Phase 3 - anything that remains on the worklist cannot be resolved, which
// means its invalid. Diagnose.
for (auto *ext : worklist)
bindExtensionDecl(ext, TC);
}

void TypeChecker::bindExtension(ExtensionDecl *ext) {
::bindExtensionDecl(ext, *this);
// Any remaining extensions are invalid. They will be diagnosed later by
// typeCheckDecl().
}

static void typeCheckFunctionsAndExternalDecls(SourceFile &SF, TypeChecker &TC) {
Expand Down
22 changes: 0 additions & 22 deletions lib/Sema/TypeChecker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1106,8 +1106,6 @@ class TypeChecker final : public LazyResolver {
validateDecl(proto);
}

virtual void bindExtension(ExtensionDecl *ext) override;

virtual void resolveExtension(ExtensionDecl *ext) override {
validateExtension(ext);
}
Expand Down Expand Up @@ -2173,26 +2171,6 @@ bool isAcceptableDynamicMemberLookupSubscript(SubscriptDecl *decl,
DeclContext *DC,
TypeChecker &TC);

/// Determine whether this is a "pass-through" typealias, which has the
/// same type parameters as the nominal type it references and specializes
/// the underlying nominal type with exactly those type parameters.
/// For example, the following typealias \c GX is a pass-through typealias:
///
/// \code
/// struct X<T, U> { }
/// typealias GX<A, B> = X<A, B>
/// \endcode
///
/// whereas \c GX2 and \c GX3 are not pass-through because \c GX2 has
/// different type parameters and \c GX3 doesn't pass its type parameters
/// directly through.
///
/// \code
/// typealias GX2<A> = X<A, A>
/// typealias GX3<A, B> = X<B, A>
/// \endcode
bool isPassThroughTypealias(TypeAliasDecl *typealias);

/// Whether an overriding declaration requires the 'override' keyword.
enum class OverrideRequiresKeyword {
/// The keyword is never required.
Expand Down