[ConstraintSystem] Track missing members via "holes"

xedin · xedin · commit fa9c3f3a10c2 · 2019-08-22T17:25:05.000-07:00
Replace specialized `MissingMembers` tracking with more general
constraint system "holes" which simplifies solver logic.
diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp
@@ -2834,18 +2834,6 @@ bool MissingMemberFailure::diagnoseAsError() {
   if (!anchor || !baseExpr)
     return false;
 
-  if (auto *typeVar = getBaseType()->getAs<TypeVariableType>()) {
-    auto &CS = getConstraintSystem();
-    auto *memberLoc = typeVar->getImpl().getLocator();
-    // Don't try to diagnose anything besides first missing
-    // member in the chain. e.g. `x.foo().bar()` let's make
-    // sure to diagnose only `foo()` as missing because we
-    // don't really know much about what `bar()` is supposed
-    // to be.
-    if (CS.MissingMembers.count(memberLoc))
-      return false;
-  }
-
   auto baseType = resolveType(getBaseType())->getWithoutSpecifierType();
 
   DeclNameLoc nameLoc(anchor->getStartLoc());
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -2177,6 +2177,9 @@ static ConstraintFix *fixPropertyWrapperFailure(
     if (!decl->hasValidSignature() || !type)
       return nullptr;
 
+    if (baseTy->isEqual(type))
+      return nullptr;
+
     if (!attemptFix(resolvedOverload, decl, type))
       return nullptr;
 
@@ -5143,12 +5146,33 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
   // If the lookup found no hits at all (either viable or unviable), diagnose it
   // as such and try to recover in various ways.
   if (shouldAttemptFixes()) {
-    // Let's record missing member in constraint system, this helps to prevent
-    // stacking up fixes for the same member, because e.g. if its base was of
-    // optional type, we'd re-introduce member constraint with optional stripped
-    // off to see if the problem is related to base not being explicitly unwrapped.
-    if (!MissingMembers.insert(locator))
-      return SolutionKind::Error;
+    auto fixMissingMember = [&](Type baseTy, Type memberTy,
+                                ConstraintLocator *locator) -> SolutionKind {
+      // Let's check whether there are any generic parameters
+      // associated with base type, we'd have to default them
+      // to `Any` and record as potential holes if so.
+      baseTy.transform([&](Type type) -> Type {
+        if (auto *typeVar = type->getAs<TypeVariableType>()) {
+          if (typeVar->getImpl().hasRepresentativeOrFixed())
+            return type;
+          recordHole(typeVar);
+        }
+        return type;
+      });
+
+      auto *fix =
+          DefineMemberBasedOnUse::create(*this, baseTy, member, locator);
+      if (recordFix(fix))
+        return SolutionKind::Error;
+
+      // Allow member type to default to `Any` to make it possible to form
+      // solutions when contextual type of the result cannot be deduced e.g.
+      // `let _ = x.foo`.
+      if (auto *memberTypeVar = memberTy->getAs<TypeVariableType>())
+        recordHole(memberTypeVar);
+
+      return SolutionKind::Solved;
+    };
 
     if (baseObjTy->getOptionalObjectType()) {
       // If the base type was an optional, look through it.
@@ -5170,6 +5194,18 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
         }
       }
 
+      // Let's check whether the problem is related to optionality of base
+      // type, or there is no member with a given name.
+      result =
+          performMemberLookup(kind, member, baseObjTy->getOptionalObjectType(),
+                              functionRefKind, locator,
+                              /*includeInaccessibleMembers*/ true);
+
+      // If uwrapped type still couldn't find anything for a given name,
+      // let's fallback to a "not such member" fix.
+      if (result.ViableCandidates.empty() && result.UnviableCandidates.empty())
+        return fixMissingMember(origBaseTy, memberTy, locator);
+
       // The result of the member access can either be the expected member type
       // (for '!' or optional members with '?'), or the original member type
       // with one extra level of optionality ('?' with non-optional members).
@@ -5197,26 +5233,27 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
       return SolutionKind::Solved;
     }
 
-    auto solveWithNewBaseOrName = [&](Type baseType, DeclName memberName,
-                                      bool allowFixes = true) -> SolutionKind {
-      // Let's re-enable fixes for this member, because
-      // the base or member name has been changed.
-      if (allowFixes)
-        MissingMembers.remove(locator);
+    auto solveWithNewBaseOrName = [&](Type baseType,
+                                      DeclName memberName) -> SolutionKind {
       return simplifyMemberConstraint(kind, baseType, memberName, memberTy,
                                       useDC, functionRefKind, outerAlternatives,
-                                      flags, locatorB);
+                                      flags | TMF_ApplyingFix, locatorB);
     };
 
+    // If this member reference is a result of a previous fix, let's not allow
+    // any more fixes expect when base is optional, because it could also be
+    // an IUO which requires a separate fix.
+    if (flags.contains(TMF_ApplyingFix))
+      return SolutionKind::Error;
+
     // Check if any property wrappers on the base of the member lookup have
     // matching members that we can fall back to, or if the type wraps any
     // properties that have matching members.
     if (auto *fix = fixPropertyWrapperFailure(
             *this, baseTy, locator,
             [&](ResolvedOverloadSetListItem *overload, VarDecl *decl,
                 Type newBase) {
-              return solveWithNewBaseOrName(newBase, member,
-                                            /*allowFixes=*/false) ==
+              return solveWithNewBaseOrName(newBase, member) ==
                      SolutionKind::Solved;
             })) {
       return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
@@ -5283,38 +5320,20 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
     // fake its presence based on use, that makes it possible to diagnose
     // problems related to member lookup more precisely.
 
-    auto defaultTypeVariableToAny = [&](TypeVariableType *typeVar) {
-      // Recording this generic parameter is a potential "hole" in
-      // the constraint system.
-      recordHole(typeVar);
-      // Default all of the generic parameters found in base to `Any`.
-      addConstraint(ConstraintKind::Defaultable, typeVar,
-                    getASTContext().TheAnyType,
-                    typeVar->getImpl().getLocator());
-    };
-
-    origBaseTy.transform([&](Type type) -> Type {
-      if (auto *typeVar = type->getAs<TypeVariableType>()) {
-        if (typeVar->getImpl().hasRepresentativeOrFixed())
-          return type;
-
-        defaultTypeVariableToAny(typeVar);
+    // If base type is a "hole" there is no reason to record any
+    // more "member not found" fixes for chained member references.
+    if (auto *baseType = origBaseTy->getMetatypeInstanceType()
+                             ->getRValueType()
+                             ->getAs<TypeVariableType>()) {
+      if (isHole(baseType)) {
+        increaseScore(SK_Fix);
+        if (auto *memberTypeVar = memberTy->getAs<TypeVariableType>())
+          recordHole(memberTypeVar);
+        return SolutionKind::Solved;
       }
-      return type;
-    });
-
-    auto *fix =
-        DefineMemberBasedOnUse::create(*this, origBaseTy, member, locator);
-    if (recordFix(fix))
-      return SolutionKind::Error;
-
-    // Allow member type to default to `Any` to make it possible to form
-    // solutions when contextual type of the result cannot be deduced e.g.
-    // `let _ = x.foo`.
-    if (auto *memberTypeVar = memberTy->getAs<TypeVariableType>())
-      defaultTypeVariableToAny(memberTypeVar);
+    }
 
-    return SolutionKind::Solved;
+    return fixMissingMember(origBaseTy, memberTy, locator);
   }
   return SolutionKind::Error;
 }
@@ -6260,18 +6279,19 @@ ConstraintSystem::simplifyApplicableFnConstraint(
   // Let's check if this member couldn't be found and is fixed
   // to exist based on its usage.
   if (auto *memberTy = type2->getAs<TypeVariableType>()) {
-    auto *locator = memberTy->getImpl().getLocator();
-    if (MissingMembers.count(locator)) {
+    if (isHole(memberTy)) {
       auto *funcTy = type1->castTo<FunctionType>();
+      auto *locator = memberTy->getImpl().getLocator();
       // Bind type variable associated with member to a type of argument
       // application, which makes it seem like member exists with the
       // types of the parameters matching argument types exactly.
       addConstraint(ConstraintKind::Bind, memberTy, funcTy, locator);
       // There might be no contextual type for result of the application,
       // in cases like `let _ = x.foo()`, so let's default result to `Any`
       // to make expressions like that type-check.
-      addConstraint(ConstraintKind::Defaultable, funcTy->getResult(),
-                    getASTContext().TheAnyType, locator);
+      auto resultTy = funcTy->getResult();
+      if (auto *typeVar = resultTy->getAs<TypeVariableType>())
+        recordHole(typeVar);
       return SolutionKind::Solved;
     }
   }
@@ -7174,6 +7194,15 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix) {
   return false;
 }
 
+void ConstraintSystem::recordHole(TypeVariableType *typeVar) {
+  assert(typeVar);
+  auto *locator = typeVar->getImpl().getLocator();
+  if (Holes.insert(locator)) {
+    addConstraint(ConstraintKind::Defaultable, typeVar,
+                  getASTContext().TheAnyType, locator);
+  }
+}
+
 ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
     ConstraintFix *fix, Type type1, Type type2, ConstraintKind matchKind,
     TypeMatchOptions flags, ConstraintLocatorBuilder locator) {
diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp
@@ -125,12 +125,6 @@ Solution ConstraintSystem::finalize() {
   }
   solution.Fixes.append(Fixes.begin() + firstFixIndex, Fixes.end());
 
-  // Remember all of the missing member references encountered,
-  // that helps diagnostics to avoid emitting error for each
-  // member in the chain.
-  for (auto *member : MissingMembers)
-    solution.MissingMembers.push_back(member);
-
   // Remember all the disjunction choices we made.
   for (auto &choice : DisjunctionChoices) {
     // We shouldn't ever register disjunction choices multiple times,
@@ -267,10 +261,6 @@ void ConstraintSystem::applySolution(const Solution &solution) {
     
   // Register any fixes produced along this path.
   Fixes.append(solution.Fixes.begin(), solution.Fixes.end());
-
-  // Register any missing members encountered along this path.
-  MissingMembers.insert(solution.MissingMembers.begin(),
-                        solution.MissingMembers.end());
 }
 
 /// Restore the type variable bindings to what they were before
@@ -460,7 +450,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs)
   numDefaultedConstraints = cs.DefaultedConstraints.size();
   numAddedNodeTypes = cs.addedNodeTypes.size();
   numCheckedConformances = cs.CheckedConformances.size();
-  numMissingMembers = cs.MissingMembers.size();
   numDisabledConstraints = cs.solverState->getNumDisabledConstraints();
   numFavoredConstraints = cs.solverState->getNumFavoredConstraints();
   numBuilderTransformedClosures = cs.builderTransformedClosures.size();
@@ -529,9 +518,6 @@ ConstraintSystem::SolverScope::~SolverScope() {
   // Remove any conformances checked along the current path.
   truncate(cs.CheckedConformances, numCheckedConformances);
 
-  // Remove any missing members found along the current path.
-  truncate(cs.MissingMembers, numMissingMembers);
-
   /// Remove any builder transformed closures.
   truncate(cs.builderTransformedClosures, numBuilderTransformedClosures);
 
diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h
@@ -634,9 +634,6 @@ class Solution {
   /// The list of fixes that need to be applied to the initial expression
   /// to make the solution work.
   llvm::SmallVector<ConstraintFix *, 4> Fixes;
-  /// The list of member references which couldn't be resolved,
-  /// and had to be assumed based on their use.
-  llvm::SmallVector<ConstraintLocator *, 4> MissingMembers;
 
   /// The set of disjunction choices used to arrive at this solution,
   /// which informs constraint application.
@@ -1103,9 +1100,6 @@ class ConstraintSystem {
   /// parameter which hasn't been explicitly specified.
   llvm::SmallSetVector<ConstraintLocator *, 4> Holes;
 
-  /// The set of type variables representing types of missing members.
-  llvm::SmallSetVector<ConstraintLocator *, 4> MissingMembers;
-
   /// The set of remembered disjunction choices used to reach
   /// the current constraint system.
   std::vector<std::pair<ConstraintLocator*, unsigned>>
@@ -1661,8 +1655,6 @@ class ConstraintSystem {
 
     unsigned numCheckedConformances;
 
-    unsigned numMissingMembers;
-
     unsigned numDisabledConstraints;
 
     unsigned numFavoredConstraints;
@@ -2045,9 +2037,7 @@ class ConstraintSystem {
   /// subsequent solution would be worse than the best known solution.
   bool recordFix(ConstraintFix *fix);
 
-  void recordHole(TypeVariableType *typeVar) {
-    Holes.insert(typeVar->getImpl().getLocator());
-  }
+  void recordHole(TypeVariableType *typeVar);
 
   bool isHole(TypeVariableType *typeVar) const {
     return isHoleAt(typeVar->getImpl().getLocator());
