Merge pull request #22379 from xedin/rdar-47787705

[ConstraintSystem] Detect invalid initializer references early

Author: xedin

include/swift/AST/Expr.h

@@ -466,17 +466,21 @@ class alignas(8) Expr {
   /// This distinguishes static references to types, like Int, from metatype
   /// values, "someTy: Any.Type".
   bool isTypeReference(llvm::function_ref<Type(const Expr *)> getType =
-                           [](const Expr *E) -> Type {
-    return E->getType();
-  }) const;
+                           [](const Expr *E) -> Type { return E->getType(); },
+                       llvm::function_ref<Decl *(const Expr *)> getDecl =
+                           [](const Expr *E) -> Decl * {
+                         return nullptr;
+                       }) const;
 
   /// Determine whether this expression refers to a statically-derived metatype.
   ///
   /// This implies `isTypeReference`, but also requires that the referenced type
   /// is not an archetype or dependent type.
   bool isStaticallyDerivedMetatype(
       llvm::function_ref<Type(const Expr *)> getType =
-          [](const Expr *E) -> Type { return E->getType(); }) const;
+          [](const Expr *E) -> Type { return E->getType(); },
+      llvm::function_ref<bool(const Expr *)> isTypeReference =
+          [](const Expr *E) { return E->isTypeReference(); }) const;
 
   /// isImplicit - Determines whether this expression was implicitly-generated,
   /// rather than explicitly written in the AST.

lib/AST/Expr.cpp

@@ -440,7 +440,8 @@ void Expr::forEachChildExpr(llvm::function_ref<Expr *(Expr *)> callback) {
 }
 
 bool Expr::isTypeReference(
-    llvm::function_ref<Type(const Expr *)> getType) const {
+    llvm::function_ref<Type(const Expr *)> getType,
+    llvm::function_ref<Decl *(const Expr *)> getDecl) const {
   // If the result isn't a metatype, there's nothing else to do.
   if (!getType(this)->is<AnyMetatypeType>())
     return false;
@@ -461,6 +462,12 @@ bool Expr::isTypeReference(
     if (auto memberRef = dyn_cast<MemberRefExpr>(expr))
       return isa<TypeDecl>(memberRef->getMember().getDecl());
 
+    // Any other expressions which might be referencing
+    // a declaration e.g. not yet type-checked ones like
+    // `UnresolvedDotExpr`.
+    if (auto *decl = getDecl(expr))
+      return isa<TypeDecl>(decl);
+
     // When the base of a "." expression is ignored, look at the member.
     if (auto ignoredDot = dyn_cast<DotSyntaxBaseIgnoredExpr>(expr)) {
       expr = ignoredDot->getRHS();
@@ -470,13 +477,13 @@ bool Expr::isTypeReference(
     // Anything else is not statically derived.
     return false;
   } while (true);
-
 }
 
 bool Expr::isStaticallyDerivedMetatype(
-    llvm::function_ref<Type(const Expr *)> getType) const {
+    llvm::function_ref<Type(const Expr *)> getType,
+    llvm::function_ref<bool(const Expr *)> isTypeReference) const {
   // The expression must first be a type reference.
-  if (!isTypeReference(getType))
+  if (!isTypeReference(this))
     return false;
 
   auto type = getType(this)

lib/Sema/CSApply.cpp

@@ -227,13 +227,31 @@ getImplicitMemberReferenceAccessSemantics(Expr *base, VarDecl *member,
   return member->getAccessSemanticsFromContext(DC, isAccessOnSelf);
 }
 
+/// This extends functionality of `Expr::isTypeReference` with
+/// support for `UnresolvedDotExpr` and `UnresolvedMemberExpr`.
+/// This method could be used on not yet fully type-checked AST.
 bool ConstraintSystem::isTypeReference(const Expr *E) {
-  return E->isTypeReference([&](const Expr *E) -> Type { return getType(E); });
+  return E->isTypeReference(
+      [&](const Expr *E) -> Type { return getType(E); },
+      [&](const Expr *E) -> Decl * {
+        if (auto *UDE = dyn_cast<UnresolvedDotExpr>(E)) {
+          return findResolvedMemberRef(
+              getConstraintLocator(UDE, ConstraintLocator::Member));
+        }
+
+        if (auto *UME = dyn_cast<UnresolvedMemberExpr>(E)) {
+          return findResolvedMemberRef(
+              getConstraintLocator(UME, ConstraintLocator::UnresolvedMember));
+        }
+
+        return nullptr;
+      });
 }
 
 bool ConstraintSystem::isStaticallyDerivedMetatype(const Expr *E) {
   return E->isStaticallyDerivedMetatype(
-      [&](const Expr *E) -> Type { return getType(E); });
+      [&](const Expr *E) -> Type { return getType(E); },
+      [&](const Expr *E) -> bool { return isTypeReference(E); });
 }
 
 Type ConstraintSystem::getInstanceType(const TypeExpr *E) {
@@ -298,78 +316,6 @@ 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);
-
-  if (type->isExistentialType())
-    return true;
-
-  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->getMetatypeInstanceType();
-
-  bool isStaticallyDerived =
-    base->isStaticallyDerivedMetatype(
-      [&](const Expr *expr) -> Type {
-        return cs.getType(expr);
-      });
-
-  // FIXME: The "hasClangNode" check here is a complete hack.
-  if (isNonFinalClass(instanceTy) &&
-      !isStaticallyDerived &&
-      !ctorDecl->hasClangNode() &&
-      !(ctorDecl->isRequired() ||
-        ctorDecl->getDeclContext()->getSelfProtocolDecl())) {
-    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;
-}
-
 /// Form a type checked expression for the index of a @dynamicMemberLookup
 /// subscript index parameter.
 /// The index expression will have a tuple type of `(dynamicMember: T)`.
@@ -877,14 +823,6 @@ 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)) {
-          if (!diagnoseInvalidDynamicConstructorReferences(cs, base, memberLoc,
-                                           ctor, SuppressDiagnostics))
-            return nullptr;
-        }
       }
 
       // Build a member reference.
