[Diagnostics] SR-2242: Fix diagnostic when argument label is omitted

Author: xedin

lib/Sema/CSDiag.cpp

@@ -1274,6 +1274,9 @@ CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType,
     void missingArgument(unsigned paramIdx) override {
       result = CC_ArgumentCountMismatch;
     }
+    void missingLabel(unsigned paramIdx) override {
+      result = CC_ArgumentLabelMismatch;
+    }
     void outOfOrderArgument(unsigned argIdx, unsigned prevArgIdx) override {
       result = CC_ArgumentLabelMismatch;
     }
@@ -4681,7 +4684,7 @@ static bool diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI,
 
     CalleeCandidateInfo CandidateInfo;
 
-    // Indicates if problem was been found and diagnostic was emitted.
+    // Indicates if problem has been found and diagnostic was emitted.
     bool Diagnosed = false;
     // Indicates if functions we are trying to call is a subscript.
     bool IsSubscript;
@@ -4740,6 +4743,15 @@ static bool diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI,
       Diagnosed = true;
     }
 
+    void missingLabel(unsigned paramIdx) override {
+      auto tuple = cast<TupleExpr>(ArgExpr);
+      TC.diagnose(tuple->getElement(paramIdx)->getStartLoc(),
+                  diag::missing_argument_labels, false,
+                  Parameters[paramIdx].Label.str(), IsSubscript);
+
+      Diagnosed = true;
+    }
+
     void outOfOrderArgument(unsigned argIdx, unsigned prevArgIdx) override {
       auto tuple = cast<TupleExpr>(ArgExpr);
       Identifier first = tuple->getElementName(argIdx);
@@ -4768,6 +4780,7 @@ static bool diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI,
       auto secondRange = argRange(prevArgIdx, second);
 
       SourceLoc diagLoc = firstRange.Start;
+
       if (first.empty() && second.empty()) {
         TC.diagnose(diagLoc, diag::argument_out_of_order_unnamed_unnamed,
                     argIdx + 1, prevArgIdx + 1)

lib/Sema/CSSimplify.cpp

@@ -28,6 +28,8 @@ void MatchCallArgumentListener::extraArgument(unsigned argIdx) { }
 
 void MatchCallArgumentListener::missingArgument(unsigned paramIdx) { }
 
+void MatchCallArgumentListener::missingLabel(unsigned paramIdx) {}
+
 void MatchCallArgumentListener::outOfOrderArgument(unsigned argIdx,
                                                    unsigned prevArgIdx) {
 }
@@ -454,6 +456,27 @@ matchCallArguments(ArrayRef<CallArgParam> args,
       }
 
       unsigned prevArgIdx = parameterBindings[prevParamIdx].front();
+
+      // First let's double check if out-of-order argument is nothing
+      // more than a simple label mismatch, because in situation where
+      // one argument requires label and another one doesn't, but caller
+      // doesn't provide either, problem is going to be identified as
+      // out-of-order argument instead of label mismatch.
+      auto &parameter = params[prevArgIdx];
+      if (parameter.hasLabel()) {
+        auto expectedLabel = parameter.Label;
+        auto argumentLabel = args[argIdx].Label;
+
+        // If there is a label but it's incorrect it can only mean
+        // situation like this: expected (x, _ y) got (y, _ x).
+        if (argumentLabel.empty() ||
+            (expectedLabel.compare(argumentLabel) != 0 &&
+             args[prevArgIdx].Label.empty())) {
+          listener.missingLabel(prevArgIdx);
+          return true;
+        }
+      }
+
       listener.outOfOrderArgument(argIdx, prevArgIdx);
       return true;
     }

lib/Sema/ConstraintSystem.h

@@ -2243,6 +2243,11 @@ class MatchCallArgumentListener {
   /// \param paramIdx The index of the parameter that is missing an argument.
   virtual void missingArgument(unsigned paramIdx);
 
+  /// Indicate that there was no label given when one was expected by parameter.
+  ///
+  /// \param paramIndex The index of the parameter that is missing a label.
+  virtual void missingLabel(unsigned paramIndex);
+
   /// Indicates that an argument is out-of-order with respect to a previously-
   /// seen argument.
   ///

test/Constraints/diagnostics.swift

@@ -713,10 +713,9 @@ func nilComparison(i: Int, o: AnyObject) {
   _ = o !== nil // expected-warning {{comparing non-optional value of type 'AnyObject' to nil always returns true}}
 }
 
-// FIXME: Bad diagnostic
 func secondArgumentNotLabeled(a:Int, _ b: Int) { }
 secondArgumentNotLabeled(10, 20)
-// expected-error@-1 {{unnamed argument #2 must precede unnamed argument #1}}
+// expected-error@-1 {{missing argument label 'a' in call}}
 
 // <rdar://problem/23709100> QoI: incorrect ambiguity error due to implicit conversion
 func testImplConversion(a : Float?) -> Bool {}
@@ -795,3 +794,24 @@ func valueForKey<K>(_ key: K) -> CacheValue? {
   let cache = NSCache<K, CacheValue>()
   return cache.object(forKey: key)?.value // expected-error {{ambiguous reference to member 'value(x:)'}}
 }
+
+// SR-2242: poor diagnostic when argument label is omitted
+
+func r27212391(x: Int, _ y: Int) {
+  let _: Int = x + y
+}
+
+func r27212391(a: Int, x: Int, _ y: Int) {
+  let _: Int = a + x + y
+}
+
+r27212391(3, 5)             // expected-error {{missing argument label 'x' in call}}
+r27212391(3, y: 5)          // expected-error {{missing argument label 'x' in call}}
+r27212391(3, x: 5)          // expected-error {{argument 'x' must precede unnamed argument #1}}
+r27212391(y: 3, x: 5)       // expected-error {{argument 'x' must precede argument 'y'}}
+r27212391(y: 3, 5)          // expected-error {{incorrect argument label in call (have 'y:_:', expected 'x:_:')}}
+r27212391(x: 3, x: 5)       // expected-error {{extraneous argument label 'x:' in call}}
+r27212391(a: 1, 3, y: 5)    // expected-error {{missing argument label 'x' in call}}
+r27212391(1, x: 3, y: 5)    // expected-error {{missing argument label 'a' in call}}
+r27212391(a: 1, y: 3, x: 5) // expected-error {{argument 'x' must precede argument 'y'}}
+r27212391(a: 1, 3, x: 5)    // expected-error {{argument 'x' must precede unnamed argument #2}}