Factor out structural error diagnostics out to a helper method (allowing

the code to be actually readable since it unnests it greatly), and call it
both before and after argument type validation.  This allows us to capture
many more structural errors than before, leading to much better diagnostics
in a lot of cases.  This also fixes the specific regressions introduced by
96a1e96.

Author: lattner

lib/Sema/CSDiag.cpp

@@ -1190,6 +1190,11 @@ namespace {
     /// overloads.
     void suggestPotentialOverloads(SourceLoc loc, bool isResult = false);
 
+    /// If the candidate set has been narrowed down to a specific structural
+    /// problem, e.g. that there are too few parameters specified or that
+    /// argument labels don't match up, diagnose that error and return true.
+    bool diagnoseAnyStructuralArgumentError(Expr *argExpr);
+    
     void dump() const LLVM_ATTRIBUTE_USED;
 
   private:
@@ -1685,6 +1690,140 @@ suggestPotentialOverloads(SourceLoc loc, bool isResult) {
   }
 }
 
+
+/// If the candidate set has been narrowed down to a specific structural
+/// problem, e.g. that there are too few parameters specified or that argument
+/// labels don't match up, diagnose that error and return true.
+bool CalleeCandidateInfo::diagnoseAnyStructuralArgumentError(Expr *argExpr) {
+  // TODO: We only handle the situation where there is exactly one candidate
+  // here.
+  if (size() != 1) return false;
+  
+  // We only handle structural errors here.
+  if (closeness != CC_ArgumentLabelMismatch &&
+      closeness != CC_ArgumentCountMismatch)
+    return false;
+    
+  auto args = decomposeArgParamType(argExpr->getType());
+  SmallVector<Identifier, 4> correctNames;
+  unsigned OOOArgIdx = ~0U, OOOPrevArgIdx = ~0U;
+  unsigned extraArgIdx = ~0U, missingParamIdx = ~0U;
+  
+  // If we have a single candidate that failed to match the argument list,
+  // attempt to use matchCallArguments to diagnose the problem.
+  struct OurListener : public MatchCallArgumentListener {
+    SmallVectorImpl<Identifier> &correctNames;
+    unsigned &OOOArgIdx, &OOOPrevArgIdx;
+    unsigned &extraArgIdx, &missingParamIdx;
+    
+  public:
+    OurListener(SmallVectorImpl<Identifier> &correctNames,
+                unsigned &OOOArgIdx, unsigned &OOOPrevArgIdx,
+                unsigned &extraArgIdx, unsigned &missingParamIdx)
+    : correctNames(correctNames),
+    OOOArgIdx(OOOArgIdx), OOOPrevArgIdx(OOOPrevArgIdx),
+    extraArgIdx(extraArgIdx), missingParamIdx(missingParamIdx) {}
+    void extraArgument(unsigned argIdx) override {
+      extraArgIdx = argIdx;
+    }
+    void missingArgument(unsigned paramIdx) override {
+      missingParamIdx = paramIdx;
+    }
+    void outOfOrderArgument(unsigned argIdx, unsigned prevArgIdx) override{
+      OOOArgIdx = argIdx;
+      OOOPrevArgIdx = prevArgIdx;
+    }
+    bool relabelArguments(ArrayRef<Identifier> newNames) override {
+      correctNames.append(newNames.begin(), newNames.end());
+      return true;
+    }
+  } listener(correctNames, OOOArgIdx, OOOPrevArgIdx,
+             extraArgIdx, missingParamIdx);
+  
+  // Use matchCallArguments to determine how close the argument list is (in
+  // shape) to the specified candidates parameters.  This ignores the
+  // concrete types of the arguments, looking only at the argument labels.
+  SmallVector<ParamBinding, 4> paramBindings;
+  auto params = decomposeArgParamType(candidates[0].getArgumentType());
+  if (!matchCallArguments(args, params, hasTrailingClosure,
+                          /*allowFixes:*/true, listener, paramBindings))
+    return false;
+  
+  
+  // If we are missing an parameter, diagnose that.
+  if (missingParamIdx != ~0U) {
+    Identifier name = params[missingParamIdx].Label;
+    auto loc = argExpr->getStartLoc();
+    if (name.empty())
+      CS->TC.diagnose(loc, diag::missing_argument_positional,
+                      missingParamIdx+1);
+    else
+      CS->TC.diagnose(loc, diag::missing_argument_named, name);
+    return true;
+  }
+  
+  if (extraArgIdx != ~0U) {
+    auto name = args[extraArgIdx].Label;
+    Expr *arg = argExpr;
+    auto tuple = dyn_cast<TupleExpr>(argExpr);
+    if (tuple)
+      arg = tuple->getElement(extraArgIdx);
+    auto loc = arg->getLoc();
+    if (tuple && extraArgIdx == tuple->getNumElements()-1 &&
+        tuple->hasTrailingClosure())
+      CS->TC.diagnose(loc, diag::extra_trailing_closure_in_call)
+        .highlight(arg->getSourceRange());
+    else if (params.empty())
+      CS->TC.diagnose(loc, diag::extra_argument_to_nullary_call)
+        .highlight(argExpr->getSourceRange());
+    else if (name.empty())
+      CS->TC.diagnose(loc, diag::extra_argument_positional)
+        .highlight(arg->getSourceRange());
+    else
+      CS->TC.diagnose(loc, diag::extra_argument_named, name)
+        .highlight(arg->getSourceRange());
+    return true;
+  }
+  
+  // If this is a argument label mismatch, then diagnose that error now.
+  if (!correctNames.empty() &&
+      CS->diagnoseArgumentLabelError(argExpr, correctNames,
+                                     /*isSubscript=*/false))
+    return true;
+  
+  // If we have an out-of-order argument, diagnose it as such.
+  if (OOOArgIdx != ~0U && isa<TupleExpr>(argExpr)) {
+    auto tuple = cast<TupleExpr>(argExpr);
+    Identifier first = tuple->getElementName(OOOArgIdx);
+    Identifier second = tuple->getElementName(OOOPrevArgIdx);
+    
+    SourceLoc diagLoc;
+    if (!first.empty())
+      diagLoc = tuple->getElementNameLoc(OOOArgIdx);
+    else
+      diagLoc = tuple->getElement(OOOArgIdx)->getStartLoc();
+    
+    if (!second.empty()) {
+      CS->TC.diagnose(diagLoc, diag::argument_out_of_order, first, second)
+        .highlight(tuple->getElement(OOOArgIdx)->getSourceRange())
+        .highlight(SourceRange(tuple->getElementNameLoc(OOOPrevArgIdx),
+                               tuple->getElement(OOOPrevArgIdx)->getEndLoc()));
+      return true;
+    }
+    
+    CS->TC.diagnose(diagLoc, diag::argument_out_of_order_named_unnamed, first,
+                    OOOPrevArgIdx)
+      .highlight(tuple->getElement(OOOArgIdx)->getSourceRange())
+      .highlight(tuple->getElement(OOOPrevArgIdx)->getSourceRange());
+    return true;
+  }
+  return false;
+}
+
+
+
+
+
 /// Flags that can be used to control name lookup.
 enum TCCFlags {
   /// Allow the result of the subexpression to be an lvalue.  If this is not
@@ -3505,7 +3644,9 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
 
   // Filter the candidate list based on the argument we may or may not have.
   calleeInfo.filterContextualMemberList(callExpr->getArg());
-  
+
+  if (calleeInfo.diagnoseAnyStructuralArgumentError(callExpr->getArg()))
+    return true;
 
   Type argType;  // Type of the argument list, if knowable.
   if (auto FTy = fnType->getAs<AnyFunctionType>())
@@ -3529,124 +3670,8 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
 
   calleeInfo.filterList(argExpr->getType());
 
-
-  // If we filtered this down to exactly one candidate, see if we can produce
-  // an extremely specific error about it.
-  if (calleeInfo.size() == 1) {
-    if (calleeInfo.closeness == CC_ArgumentLabelMismatch ||
-        calleeInfo.closeness == CC_ArgumentCountMismatch) {
-      auto args = decomposeArgParamType(argExpr->getType());
-      SmallVector<Identifier, 4> correctNames;
-      unsigned OOOArgIdx = ~0U, OOOPrevArgIdx = ~0U;
-      unsigned extraArgIdx = ~0U, missingParamIdx = ~0U;
-      
-      // If we have a single candidate that failed to match the argument list,
-      // attempt to use matchCallArguments to diagnose the problem.
-      struct OurListener : public MatchCallArgumentListener {
-        SmallVectorImpl<Identifier> &correctNames;
-        unsigned &OOOArgIdx, &OOOPrevArgIdx;
-        unsigned &extraArgIdx, &missingParamIdx;
-
-      public:
-        OurListener(SmallVectorImpl<Identifier> &correctNames,
-                    unsigned &OOOArgIdx, unsigned &OOOPrevArgIdx,
-                    unsigned &extraArgIdx, unsigned &missingParamIdx)
-           : correctNames(correctNames),
-             OOOArgIdx(OOOArgIdx), OOOPrevArgIdx(OOOPrevArgIdx),
-             extraArgIdx(extraArgIdx), missingParamIdx(missingParamIdx) {}
-        void extraArgument(unsigned argIdx) override {
-          extraArgIdx = argIdx;
-        }
-        void missingArgument(unsigned paramIdx) override {
-          missingParamIdx = paramIdx;
-        }
-        void outOfOrderArgument(unsigned argIdx, unsigned prevArgIdx) override{
-          OOOArgIdx = argIdx;
-          OOOPrevArgIdx = prevArgIdx;
-        }
-        bool relabelArguments(ArrayRef<Identifier> newNames) override {
-          correctNames.append(newNames.begin(), newNames.end());
-          return true;
-        }
-      } listener(correctNames, OOOArgIdx, OOOPrevArgIdx,
-                 extraArgIdx, missingParamIdx);
-
-      // Use matchCallArguments to determine how close the argument list is (in
-      // shape) to the specified candidates parameters.  This ignores the
-      // concrete types of the arguments, looking only at the argument labels.
-      SmallVector<ParamBinding, 4> paramBindings;
-      auto params = decomposeArgParamType(calleeInfo[0].getArgumentType());
-      if (matchCallArguments(args, params, hasTrailingClosure,
-                             /*allowFixes:*/true, listener, paramBindings)) {
-       
-        // If we are missing an parameter, diagnose that.
-        if (missingParamIdx != ~0U) {
-          Identifier name = params[missingParamIdx].Label;
-          auto loc = callExpr->getArg()->getStartLoc();
-          if (name.empty())
-            diagnose(loc, diag::missing_argument_positional,
-                        missingParamIdx+1);
-          else
-            diagnose(loc, diag::missing_argument_named, name);
-          return true;
-        }
-        
-        if (extraArgIdx != ~0U) {
-          auto name = args[extraArgIdx].Label;
-          Expr *arg = argExpr;
-          auto tuple = dyn_cast<TupleExpr>(argExpr);
-          if (tuple)
-            arg = tuple->getElement(extraArgIdx);
-          auto loc = arg->getLoc();
-          if (tuple && extraArgIdx == tuple->getNumElements()-1 &&
-              tuple->hasTrailingClosure())
-            diagnose(loc, diag::extra_trailing_closure_in_call)
-              .highlight(arg->getSourceRange());
-          else if (name.empty())
-            diagnose(loc, diag::extra_argument_positional)
-              .highlight(arg->getSourceRange());
-          else
-            diagnose(loc, diag::extra_argument_named, name)
-              .highlight(arg->getSourceRange());
-          return true;
-        }
-        
-        // If this is a argument label mismatch, then diagnose that error now.
-        if (!correctNames.empty() &&
-            CS->diagnoseArgumentLabelError(argExpr, correctNames,
-                                           /*isSubscript=*/false))
-          return true;
-        
-        // If we have an out-of-order argument, diagnose it as such.
-        if (OOOArgIdx != ~0U && isa<TupleExpr>(callExpr->getArg())) {
-          auto tuple = cast<TupleExpr>(callExpr->getArg());
-          Identifier first = tuple->getElementName(OOOArgIdx);
-          Identifier second = tuple->getElementName(OOOPrevArgIdx);
-
-          SourceLoc diagLoc;
-          if (!first.empty())
-            diagLoc = tuple->getElementNameLoc(OOOArgIdx);
-          else
-            diagLoc = tuple->getElement(OOOArgIdx)->getStartLoc();
-          
-          if (!second.empty()) {
-            diagnose(diagLoc,
-                     diag::argument_out_of_order, first, second)
-            .highlight(tuple->getElement(OOOArgIdx)->getSourceRange())
-            .highlight(SourceRange(tuple->getElementNameLoc(OOOPrevArgIdx),
-                               tuple->getElement(OOOPrevArgIdx)->getEndLoc()));
-            return true;
-          }
-          
-          diagnose(diagLoc, diag::argument_out_of_order_named_unnamed, first,
-                   OOOPrevArgIdx)
-            .highlight(tuple->getElement(OOOArgIdx)->getSourceRange())
-            .highlight(tuple->getElement(OOOPrevArgIdx)->getSourceRange());
-          return true;
-        }
-      }
-    }
-  }
+  if (calleeInfo.diagnoseAnyStructuralArgumentError(argExpr))
+    return true;
 
   // If we have a failure where the candidate set differs on exactly one
   // argument, and where we have a consistent mismatch across the candidate set

test/ClangModules/cfuncs_parse.swift

@@ -12,8 +12,8 @@ func test_cfunc1(i: Int) {
 func test_cfunc2(i: Int) {
   let f = cfunc2(i, 17)
   _ = f as Float
-  // FIXME: Should report this error: {{cannot convert the expression's type '$T3' to type 'CLong'}}
-  cfunc2(b:17, a:i) // expected-error{{cannot convert value of type 'Int' to expected argument type 'Int32'}}
+  cfunc2(b:17, a:i) // expected-error{{extraneous argument labels 'b:a:' in call}}
+  cfunc2(17, i) // expected-error{{cannot convert value of type 'Int' to expected argument type 'Int32'}}
 }
 
 func test_cfunc3_a() {

test/Constraints/closures.swift

@@ -98,7 +98,7 @@ func testMap() {
 }
 
 // <rdar://problem/22414757> "UnresolvedDot" "in wrong phase" assertion from verifier
-[].reduce { $0 + $1 }  // expected-error {{cannot convert value of type '(_, _) -> _' to expected argument type '(_, combine: @noescape (_, _) throws -> _)'}}
+[].reduce { $0 + $1 }  // expected-error {{missing argument for parameter #1 in call}}
 
 
 

test/Constraints/diagnostics.swift

@@ -308,12 +308,15 @@ func f7(a: Int)(b : Int) -> Int { // expected-warning{{curried function declarat
 f7(1)(b: 1)
 f7(1.0)(2)       // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
 
-f7(1)(1.0)       // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
+f7(1)(1.0)       // expected-error {{missing argument label 'b:' in call}}
+f7(1)(b: 1.0)       // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
 
 let f8 = f7(2)
 f8(b: 1)
 f8(10)          // expected-error {{missing argument label 'b:' in call}} {{4-4=b: }}
-f8(1.0)         // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
+f8(1.0)         // expected-error {{missing argument label 'b:' in call}}
+f8(b: 1.0)         // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
+
 
 class CurriedClass {
   func method1() {}
@@ -352,7 +355,7 @@ _ = CurriedClass.method3(c)(1, 2)        // expected-error {{missing argument la
 _ = CurriedClass.method3(c)(1, b: 2)(32) // expected-error {{cannot call value of non-function type '()'}}
 _ = CurriedClass.method3(1, 2)           // expected-error {{extra argument in call}}
 CurriedClass.method3(c)(1.0, b: 1)       // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
-CurriedClass.method3(c)(1)               // expected-error {{cannot convert value of type 'Int' to expected argument type '(Int, b: Int)'}}
+CurriedClass.method3(c)(1)               // expected-error {{missing argument for parameter 'b' in call}}
 
 CurriedClass.method3(c)(c: 1.0)          // expected-error {{missing argument for parameter 'b' in call}}
 
@@ -361,8 +364,8 @@ extension CurriedClass {
   func f() {
     method3(1, b: 2)
     method3()            // expected-error {{missing argument for parameter #1 in call}}
-    method3(42)          // expected-error {{cannot convert value of type 'Int' to expected argument type '(Int, b: Int)'}}
-    method3(self)        // expected-error {{cannot convert value of type 'CurriedClass' to expected argument type '(Int, b: Int)'}}
+    method3(42)          // expected-error {{missing argument for parameter 'b' in call}}
+    method3(self)        // expected-error {{missing argument for parameter 'b' in call}}
   }
 }
 
@@ -513,7 +516,8 @@ class B {
 
 func test(a : B) {
   B.f1(nil)    // expected-error {{nil is not compatible with expected argument type 'AOpts'}}
-  a.function(42, nil) //expected-error {{nil is not compatible with expected argument type 'AOpts'}}
+  a.function(42, a: nil) //expected-error {{nil is not compatible with expected argument type 'AOpts'}}
+  a.function(42, nil) //expected-error {{missing argument label 'a:' in call}}
   a.f2(nil)  // expected-error {{nil is not compatible with expected argument type 'AOpts'}}
 }
 

test/Generics/generic_types.swift

@@ -195,7 +195,8 @@ func useNested(ii: Int, hni: HasNested<Int>,
   typealias HNI = HasNested<Int>
   var id = hni.f(1, u: 3.14159)
   id = (2, 3.14159)
-  hni.f(1.5, 3.14159) // expected-error{{cannot convert value of type 'Double' to expected argument type 'Int'}}
+  hni.f(1.5, 3.14159) // expected-error{{missing argument label 'u:' in call}}
+  hni.f(1.5, u: 3.14159) // expected-error{{cannot convert value of type 'Double' to expected argument type 'Int'}}
 
   // Generic constructor of a generic struct
   HNI(1, 2.71828) // expected-warning{{unused}}

test/TypeCoercion/overload_member.swift

@@ -71,7 +71,7 @@ func test_mixed_overload(a: A, x: X, y: Y) {
   x1 = x
   var y1 = a.mixed(y: y) // expected-error{{incorrect argument label in call (have 'y:', expected 'x:')}}
 
-  A.mixed(x) // expected-error{{cannot convert value of type 'X' to expected argument type 'Y'}}
+  A.mixed(x) // expected-error{{missing argument label 'y:' in call}}
   var x2 = A.mixed(a)(x: x)
   x2 = x
   var y2 = A.mixed(y: y)
@@ -157,7 +157,7 @@ extension A {
   }
 
   class func test_mixed_overload_static(a a: A, x: X, y: Y) {
-    mixed(x) // expected-error{{cannot convert value of type 'X' to expected argument type 'Y'}}
+    mixed(x) // expected-error{{missing argument label 'y:' in call}}
     var x2 = mixed(a)(x: x)
     x2 = x
     var y2 = mixed(y: y)

test/decl/func/complete_object_init.swift

@@ -94,7 +94,7 @@ class B2 : A {
 
 func testConstructB2(i: Int) {
   var b2a = B2()
-  var b2b = B2(int: i) // expected-error{{extra argument 'int' in call}}
+  var b2b = B2(int: i) // expected-error{{argument passed to call that takes no arguments}}
 
   var b2: B2 = b2a
 }