Skip to content

[4.1] [SR-5022] Diagnose an invalid use of non-polymorphic constructors #14097

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.

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 3 commits into from
Jan 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
153 changes: 89 additions & 64 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,85 @@ static bool buildObjCKeyPathString(KeyPathExpr *E,
return true;
}

/// Determine whether the given type refers to a non-final class (or
/// dynamic self of one).
static bool isNonFinalClass(Type type) {
if (auto dynamicSelf = type->getAs<DynamicSelfType>())
type = dynamicSelf->getSelfType();

if (auto classDecl = type->getClassOrBoundGenericClass())
return !classDecl->isFinal();

if (auto archetype = type->getAs<ArchetypeType>())
if (auto super = archetype->getSuperclass())
return isNonFinalClass(super);

return false;
}

// Non-required constructors may not be not inherited. Therefore when
// constructing a class object, either the metatype must be statically
// derived (rather than an arbitrary value of metatype type) or the referenced
// constructor must be required.
static bool
diagnoseInvalidDynamicConstructorReferences(ConstraintSystem &cs,
Expr *base,
DeclNameLoc memberRefLoc,
ConstructorDecl *ctorDecl,
bool SuppressDiagnostics) {
auto &tc = cs.getTypeChecker();
auto baseTy = cs.getType(base)->getRValueType();
auto instanceTy = baseTy->getRValueInstanceType();

bool isStaticallyDerived =
base->isStaticallyDerivedMetatype(
[&](const Expr *expr) -> Type {
return cs.getType(expr);
});

// 'super.' is always OK
if (isa<SuperRefExpr>(base))
return true;

// 'self.' reference with concrete type is OK
if (isa<DeclRefExpr>(base) &&
cast<DeclRefExpr>(base)->getDecl()->getBaseName() == tc.Context.Id_self &&
!baseTy->is<ArchetypeType>())
return true;

// FIXME: The "hasClangNode" check here is a complete hack.
if (isNonFinalClass(instanceTy) &&
!isStaticallyDerived &&
!ctorDecl->hasClangNode() &&
!(ctorDecl->isRequired() ||
ctorDecl->getDeclContext()->getAsProtocolOrProtocolExtensionContext())) {
if (SuppressDiagnostics)
return false;

tc.diagnose(memberRefLoc, diag::dynamic_construct_class, instanceTy)
.highlight(base->getSourceRange());
auto ctor = cast<ConstructorDecl>(ctorDecl);
tc.diagnose(ctorDecl, diag::note_nonrequired_initializer,
ctor->isImplicit(), ctor->getFullName());
// Constructors cannot be called on a protocol metatype, because there is no
// metatype to witness it.
} else if (isa<ConstructorDecl>(ctorDecl) &&
baseTy->is<MetatypeType>() &&
instanceTy->isExistentialType()) {
if (SuppressDiagnostics)
return false;

if (isStaticallyDerived) {
tc.diagnose(memberRefLoc, diag::construct_protocol_by_name, instanceTy)
.highlight(base->getSourceRange());
} else {
tc.diagnose(memberRefLoc, diag::construct_protocol_value, baseTy)
.highlight(base->getSourceRange());
}
}
return true;
}

