[ConstraintSystem] Detect invalid initializer references early

Currently invalid initializer references are detected and
diagnosed in solution application phase, but that's too
late because solver wouldn't have required information while
attempting to determine the best solution, which might result
in viable solutions being ignored in favour of incorrect ones e.g.

```swift
protocol P {
  init(value: Int)
}

class C {
  init(value: Int, _: String = "") {}
}

func make<T: P & C>(type: T.Type) -> T {
  return T.init(value: 0)
}
```

In this example `init` on `C` would be preferred since it
comes from the concrete type, but reference itself is invalid
because it's an attempt to construct class object using
metatype value via non-required initalizer.

Situations like these should be recognized early and invalid
use like in case of `C.init` should be ranked lower or diagnosed
if that is the only possible solution.

Resolves: rdar://problem/47787705

Author: xedin

include/swift/AST/Expr.h

@@ -476,7 +476,9 @@ class alignas(8) Expr {
   /// 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

@@ -474,9 +474,10 @@ bool Expr::isTypeReference(
 }
 
 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,57 @@ getImplicitMemberReferenceAccessSemantics(Expr *base, VarDecl *member,
   return member->getAccessSemanticsFromContext(DC, isAccessOnSelf);
 }
 
+/// This effectively duplicates logic of `Expr::isTypeReference` with
+/// one significant difference - 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); });
+  // If the result isn't a metatype, there's nothing else to do.
+  if (!getType(E)->is<AnyMetatypeType>())
+    return false;
+
+  Expr *expr = const_cast<Expr *>(E);
+  do {
+    // Skip syntax.
+    expr = expr->getSemanticsProvidingExpr();
+
+    // Direct reference to a type.
+    if (auto declRef = dyn_cast<DeclRefExpr>(expr))
+      if (isa<TypeDecl>(declRef->getDecl()))
+        return true;
+
+    if (isa<TypeExpr>(expr))
+      return true;
+
+    if (auto *UDE = dyn_cast<UnresolvedDotExpr>(expr)) {
+      return isa<TypeDecl>(findResolvedMemberRef(
+          getConstraintLocator(UDE, ConstraintLocator::Member)));
+    }
+
+    if (auto *UME = dyn_cast<UnresolvedMemberExpr>(expr)) {
+      return isa<TypeDecl>(findResolvedMemberRef(
+          getConstraintLocator(UME, ConstraintLocator::UnresolvedMember)));
+    }
+
+    // A "." expression that refers to a member.
+    if (auto memberRef = dyn_cast<MemberRefExpr>(expr))
+      return isa<TypeDecl>(memberRef->getMember().getDecl());
+
+    // When the base of a "." expression is ignored, look at the member.
+    if (auto ignoredDot = dyn_cast<DotSyntaxBaseIgnoredExpr>(expr)) {
+      expr = ignoredDot->getRHS();
+      continue;
+    }
+
+    // Anything else is not statically derived.
+    return false;
+  } while (true);
 }
 
 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) {
@@ -317,10 +361,6 @@ static bool isNonFinalClass(Type type) {
   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,
@@ -877,14 +917,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.
@@ -2638,18 +2670,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/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,30 @@ AllowInvalidPartialApplication::create(bool isWarning, ConstraintSystem &cs,
   return new (cs.getAllocator())
       AllowInvalidPartialApplication(isWarning, cs, locator);
 }
+
+bool AllowInvalidInitRef::diagnose(Expr *root, bool asNote) const {
+  return false;
+}
+
+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);
+}

lib/Sema/CSFix.h

