Merge pull request #29065 from hborla/subscript-error-diag

[Diagnostics] Finish porting subscript errors

Author: hborla

lib/Sema/CSDiag.cpp

@@ -234,8 +234,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
       Optional<std::function<bool(ArrayRef<OverloadChoice>)>> callback = None,
       bool includeInaccessibleMembers = true);
 
-  bool diagnoseSubscriptErrors(SubscriptExpr *SE, bool performingSet);
-
   bool visitExpr(Expr *E);
   bool visitIdentityExpr(IdentityExpr *E);
   bool visitTryExpr(TryExpr *E);
@@ -245,7 +243,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
   bool visitDictionaryExpr(DictionaryExpr *E);
   bool visitObjectLiteralExpr(ObjectLiteralExpr *E);
 
-  bool visitSubscriptExpr(SubscriptExpr *SE);
   bool visitApplyExpr(ApplyExpr *AE);
   bool visitRebindSelfInConstructorExpr(RebindSelfInConstructorExpr *E);
 };
@@ -1410,163 +1407,6 @@ bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI,
   return false;
 }
 
-bool FailureDiagnosis::diagnoseSubscriptErrors(SubscriptExpr *SE,
-                                               bool inAssignmentDestination) {
-  auto baseExpr = typeCheckChildIndependently(SE->getBase());
-  if (!baseExpr) return true;
-  auto baseType = CS.getType(baseExpr);
-
-  if (isa<NilLiteralExpr>(baseExpr)) {
-    diagnose(baseExpr->getLoc(), diag::cannot_subscript_nil_literal)
-      .highlight(baseExpr->getSourceRange());
-    return true;
-  }
-
-  std::function<bool(ArrayRef<OverloadChoice>)> callback =
-      [&](ArrayRef<OverloadChoice> candidates) -> bool {
-    CalleeCandidateInfo calleeInfo(Type(), candidates, SE->hasTrailingClosure(),
-                                   CS, /*selfAlreadyApplied*/ false);
-
-    // We're about to typecheck the index list, which needs to be processed with
-    // self already applied.
-    for (unsigned i = 0, e = calleeInfo.size(); i != e; ++i)
-      calleeInfo.candidates[i].skipCurriedSelf = true;
-
-    auto indexExpr =
-        typeCheckArgumentChildIndependently(SE->getIndex(), Type(), calleeInfo);
-    if (!indexExpr)
-      return true;
-
-    // Back to analyzing the candidate list with self applied.
-    for (unsigned i = 0, e = calleeInfo.size(); i != e; ++i)
-      calleeInfo.candidates[i].skipCurriedSelf = false;
-
-    ArrayRef<Identifier> argLabels = SE->getArgumentLabels();
-    if (diagnoseParameterErrors(calleeInfo, SE, indexExpr, argLabels))
-      return true;
-
-    auto indexType = CS.getType(indexExpr);
-
-    auto decomposedBaseType = decomposeArgType(baseType, {Identifier()});
-    auto decomposedIndexType = decomposeArgType(indexType, argLabels);
-    calleeInfo.filterList(
-        [&](OverloadCandidate cand) -> CalleeCandidateInfo::ClosenessResultTy {
-          // Classify how close this match is.  Non-subscript decls don't match.
-          auto subscriptDecl = dyn_cast_or_null<SubscriptDecl>(cand.getDecl());
-          if (!subscriptDecl ||
-              (inAssignmentDestination && !subscriptDecl->supportsMutation()))
-            return {CC_GeneralMismatch, {}};
-
-          // Check whether the self type matches.
-          auto selfConstraint = CC_ExactMatch;
-          if (calleeInfo.evaluateCloseness(cand, decomposedBaseType).first !=
-              CC_ExactMatch)
-            selfConstraint = CC_SelfMismatch;
-
-          // Set a flag to look past the self argument to the indices.
-          cand.skipCurriedSelf = true;
-
-          // Explode out multi-index subscripts to find the best match.
-          auto indexResult =
-              calleeInfo.evaluateCloseness(cand, decomposedIndexType);
-          if (selfConstraint > indexResult.first)
-            return {selfConstraint, {}};
-          return indexResult;
-        });
-
-    // If the closest matches all mismatch on self, we either have something
-    // that cannot be subscripted, or an ambiguity.
-    if (calleeInfo.closeness == CC_SelfMismatch) {
-      diagnose(SE->getLoc(), diag::cannot_subscript_base, baseType)
-          .highlight(SE->getBase()->getSourceRange());
-      // FIXME: Should suggest overload set, but we're not ready for that until
-      // it points to candidates and identifies the self type in the diagnostic.
-      // calleeInfo.suggestPotentialOverloads(SE->getLoc());
-      return true;
-    }
-
-    // Any other failures relate to the index list.
-    for (unsigned i = 0, e = calleeInfo.size(); i != e; ++i)
-      calleeInfo.candidates[i].skipCurriedSelf = true;
-
-    // TODO: Is there any reason to check for CC_NonLValueInOut here?
-
-    if (calleeInfo.closeness == CC_ExactMatch) {
-      auto message = diag::ambiguous_subscript;
-
-      // If there is an exact match on the argument with
-      // a single candidate, let's type-check subscript
-      // as a whole to figure out if there is any structural
-      // problem after all.
-      if (calleeInfo.size() == 1) {
-        Expr *expr = SE;
-        ConcreteDeclRef decl = nullptr;
-        message = diag::cannot_subscript_with_index;
-
-        if (TypeChecker::getTypeOfExpressionWithoutApplying(expr, CS.DC, decl))
-          return false;
-
-        // If we are down to a single candidate but with an unresolved
-        // index type, we can substitute in the base type to get a simpler
-        // and more concrete expected type for this subscript decl, in order
-        // to diagnose a better error.
-        if (baseType && indexType->hasUnresolvedType()) {
-          auto cand = calleeInfo.candidates[0];
-          auto candType = baseType->getTypeOfMember(CS.DC->getParentModule(),
-                                                    cand.getDecl(), nullptr);
-          if (auto *candFunc = candType->getAs<FunctionType>()) {
-            auto paramsType = FunctionType::composeInput(CS.getASTContext(),
-                                                         candFunc->getParams(),
-                                                         false);
-            if (!typeCheckChildIndependently(
-                    indexExpr, paramsType, CTP_CallArgument, TCC_ForceRecheck))
-              return true;
-          }
-        }
-      }
-
-      diagnose(SE->getLoc(), message, baseType, indexType)
-          .highlight(indexExpr->getSourceRange())
-          .highlight(baseExpr->getSourceRange());
-
-      // FIXME: suggestPotentialOverloads should do this.
-      // calleeInfo.suggestPotentialOverloads(SE->getLoc());
-      for (auto candidate : calleeInfo.candidates)
-        if (auto decl = candidate.getDecl())
-          diagnose(decl, diag::found_candidate);
-        else
-          diagnose(candidate.getExpr()->getLoc(), diag::found_candidate);
-
-      return true;
-    }
-
-    if (diagnoseParameterErrors(calleeInfo, SE, indexExpr, argLabels))
-      return true;
-
-    // Diagnose some simple and common errors.
-    if (calleeInfo.diagnoseSimpleErrors(SE))
-      return true;
-
-    diagnose(SE->getLoc(), diag::cannot_subscript_with_index, baseType,
-             indexType);
-
-    calleeInfo.suggestPotentialOverloads(SE->getLoc());
-    return true;
-  };
-
-  auto locator =
-      CS.getConstraintLocator(SE, ConstraintLocator::SubscriptMember);
-
-  return diagnoseMemberFailures(SE, baseExpr, ConstraintKind::ValueMember,
-                                DeclNameRef::createSubscript(),
-                                FunctionRefKind::DoubleApply, locator,
-                                callback);
-}
-
-bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
-  return diagnoseSubscriptErrors(SE, /* inAssignmentDestination = */ false);
-}
-
 // Check if there is a structural problem in the function expression
 // by performing type checking with the option to allow unresolved
 // type variables. If that is going to produce a function type with

