Merge pull request #29000 from hborla/unresolved-member-expr-diagnostics

[ConstraintSystem] Finish porting unresolved member reference failures.

Author: hborla

lib/Sema/CSDiag.cpp

@@ -240,7 +240,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
   bool visitIdentityExpr(IdentityExpr *E);
   bool visitTryExpr(TryExpr *E);
 
-  bool visitUnresolvedMemberExpr(UnresolvedMemberExpr *E);
   bool visitUnresolvedDotExpr(UnresolvedDotExpr *UDE);
   bool visitArrayExpr(ArrayExpr *E);
   bool visitDictionaryExpr(DictionaryExpr *E);
@@ -2126,65 +2125,6 @@ bool FailureDiagnosis::visitObjectLiteralExpr(ObjectLiteralExpr *E) {
   return true;
 }
 
-bool FailureDiagnosis::visitUnresolvedMemberExpr(UnresolvedMemberExpr *E) {
-  // If we have no contextual type, there is no way to resolve this.  Just
-  // diagnose this as an ambiguity.
-  if (!CS.getContextualType())
-    return false;
-
-  // OTOH, if we do have a contextual type, we can provide a more specific
-  // error.  Dig out the UnresolvedValueMember constraint for this expr node.
-  Constraint *memberConstraint = nullptr;
-  auto checkConstraint = [&](Constraint *C) {
-    if (C->getKind() == ConstraintKind::UnresolvedValueMember &&
-        simplifyLocatorToAnchor(C->getLocator()) == E)
-      memberConstraint = C;
-  };
-
-  if (CS.failedConstraint)
-    checkConstraint(CS.failedConstraint);
-  for (auto &C : CS.getConstraints()) {
-    if (memberConstraint) break;
-    checkConstraint(&C);
-  }
-  
-  // If we can't find the member constraint in question, then we failed.
-  if (!memberConstraint)
-    return false;
-
-  std::function<bool(ArrayRef<OverloadChoice>)> callback = [&](
-      ArrayRef<OverloadChoice> candidates) {
-    bool hasTrailingClosure = callArgHasTrailingClosure(E->getArgument());
-
-    // Dump all of our viable candidates into a CalleeCandidateInfo & sort it
-    // out.
-    CalleeCandidateInfo candidateInfo(Type(), candidates, hasTrailingClosure,
-                                      CS);
-
-    // Filter the candidate list based on the argument we may or may not have.
-    candidateInfo.filterContextualMemberList(E->getArgument());
-
-    // If we have multiple candidates, then we have an ambiguity.
-    if (candidateInfo.size() != 1) {
-      SourceRange argRange;
-      if (auto arg = E->getArgument())
-        argRange = arg->getSourceRange();
-      diagnose(E->getNameLoc(), diag::ambiguous_member_overload_set,
-               E->getName())
-          .highlight(argRange);
-      candidateInfo.suggestPotentialOverloads(E->getNameLoc().getBaseNameLoc());
-      return true;
-    }
-
-    return false;
-  };
-
-  return diagnoseMemberFailures(E, nullptr, memberConstraint->getKind(),
-                                memberConstraint->getMember(),
-                                memberConstraint->getFunctionRefKind(),
-                                memberConstraint->getLocator(), callback);
-}
-
 bool FailureDiagnosis::diagnoseMemberFailures(
     Expr *E, Expr *baseExpr, ConstraintKind lookupKind, DeclNameRef memberName,
     FunctionRefKind funcRefKind, ConstraintLocator *locator,

lib/Sema/CSFix.cpp

@@ -472,14 +472,15 @@ UseSubscriptOperator *UseSubscriptOperator::create(ConstraintSystem &cs,
 bool DefineMemberBasedOnUse::diagnose(bool asNote) const {
   auto failure = MissingMemberFailure(getConstraintSystem(), BaseType,
                                       Name, getLocator());
-  return failure.diagnose(asNote);
+  return AlreadyDiagnosed || failure.diagnose(asNote);
 }
 
 DefineMemberBasedOnUse *
 DefineMemberBasedOnUse::create(ConstraintSystem &cs, Type baseType,
-                               DeclNameRef member, ConstraintLocator *locator) {
+                               DeclNameRef member, bool alreadyDiagnosed,
+                               ConstraintLocator *locator) {
   return new (cs.getAllocator())
-      DefineMemberBasedOnUse(cs, baseType, member, locator);
+      DefineMemberBasedOnUse(cs, baseType, member, alreadyDiagnosed, locator);
 }
 
 AllowMemberRefOnExistential *

lib/Sema/CSFix.h

@@ -806,10 +806,19 @@ class DefineMemberBasedOnUse final : public ConstraintFix {
   Type BaseType;
   DeclNameRef Name;
 
+  /// Whether or not the member error is already diagnosed. This can happen
+  /// when referencing an erroneous member, and the error is diagnosed at the
+  /// member declaration.
+  ///
+  /// We still want to define erroneous members based on use in order to find
+  /// a solution through the new diagnostic infrastructure, but we don't
+  /// want to report a second error message.
+  bool AlreadyDiagnosed;
+
   DefineMemberBasedOnUse(ConstraintSystem &cs, Type baseType, DeclNameRef member,
-                         ConstraintLocator *locator)
+                         bool alreadyDiagnosed, ConstraintLocator *locator)
       : ConstraintFix(cs, FixKind::DefineMemberBasedOnUse, locator),
-        BaseType(baseType), Name(member) {}
+        BaseType(baseType), Name(member), AlreadyDiagnosed(alreadyDiagnosed) {}
 
 public:
   std::string getName() const override {
@@ -822,7 +831,7 @@ class DefineMemberBasedOnUse final : public ConstraintFix {
   bool diagnose(bool asNote = false) const override;
 
   static DefineMemberBasedOnUse *create(ConstraintSystem &cs, Type baseType,
-                                        DeclNameRef member,
+                                        DeclNameRef member, bool alreadyDiagnosed,
                                         ConstraintLocator *locator);
 };
 

lib/Sema/CSSimplify.cpp

@@ -6121,8 +6121,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
     return formUnsolved();
 
   case MemberLookupResult::ErrorAlreadyDiagnosed:
-    return SolutionKind::Error;
-
   case MemberLookupResult::HasResults:
     // Keep going!
     break;
@@ -6198,8 +6196,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
         return type;
       });
 
-      auto *fix =
-          DefineMemberBasedOnUse::create(*this, baseTy, member, locator);
+      bool alreadyDiagnosed = (result.OverallResult ==
+                               MemberLookupResult::ErrorAlreadyDiagnosed);
+      auto *fix = DefineMemberBasedOnUse::create(*this, baseTy, member,
+                                                 alreadyDiagnosed, locator);
       // Impact is higher if the base is expected to be inferred from context,
       // because a failure to find a member ultimately means that base type is
       // not a match in this case.
@@ -6208,9 +6208,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
       if (recordFix(fix, impact))
         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`.
+      // Record a hole for memberTy to make it possible to form solutions
+      // when contextual result type cannot be deduced e.g. `let _ = x.foo`.
       if (auto *memberTypeVar = memberTy->getAs<TypeVariableType>())
         recordPotentialHole(memberTypeVar);
 
@@ -7860,7 +7859,7 @@ ConstraintSystem::simplifyDynamicCallableApplicableFnConstraint(
     DeclNameRef memberName({ ctx, ctx.Id_dynamicallyCall, {argLabel} });
 
     auto *fix = DefineMemberBasedOnUse::create(
-        *this, desugar2, memberName,
+        *this, desugar2, memberName, /*alreadyDiagnosed=*/false,
         getConstraintLocator(loc, ConstraintLocator::DynamicCallable));
 
     if (recordFix(fix))

test/Constraints/diagnostics.swift

@@ -251,7 +251,8 @@ struct Toe {
   let toenail: Nail // expected-error {{use of undeclared type 'Nail'}}
 
   func clip() {
-    toenail.inspect { x in
+    // FIXME: We shouldn't report this because toenail.inspect is a hole
+    toenail.inspect { x in // expected-error {{unable to infer closure return type; add explicit type to disambiguate}}
       toenail.inspect { y in }
     }
   }

test/Constraints/rdar46377919.swift

@@ -9,5 +9,4 @@ class Foo {
 
 func foo() -> Foo {
   return Foo(lhs: 2, rhs: 2)
-  // expected-error@-1 {{cannot convert value of type 'Int' to expected argument type '<<error type>>'}}
 }