[CSSimplify] Allow overload choices with missing labels to be considered for diagnostics

Let's make use of a newly added "disable for performance" flag to
allow solver to consider overload choices where the only issue is
missing one or more labels - this makes it for a much better
diagnostic experience without any performance impact for valid code.

Author: xedin

lib/Sema/CSSimplify.cpp

@@ -125,6 +125,7 @@ bool constraints::doesMemberRefApplyCurriedSelf(Type baseTy,
 
 static bool areConservativelyCompatibleArgumentLabels(
     OverloadChoice choice, SmallVectorImpl<FunctionType::Param> &args,
+    MatchCallArgumentListener &listener,
     Optional<unsigned> unlabeledTrailingClosureArgIndex) {
   ValueDecl *decl = nullptr;
   switch (choice.getKind()) {
@@ -163,7 +164,6 @@ static bool areConservativelyCompatibleArgumentLabels(
   auto params = fnType->getParams();
   ParameterListInfo paramInfo(params, decl, hasAppliedSelf);
 
-  MatchCallArgumentListener listener;
   return matchCallArguments(args, params, paramInfo,
                             unlabeledTrailingClosureArgIndex,
                             /*allow fixes*/ false, listener,
@@ -1220,6 +1220,32 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
   }
 };
 
+class AllowLabelMismatches : public MatchCallArgumentListener {
+  SmallVector<Identifier, 4> NewLabels;
+  bool HadLabelingIssues = false;
+
+public:
+  bool missingLabel(unsigned paramIndex) override {
+    HadLabelingIssues = true;
+    return false;
+  }
+
+  bool relabelArguments(ArrayRef<Identifier> newLabels) override {
+    NewLabels.append(newLabels.begin(), newLabels.end());
+    HadLabelingIssues = true;
+    return false;
+  }
+
+  bool hadLabelingIssues() const { return HadLabelingIssues; }
+
+  Optional<ArrayRef<Identifier>> getLabelReplacements() const {
+    if (!hadLabelingIssues() || NewLabels.empty())
+      return None;
+
+    return {NewLabels};
+  }
+};
+
 // Match the argument of a call to the parameter.
 ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
     ConstraintSystem &cs, FunctionType *contextualType,
@@ -9726,11 +9752,57 @@ bool ConstraintSystem::simplifyAppliedOverloadsImpl(
           argsWithLabels.append(args.begin(), args.end());
           FunctionType::relabelParams(argsWithLabels, argumentInfo->Labels);
 
-          if (!areConservativelyCompatibleArgumentLabels(
-                  choice, argsWithLabels,
-                  argumentInfo->UnlabeledTrailingClosureIndex)) {
+          auto labelsMatch = [&](MatchCallArgumentListener &listener) {
+            if (areConservativelyCompatibleArgumentLabels(
+                    choice, argsWithLabels, listener,
+                    argumentInfo->UnlabeledTrailingClosureIndex))
+              return true;
+
             labelMismatch = true;
             return false;
+          };
+
+          AllowLabelMismatches listener;
+
+          // This overload has more problems than just missing/invalid labels.
+          if (!labelsMatch(listener))
+            return false;
+
+          // If overload did match, let's check if it needs to be disabled
+          // in "performance" mode because it has missing labels.
+          if (listener.hadLabelingIssues()) {
+            // In performance mode, let's just disable the choice,
+            // this decision could be rolled back for diagnostics.
+            if (!shouldAttemptFixes())
+              return false;
+
+            // Match expected vs. actual to see whether the only kind
+            // of problem here is missing label(s).
+            auto onlyMissingLabels =
+                [&argumentInfo](ArrayRef<Identifier> expectedLabels) {
+                  auto actualLabels = argumentInfo->Labels;
+                  if (actualLabels.size() != expectedLabels.size())
+                    return false;
+
+                  for (unsigned i = 0; i != actualLabels.size(); ++i) {
+                    auto actual = actualLabels[i];
+                    auto expected = expectedLabels[i];
+
+                    if (actual.compare(expected) != 0 && !actual.empty())
+                      return false;
+                  }
+
+                  return true;
+                };
+
+            auto replacementLabels = listener.getLabelReplacements();
+            // Either it's just one argument or all issues are missing labels.
+            if (!replacementLabels || onlyMissingLabels(*replacementLabels)) {
+              constraint->setDisabled(/*enableForDiagnostics=*/true);
+              // Don't include this overload in "common result" computation
+              // because it has issues.
+              return true;
+            }
           }
         }
 

test/Constraints/diagnostics.swift

@@ -1102,18 +1102,10 @@ func rdar17170728() {
     // expected-error@-1 4 {{optional type 'Int?' cannot be used as a boolean; test for '!= nil' instead}}
   }
 
-  let _ = [i, j, k].reduce(0 as Int?) {
-    // expected-error@-1 3 {{cannot convert value of type 'Int?' to expected element type 'Bool'}}
-    // expected-error@-2 {{value of optional type 'Int?' must be unwrapped to a value of type 'Int'}}
-    // expected-note@-3 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
-    // expected-note@-4 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+  let _ = [i, j, k].reduce(0 as Int?) { // expected-error {{missing argument label 'into:' in call}}
+    // expected-error@-1 {{cannot convert value of type 'Int?' to expected argument type '(inout @escaping (Bool, Bool) -> Bool?, Int?) throws -> ()'}}
     $0 && $1 ? $0 + $1 : ($0 ? $0 : ($1 ? $1 : nil))
-    // expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
-    // expected-error@-2 {{cannot convert value of type 'Bool?' to closure result type 'Int'}}
-    // expected-error@-3 {{result values in '? :' expression have mismatching types 'Int' and 'Bool?'}}
-    // expected-error@-4 {{cannot convert value of type 'Bool' to expected argument type 'Int'}}
-    // expected-error@-5 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
-    // expected-error@-6 {{result values in '? :' expression have mismatching types 'Int' and 'Bool?'}}
+    // expected-error@-1 {{binary operator '+' cannot be applied to two 'Bool' operands}}
   }
 }
 

test/Constraints/generics.swift

@@ -648,7 +648,7 @@ let arr = [BottleLayout]()
 let layout = BottleLayout(count:1)
 let ix = arr.firstIndex(of:layout) // expected-error {{referencing instance method 'firstIndex(of:)' on 'Collection' requires that 'BottleLayout' conform to 'Equatable'}}
 
-let _: () -> UInt8 = { .init("a" as Unicode.Scalar) } // expected-error {{initializer 'init(_:)' requires that 'Unicode.Scalar' conform to 'BinaryInteger'}}
+let _: () -> UInt8 = { .init("a" as Unicode.Scalar) } // expected-error {{missing argument label 'ascii:' in call}}
 
 // https://bugs.swift.org/browse/SR-9068
 func compare<C: Collection, Key: Hashable, Value: Equatable>(c: C)

test/Constraints/overload_filtering_objc.swift

@@ -19,6 +19,6 @@ import Foundation
 }
 
 func testOptional(obj: P) {
-  // CHECK: disabled disjunction term $T2 bound to decl overload_filtering_objc.(file).P.opt(double:)
+  // CHECK: [disabled] $T2 bound to decl overload_filtering_objc.(file).P.opt(double:)
   _ = obj.opt?(1)
 }

test/Constraints/result_builder_diags.swift

@@ -249,7 +249,7 @@ func erroneousSR11350(x: Int) {
       if b {
         acceptInt(0) { }
       }
-    }).domap(0) // expected-error{{value of type 'Optional<()>' has no member 'domap'}}
+    }).domap(0) // expected-error{{value of type '()?' has no member 'domap'}}
   }
 }
 