@@ -2634,18 +2572,6 @@ namespace {
           if (dre->getDecl()->getFullName() == cs.getASTContext().Id_self) {
             // We have a reference to 'self'.
             diagnoseBadInitRef = false;
-
-            // Special case -- in a protocol extension initializer with a class
-            // constrainted Self type, 'self' has archetype type, and only
-            // required initializers can be called.
-            if (cs.getType(dre)->getRValueType()->is<ArchetypeType>()) {
-              if (!diagnoseInvalidDynamicConstructorReferences(cs, base,
-                                                               nameLoc,
-                                                               ctor,
-                                                               SuppressDiagnostics))
-                return nullptr;
-            }
-
             // Make sure the reference to 'self' occurs within an initializer.
             if (!dyn_cast_or_null<ConstructorDecl>(
                    cs.DC->getInnermostMethodContext())) {

lib/Sema/CSDiagnostics.cpp

@@ -1506,3 +1506,27 @@ bool PartialApplicationFailure::diagnoseAsError() {
   emitDiagnostic(anchor->getNameLoc(), diagnostic, kind);
   return true;
 }
+
+bool InvalidDynamicInitOnMetatypeFailure::diagnoseAsError() {
+  auto *anchor = getRawAnchor();
+  emitDiagnostic(anchor->getLoc(), diag::dynamic_construct_class,
+                 BaseType->getMetatypeInstanceType())
+      .highlight(BaseRange);
+  emitDiagnostic(Init, diag::note_nonrequired_initializer, Init->isImplicit(),
+                 Init->getFullName());
+  return true;
+}
+
+bool InitOnProtocolMetatypeFailure::diagnoseAsError() {
+  auto *anchor = getRawAnchor();
+  if (IsStaticallyDerived) {
+    emitDiagnostic(anchor->getLoc(), diag::construct_protocol_by_name,
+                   BaseType->getMetatypeInstanceType())
+        .highlight(BaseRange);
+  } else {
+    emitDiagnostic(anchor->getLoc(), diag::construct_protocol_value, BaseType)
+        .highlight(BaseRange);
+  }
+
+  return true;
+}

lib/Sema/CSDiagnostics.h

@@ -707,6 +707,70 @@ class PartialApplicationFailure final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+class InvalidInitRefFailure : public FailureDiagnostic {
+protected:
+  Type BaseType;
+  const ConstructorDecl *Init;
+  SourceRange BaseRange;
+
+  InvalidInitRefFailure(Expr *root, ConstraintSystem &cs, Type baseTy,
+                        const ConstructorDecl *init, SourceRange baseRange,
+                        ConstraintLocator *locator)
+      : FailureDiagnostic(root, cs, locator), BaseType(baseTy), Init(init),
+        BaseRange(baseRange) {}
+
+public:
+  bool diagnoseAsError() override = 0;
+};
+
+/// Diagnose an attempt to construct an object of class type with a metatype
+/// value without using 'required' initializer:
+///
+/// ```swift
+///  class C {
+///    init(value: Int) {}
+///  }
+///
+///  func make<T: C>(type: T.Type) -> T {
+///    return T.init(value: 42)
+///  }
+/// ```
+class InvalidDynamicInitOnMetatypeFailure final : public InvalidInitRefFailure {
+public:
+  InvalidDynamicInitOnMetatypeFailure(Expr *root, ConstraintSystem &cs,
+                                      Type baseTy, const ConstructorDecl *init,
+                                      SourceRange baseRange,
+                                      ConstraintLocator *locator)
+      : InvalidInitRefFailure(root, cs, baseTy, init, baseRange, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
+/// Diagnose an attempt to call initializer on protocol metatype:
+///
+/// ```swift
+///  protocol P {
+///    init(value: Int)
+///  }
+///
+///  func make(type: P.Type) -> P {
+///    return type.init(value: 42)
+///  }
+/// ```
+class InitOnProtocolMetatypeFailure final : public InvalidInitRefFailure {
+  bool IsStaticallyDerived;
+
+public:
+  InitOnProtocolMetatypeFailure(Expr *root, ConstraintSystem &cs, Type baseTy,
+                                const ConstructorDecl *init,
+                                bool isStaticallyDerived, SourceRange baseRange,
+                                ConstraintLocator *locator)
+      : InvalidInitRefFailure(root, cs, baseTy, init, baseRange, locator),
+        IsStaticallyDerived(isStaticallyDerived) {}
+
+  bool diagnoseAsError() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSFix.cpp

@@ -20,6 +20,7 @@
 #include "CSDiagnostics.h"
 #include "ConstraintLocator.h"
 #include "ConstraintSystem.h"
+#include "OverloadChoice.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/Type.h"
 #include "swift/AST/Types.h"
@@ -274,3 +275,43 @@ AllowInvalidPartialApplication::create(bool isWarning, ConstraintSystem &cs,
   return new (cs.getAllocator())
       AllowInvalidPartialApplication(isWarning, cs, locator);
 }
+
+bool AllowInvalidInitRef::diagnose(Expr *root, bool asNote) const {
+  switch (Kind) {
+  case RefKind::DynamicOnMetatype: {
+    InvalidDynamicInitOnMetatypeFailure failure(
+        root, getConstraintSystem(), BaseType, Init, BaseRange, getLocator());
+    return failure.diagnose(asNote);
+  }
+
+  case RefKind::ProtocolMetatype: {
+    InitOnProtocolMetatypeFailure failure(root, getConstraintSystem(), BaseType,
+                                          Init, IsStaticallyDerived, BaseRange,
+                                          getLocator());
+    return failure.diagnose(asNote);
+  }
+  }
+}
+
+AllowInvalidInitRef *AllowInvalidInitRef::dynamicOnMetatype(
+    ConstraintSystem &cs, Type baseTy, ConstructorDecl *init,
+    SourceRange baseRange, ConstraintLocator *locator) {
+  return create(RefKind::DynamicOnMetatype, cs, baseTy, init,
+                /*isStaticallyDerived=*/false, baseRange, locator);
+}
+
+AllowInvalidInitRef *AllowInvalidInitRef::onProtocolMetatype(
+    ConstraintSystem &cs, Type baseTy, ConstructorDecl *init,
+    bool isStaticallyDerived, SourceRange baseRange,
+    ConstraintLocator *locator) {
+  return create(RefKind::ProtocolMetatype, cs, baseTy, init,
+                isStaticallyDerived, baseRange, locator);
+}
+
+AllowInvalidInitRef *
+AllowInvalidInitRef::create(RefKind kind, ConstraintSystem &cs, Type baseTy,
+                            ConstructorDecl *init, bool isStaticallyDerived,
+                            SourceRange baseRange, ConstraintLocator *locator) {
+  return new (cs.getAllocator()) AllowInvalidInitRef(
+      cs, kind, baseTy, init, isStaticallyDerived, baseRange, locator);
+}