@@ -37,6 +37,7 @@ class SourceManager;
 
 namespace constraints {
 
+class OverloadChoice;
 class ConstraintSystem;
 class ConstraintLocator;
 class Solution;
@@ -112,6 +113,12 @@ enum class FixKind : uint8_t {
   /// Allow expressions where initializer call (either `self.init` or
   /// `super.init`) is only partially applied.
   AllowInvalidPartialApplication,
+
+  /// 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.
+  AllowInvalidInitRef,
 };
 
 class ConstraintFix {
@@ -535,6 +542,45 @@ class AllowInvalidPartialApplication final : public ConstraintFix {
                                                 ConstraintLocator *locator);
 };
 
+class AllowInvalidInitRef final : public ConstraintFix {
+  enum class RefKind { DynamicOnMetatype, ProtocolMetatype } Kind;
+
+  Type BaseType;
+  const ConstructorDecl *Init;
+  bool IsStaticallyDerived;
+  SourceRange BaseRange;
+
+  AllowInvalidInitRef(ConstraintSystem &cs, RefKind kind, Type baseTy,
+                      ConstructorDecl *init, bool isStaticallyDerived,
+                      SourceRange baseRange, ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::AllowInvalidInitRef, locator), Kind(kind),
+        BaseType(baseTy), Init(init), IsStaticallyDerived(isStaticallyDerived),
+        BaseRange(baseRange) {}
+
+public:
+  std::string getName() const override {
+    return "allow invalid initializer reference";
+  }
+
+  bool diagnose(Expr *root, bool asNote = false) const override;
+
+  static AllowInvalidInitRef *
+  dynamicOnMetatype(ConstraintSystem &cs, Type baseTy, ConstructorDecl *init,
+                    SourceRange baseRange, ConstraintLocator *locator);
+
+  static AllowInvalidInitRef *
+  onProtocolMetatype(ConstraintSystem &cs, Type baseTy, ConstructorDecl *init,
+                     bool isStaticallyDerived, SourceRange baseRange,
+                     ConstraintLocator *locator);
+
+private:
+  static AllowInvalidInitRef *create(RefKind kind, ConstraintSystem &cs,
+                                     Type baseTy, ConstructorDecl *init,
+                                     bool isStaticallyDerived,
+                                     SourceRange baseRange,
+                                     ConstraintLocator *locator);
+};
+
 } // end namespace constraints
 } // end namespace swift
 

lib/Sema/CSSimplify.cpp

@@ -5517,6 +5517,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::RemoveUnwrap:
   case FixKind::DefineMemberBasedOnUse:
   case FixKind::AllowInvalidPartialApplication:
+  case FixKind::AllowInvalidInitRef:
     llvm_unreachable("handled elsewhere");
   }
 

lib/Sema/ConstraintSystem.cpp

@@ -1703,6 +1703,129 @@ isInvalidPartialApplication(ConstraintSystem &cs, const ValueDecl *member,
   return {true, level};
 }
 