test/Sema/keypath_subscript_nolabel.swift

@@ -19,13 +19,7 @@ struct S3 {
   subscript(v v: KeyPath<S3, Int>) -> Int { get { 0 } set(newValue) {} }
 }
 var s3 = S3()
-// TODO(diagnostics): This should actually be a diagnostic that correctly identifies that in the presence
-// of a missing label, there are two options for resolution: 'keyPath' and 'v:' and to offer the user
-// a choice.
-// Today, the ExprTypeChecker identifies the disjunction with two of these possibilities, but 
-// filters out some of the terms based on label mismatch (but not implicit keypath terms, for example).
-// It should probably not do that.
-s3[\.x] = 10 // expected-error {{missing argument label 'keyPath:' in subscript}} {{4-4=keyPath: }}
+s3[\.x] = 10 // expected-error {{missing argument label 'v:' in subscript}} {{4-4=v: }}
 
 struct S4 {
   var x : Int = 0

test/stdlib/UnsafePointerDiagnostics.swift

@@ -113,7 +113,7 @@ func unsafeRawBufferPointerConversions(
   _ = UnsafeRawBufferPointer(start: rp, count: 1)
   _ = UnsafeMutableRawBufferPointer(mrbp)
   _ = UnsafeRawBufferPointer(mrbp)
-  _ = UnsafeMutableRawBufferPointer(rbp) // expected-error {{cannot convert value of type 'UnsafeRawBufferPointer' to expected argument type 'UnsafeMutableRawBufferPointer'}}
+  _ = UnsafeMutableRawBufferPointer(rbp) // expected-error {{missing argument label 'mutating:' in call}}
   _ = UnsafeRawBufferPointer(rbp)
   _ = UnsafeMutableRawBufferPointer(mbpi)
   _ = UnsafeRawBufferPointer(mbpi)

validation-test/Sema/type_checker_crashers_fixed/rdar45470505.swift

@@ -3,7 +3,7 @@
 extension BinaryInteger {
   init(bytes: [UInt8]) { fatalError() }
 
-  init<S: Sequence>(bytes: S) where S.Iterator.Element == UInt8 {
+  init<S: Sequence>(bytes: S) where S.Iterator.Element == UInt8 { // expected-note {{incorrect labels for candidate (have: '(_:)', expected: '(bytes:)')}}
     self.init(bytes // expected-error {{no exact matches in call to initializer}}
     // expected-note@-1 {{}}
 

validation-test/compiler_crashers_2_fixed/0158-rdar40165062.swift

@@ -1,14 +1,14 @@
 // RUN: %target-typecheck-verify-swift
 
-struct Foo<T, U> {
+struct Foo<T, U> { // expected-note {{incorrect labels for candidate (have: '(_:)', expected: '(value:)')}}
   var value: U
   func bar() -> Foo<T, U> {
     return Foo(value)
-    // expected-error@-1 {{referencing initializer 'init(_:)' on 'Foo' requires the types 'T' and 'U' be equivalent}}
+    // expected-error@-1 {{no exact matches in call to initializer}}
   }
 }
 
-extension Foo where T == U { // expected-note {{where 'T' = 'T', 'U' = 'U'}}
+extension Foo where T == U { // expected-note {{candidate requires that the types 'T' and 'U' be equivalent (requirement specified as 'T' == 'U')}}
   init(_ value: U)  {
     self.value = value
   }