namespace {

/// \brief Rewrites an expression by applying the solution of a constraint
Expand Down Expand Up @@ -825,12 +904,12 @@ namespace {
if (auto baseMeta = baseTy->getAs<AnyMetatypeType>()) {
baseIsInstance = false;
baseTy = baseMeta->getInstanceType();

// If the member is a constructor, verify that it can be legally
// referenced from this base.
if (auto ctor = dyn_cast<ConstructorDecl>(member)) {
cs.setExprTypes(base);
if (!tc.diagnoseInvalidDynamicConstructorReferences(base, memberLoc,
baseMeta, ctor, SuppressDiagnostics))
if (!diagnoseInvalidDynamicConstructorReferences(cs, base, memberLoc,
ctor, SuppressDiagnostics))
return nullptr;
}
}
Expand Down Expand Up @@ -2461,6 +2540,13 @@ namespace {
ConstructorDecl *ctor,
FunctionRefKind functionRefKind,
Type openedType) {

// If the member is a constructor, verify that it can be legally
// referenced from this base.
if (!diagnoseInvalidDynamicConstructorReferences(cs, base, nameLoc,
ctor, SuppressDiagnostics))
return nullptr;

// If the subexpression is a metatype, build a direct reference to the
// constructor.
if (cs.getType(base)->is<AnyMetatypeType>()) {
Expand Down Expand Up @@ -6805,67 +6891,6 @@ Expr *ExprRewriter::convertLiteralInPlace(Expr *literal,
return literal;
}

/// Determine whether the given type refers to a non-final class (or
/// dynamic self of one).
static bool isNonFinalClass(Type type) {
if (auto dynamicSelf = type->getAs<DynamicSelfType>())
type = dynamicSelf->getSelfType();

if (auto classDecl = type->getClassOrBoundGenericClass())
return !classDecl->isFinal();

if (auto archetype = type->getAs<ArchetypeType>())
if (auto super = archetype->getSuperclass())
return isNonFinalClass(super);

return false;
}

// Non-required constructors may not be not inherited. Therefore when
// constructing a class object, either the metatype must be statically
// derived (rather than an arbitrary value of metatype type) or the referenced
// constructor must be required.
bool
TypeChecker::diagnoseInvalidDynamicConstructorReferences(Expr *base,
DeclNameLoc memberRefLoc,
AnyMetatypeType *metaTy,
ConstructorDecl *ctorDecl,
bool SuppressDiagnostics) {
auto ty = metaTy->getInstanceType();

// FIXME: The "hasClangNode" check here is a complete hack.
if (isNonFinalClass(ty) &&
!base->isStaticallyDerivedMetatype() &&
!ctorDecl->hasClangNode() &&
!(ctorDecl->isRequired() ||
ctorDecl->getDeclContext()->getAsProtocolOrProtocolExtensionContext())) {
if (SuppressDiagnostics)
return false;

diagnose(memberRefLoc, diag::dynamic_construct_class, ty)
.highlight(base->getSourceRange());
auto ctor = cast<ConstructorDecl>(ctorDecl);
diagnose(ctorDecl, diag::note_nonrequired_initializer,
ctor->isImplicit(), ctor->getFullName());
// Constructors cannot be called on a protocol metatype, because there is no
// metatype to witness it.
} else if (isa<ConstructorDecl>(ctorDecl) &&
isa<MetatypeType>(metaTy) &&
ty->isExistentialType()) {
if (SuppressDiagnostics)
return false;

if (base->isStaticallyDerivedMetatype()) {
diagnose(memberRefLoc, diag::construct_protocol_by_name, ty)
.highlight(base->getSourceRange());
} else {
diagnose(memberRefLoc, diag::construct_protocol_value, metaTy)
.highlight(base->getSourceRange());
}
}
return true;
}

Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
ConstraintLocatorBuilder locator) {
TypeChecker &tc = cs.getTypeChecker();
Expand Down
9 changes: 0 additions & 9 deletions lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -2483,15 +2483,6 @@ class TypeChecker final : public LazyResolver {
/// Diagnose assigning variable to itself.
bool diagnoseSelfAssignment(const Expr *E);

/// When referencing a class initializer, check that the base expression is
/// either a static metatype or that the initializer is 'required'.
bool
diagnoseInvalidDynamicConstructorReferences(Expr *base,
DeclNameLoc memberRefLoc,
AnyMetatypeType *metaTy,
ConstructorDecl *ctorDecl,
bool SuppressDiagnostics);

/// Builds a string representing a "default" generic argument list for
/// \p typeDecl. In general, this means taking the bound of each generic
/// parameter. The \p getPreferredType callback can be used to provide a
Expand Down
2 changes: 1 addition & 1 deletion test/SILGen/protocol_extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ extension ObjCInitRequirement {
// rdar://problem/21370992 - delegation from an initializer in a
// protocol extension to an @objc initializer in a class.
class ObjCInitClass {
@objc init() { }
@objc required init() { }
}

protocol ProtoDelegatesToObjC { }
Expand Down
16 changes: 16 additions & 0 deletions test/decl/ext/protocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,22 @@ extension ExtendedProtocol where Self : DerivedWithAlias {
func f4(x: NestedNominal) {}
}

// rdar://problem/21991470 & https://bugs.swift.org/browse/SR-5022
class NonPolymorphicInit {
init() { } // expected-note {{selected non-required initializer 'init()'}}
}

protocol EmptyProtocol { }

// The diagnostic is not very accurate, but at least we reject this.

extension EmptyProtocol where Self : NonPolymorphicInit {
init(string: String) {
self.init()
// expected-error@-1 {{constructing an object of class type 'Self' with a metatype value must use a 'required' initializer}}
}
}

// ----------------------------------------------------------------------------
// Using protocol extensions to satisfy requirements
// ----------------------------------------------------------------------------
Expand Down