+/// 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;
+}
+
+/// Determine whether given constructor reference is valid or does it require
+/// any fixes e.g. when base is a protocol metatype.
+static void validateInitializerRef(ConstraintSystem &cs, ConstructorDecl *init,
+                                   ConstraintLocator *locator) {
+  auto *anchor = locator->getAnchor();
+  if (!anchor)
+    return;
+
+  auto getType = [&cs](const Expr *expr) -> Type {
+    return cs.simplifyType(cs.getType(expr))->getRValueType();
+  };
+
+  auto locatorEndsWith =
+      [](ConstraintLocator *locator,
+         ConstraintLocator::PathElementKind eltKind) -> bool {
+    auto path = locator->getPath();
+    return !path.empty() && path.back().getKind() == eltKind;
+  };
+
+  Expr *baseExpr = nullptr;
+  Type baseType;
+
+  // Explicit initializer reference e.g. `T.init(...)` or `T.init`.
+  if (auto *UDE = dyn_cast<UnresolvedDotExpr>(anchor)) {
+    baseExpr = UDE->getBase();
+    baseType = getType(baseExpr);
+  // Initializer call e.g. `T(...)`
+  } else if (auto *CE = dyn_cast<CallExpr>(anchor)) {
+    baseExpr = CE->getFn();
+    baseType = getType(baseExpr);
+  // Initializer reference which requires contextual base type e.g. `.init(...)`.
+  } else if (auto *UME = dyn_cast<UnresolvedMemberExpr>(anchor)) {
+    // We need to find type variable which represents contextual base.
+    auto *baseLocator = cs.getConstraintLocator(
+        UME, locatorEndsWith(locator, ConstraintLocator::ConstructorMember)
+                 ? ConstraintLocator::UnresolvedMember
+                 : ConstraintLocator::MemberRefBase);
+
+    // FIXME: Type variables responsible for contextual base could be cached
+    // in the constraint system to speed up lookup.
+    auto result = llvm::find_if(
+        cs.getTypeVariables(), [&baseLocator](const TypeVariableType *typeVar) {
+          return typeVar->getImpl().getLocator() == baseLocator;
+        });
+
+    assert(result != cs.getTypeVariables().end());
+    baseType = cs.simplifyType(*result)->getRValueType();
+    // Constraint for member base is formed as '$T.Type[.<member] = ...`
+    // which means MetatypeType has to be added after finding a type variable.
+    if (locatorEndsWith(baseLocator, ConstraintLocator::MemberRefBase))
+      baseType = MetatypeType::get(baseType);
+  }
+
+  if (!baseType)
+    return;
+
+  if (!baseType->is<AnyMetatypeType>()) {
+    bool applicable = 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 (baseExpr && !baseExpr->isSuperExpr()) {
+      auto &ctx = cs.getASTContext();
+      if (auto *DRE =
+              dyn_cast<DeclRefExpr>(baseExpr->getSemanticsProvidingExpr())) {
+        if (DRE->getDecl()->getFullName() == ctx.Id_self) {
+          if (getType(DRE)->is<ArchetypeType>())
+            applicable = true;
+        }
+      }
+    }
+
+    if (!applicable)
+      return;
+  }
+
+  auto instanceType = baseType->getMetatypeInstanceType();
+  bool isStaticallyDerived = true;
+  // If this is expression like `.init(...)` where base type is
+  // determined by a contextual type.
+  if (!baseExpr) {
+    isStaticallyDerived = !(instanceType->is<DynamicSelfType>() ||
+                            instanceType->is<ArchetypeType>());
+  // Otherwise this is something like `T.init(...)`
+  } else {
+    isStaticallyDerived = cs.isStaticallyDerivedMetatype(baseExpr);
+  }
+
+  auto baseRange = baseExpr ? baseExpr->getSourceRange() : SourceRange();
+  // FIXME: The "hasClangNode" check here is a complete hack.
+  if (isNonFinalClass(instanceType) && !isStaticallyDerived &&
+      !init->hasClangNode() &&
+      !(init->isRequired() || init->getDeclContext()->getSelfProtocolDecl())) {
+    (void)cs.recordFix(AllowInvalidInitRef::dynamicOnMetatype(
+        cs, baseType, init, baseRange, locator));
+  // Constructors cannot be called on a protocol metatype, because there is no
+  // metatype to witness it.
+  } else if (baseType->is<MetatypeType>() &&
+             instanceType->isExistentialType()) {
+    (void)cs.recordFix(AllowInvalidInitRef::onProtocolMetatype(
+        cs, baseType, init, isStaticallyDerived, baseRange, locator));
+  }
+}
+
 void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
                                        Type boundType,
                                        OverloadChoice choice,
@@ -1955,6 +2078,8 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
         boundType = boundFunctionType->withExtInfo(
             boundFunctionType->getExtInfo().withThrows());
       }
+
+      validateInitializerRef(*this, CD, locator);
     }
 
     // Check whether applying this overload would result in invalid

test/Constraints/rdar47787705.swift

@@ -0,0 +1,14 @@
+// RUN: %target-swift-frontend -emit-sil -verify %s | %FileCheck %s
+
+protocol P {
+  init(value: Int)
+}
+
+class C {
+  init(value: Int, otherValue: String = "") {}
+}
+
+func make<T: P & C>(type: T.Type) -> T {
+  // CHECK: [[INIT:.*]] = witness_method $T, #P.init
+  return T.init(value: 0)
+}