Merge pull request #14097 from DougGregor/sr-5022-4.1

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

Author: AnnaZaks

lib/Sema/CSApply.cpp

@@ -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
@@ -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;
         }
       }
@@ -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>()) {
@@ -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();

lib/Sema/TypeChecker.h

@@ -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

test/SILGen/protocol_extensions.swift

@@ -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 { }

test/decl/ext/protocol.swift

@@ -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
 // ----------------------------------------------------------------------------