lib/Sema/CSDiagnostics.cpp

@@ -3824,6 +3824,21 @@ bool MissingArgumentsFailure::diagnoseAsError() {
   return true;
 }
 
+bool MissingArgumentsFailure::diagnoseAsNote() {
+  auto *locator = getLocator();
+  if (auto overload = getChoiceFor(locator)) {
+    auto *fn = resolveType(overload->openedType)->getAs<AnyFunctionType>();
+    auto loc = overload->choice.getDecl()->getLoc();
+    if (loc.isInvalid())
+      loc = getAnchor()->getLoc();
+    emitDiagnostic(loc, diag::candidate_partial_match,
+                   fn->getParamListAsString(fn->getParams()));
+    return true;
+  }
+
+  return false;
+}
+
 bool MissingArgumentsFailure::diagnoseSingleMissingArgument() const {
   auto &ctx = getASTContext();
 

lib/Sema/CSDiagnostics.h

@@ -1210,6 +1210,8 @@ class MissingArgumentsFailure final : public FailureDiagnostic {
 
   bool diagnoseAsError() override;
 
+  bool diagnoseAsNote() override;
+
   bool diagnoseSingleMissingArgument() const;
 
 private:

lib/Sema/CSGen.cpp

@@ -930,6 +930,16 @@ namespace {
       = { nullptr, nullptr };
     unsigned currentEditorPlaceholderVariable = 0;
 
+    /// Returns false and emits the specified diagnostic if the member reference
+    /// base is a nil literal. Returns true otherwise.
+    bool isValidBaseOfMemberRef(Expr *base, Diag<> diagnostic) {
+      if (auto nilLiteral = dyn_cast<NilLiteralExpr>(base)) {
+        CS.getASTContext().Diags.diagnose(nilLiteral->getLoc(), diagnostic);
+        return false;
+      }
+      return true;
+    }
+
     /// Add constraints for a reference to a named member of the given
     /// base type, and return the type of such a reference.
     Type addMemberRefConstraints(Expr *expr, Expr *base, DeclNameRef name,
@@ -1792,7 +1802,11 @@ namespace {
           return Type();
       }
 
-      return addSubscriptConstraints(expr, CS.getType(expr->getBase()),
+      auto *base = expr->getBase();
+      if (!isValidBaseOfMemberRef(base, diag::cannot_subscript_nil_literal))
+        return nullptr;
+
+      return addSubscriptConstraints(expr, CS.getType(base),
                                      expr->getIndex(),
                                      decl, expr->getArgumentLabels(),
                                      expr->hasTrailingClosure());

test/decl/subscript/subscripting.swift

@@ -398,12 +398,13 @@ func testSubscript1(_ s1 : SubscriptTest1) {
 struct SubscriptTest2 {
   subscript(a : String, b : Int) -> Int { return 0 } // expected-note {{candidate expects value of type 'Int' for parameter #2}}
   // expected-note@-1 {{declared here}}
+  // expected-note@-2 {{candidate has partially matching parameter list (String, Int)}}
     subscript(a : String, b : String) -> Int { return 0 } // expected-note {{candidate expects value of type 'String' for parameter #2}}
+  // expected-note@-1 {{candidate has partially matching parameter list (String, String)}}
 }
 
 func testSubscript1(_ s2 : SubscriptTest2) {
-  _ = s2["foo"] // expected-error {{cannot subscript a value of type 'SubscriptTest2' with an argument of type 'String'}}
-  // expected-note @-1 {{overloads for 'subscript' exist with these partially matching parameter lists: (String, Int), (String, String)}}
+  _ = s2["foo"] // expected-error {{no exact matches in call to subscript}}
 
   let a = s2["foo", 1.0] // expected-error {{no exact matches in call to subscript}}
 

test/type/types.swift

@@ -19,8 +19,12 @@ var d3 : () -> Float = { 4 }
 var d4 : () -> Int = { d2 }  // expected-error{{function produces expected type 'Int'; did you mean to call it with '()'?}} {{26-26=()}}
 
 var e0 : [Int]
-e0[] // expected-error {{cannot subscript a value of type '[Int]' with an argument of type '()'}}
-  // expected-note @-1 {{overloads for 'subscript' exist with these partially matching parameter lists: ((UnboundedRange_) -> ()), (Int), (R), (Range<Int>), (Range<Self.Index>)}}
+e0[] // expected-error {{no exact matches in call to subscript}}
+// expected-note@-1 {{candidate has partially matching parameter list (Int)}}
+// expected-note@-2 {{candidate has partially matching parameter list (Range<Int>)}}
+// expected-note@-3 {{candidate has partially matching parameter list ((UnboundedRange_) -> ())}}
+// expected-note@-4 {{candidate has partially matching parameter list (Range<Array<Int>.Index>)}}
+// expected-note@-5 {{candidate has partially matching parameter list ((UnboundedRange_) -> ())}}
 
 var f0 : [Float]
 var f1 : [(Int,Int)]