Merge pull request #1160 from gregomni/generic-func-args

[Sema] Extend callee diagnosis to complex args including generics, e.g. (Void) -> T

Author: slavapestov

include/swift/AST/TypeMatcher.h

@@ -230,6 +230,12 @@ class TypeMatcher {
     /// FIXME: Split this out into cases?
     bool visitAnyFunctionType(CanAnyFunctionType firstFunc, Type secondType) {
       if (auto secondFunc = secondType->getAs<AnyFunctionType>()) {
+        // FIXME: Compare throws()? Both existing subclasses would prefer
+        // to mismatch on (!firstFunc->throws() && secondFunc->throws()), but
+        // embedding that non-commutativity in this general matcher is icky.
+        if (firstFunc->isNoEscape() != secondFunc->isNoEscape())
+          return mismatch(firstFunc.getPointer(), secondFunc);
+        
         return this->visit(firstFunc.getInput(), secondFunc->getInput()) &&
                this->visit(firstFunc.getResult(), secondFunc->getResult());
       }

lib/Sema/CSDiag.cpp

@@ -17,6 +17,8 @@
 #include "ConstraintSystem.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "swift/AST/ASTWalker.h"
+#include "swift/AST/TypeWalker.h"
+#include "swift/AST/TypeMatcher.h"
 
 using namespace swift;
 using namespace constraints;
@@ -836,7 +838,7 @@ namespace {
     /// obviously mismatching candidates and compute a "closeness" for the
     /// resultant set.
     std::pair<CandidateCloseness, CalleeCandidateInfo::FailedArgumentInfo>
-    evaluateCloseness(Type candArgListType, ArrayRef<CallArgParam> actualArgs);
+    evaluateCloseness(DeclContext *dc, Type candArgListType, ArrayRef<CallArgParam> actualArgs);
 
     void filterList(ArrayRef<CallArgParam> actualArgs);
     void filterList(Type actualArgsType) {
@@ -980,11 +982,71 @@ static bool argumentMismatchIsNearMiss(Type argType, Type paramType) {
   return false;
 }
 
+/// Given a parameter type that may contain generic type params and an actual
+/// argument type, decide whether the param and actual arg have the same shape
+/// and equal fixed type portions, and return by reference each archetype and
+/// the matching portion of the actual arg type where that archetype appears.
+static bool findGenericSubstitutions(DeclContext *dc, Type paramType, Type actualArgType,
+                                     SmallVector<ArchetypeType *, 4> &archetypes,
+                                     SmallVector<Type, 4> &substitutions) {
+  class GenericVisitor : public TypeMatcher<GenericVisitor> {
+    DeclContext *dc;
+    SmallVector<ArchetypeType *, 4> &archetypes;
+    SmallVector<Type, 4> &substitutions;
+
+  public:
+    GenericVisitor(DeclContext *dc,
+                   SmallVector<ArchetypeType *, 4> &archetypes,
+                   SmallVector<Type, 4> &substitutions)
+    : dc(dc), archetypes(archetypes), substitutions(substitutions) {}
+    
+    bool mismatch(TypeBase *paramType, TypeBase *argType) {
+      return paramType->isEqual(argType);
+    }
+    
+    bool mismatch(SubstitutableType *paramType, TypeBase *argType) {
+      Type type = paramType;
+      if (dc && type->isTypeParameter())
+        type = ArchetypeBuilder::mapTypeIntoContext(dc, paramType);
+      
+      if (auto archetype = type->getAs<ArchetypeType>()) {
+        for (unsigned i = 0, c = archetypes.size(); i < c; i++) {
+          if (archetypes[i]->isEqual(archetype))
+            return substitutions[i]->isEqual(argType);
+        }
+        archetypes.push_back(archetype);
+        substitutions.push_back(argType);
+        return true;
+      }
+      return false;
+    }
+  };
+  
+  // If paramType contains any substitutions already, find them and add them
+  // to our list before matching the two types to find more.
+  paramType.findIf([&](Type type) -> bool {
+    if (auto substitution = dyn_cast<SubstitutedType>(type.getPointer())) {
+      Type original = substitution->getOriginal();
+      if (dc && original->isTypeParameter())
+        original = ArchetypeBuilder::mapTypeIntoContext(dc, original);
+      
+      if (auto archetype = original->getAs<ArchetypeType>()) {
+        archetypes.push_back(archetype);
+        substitutions.push_back(substitution->getReplacementType());
+      }
+    }
+    return false;
+  });
+  
+  GenericVisitor visitor(dc, archetypes, substitutions);
+  return visitor.match(paramType, actualArgType);
+}
+
 /// Determine how close an argument list is to an already decomposed argument
 /// list.  If the closeness is a miss by a single argument, then this returns
 /// information about that failure.
 std::pair<CandidateCloseness, CalleeCandidateInfo::FailedArgumentInfo>
-CalleeCandidateInfo::evaluateCloseness(Type candArgListType,
+CalleeCandidateInfo::evaluateCloseness(DeclContext *dc, Type candArgListType,
                   ArrayRef<CallArgParam> actualArgs) {
   auto candArgs = decomposeArgParamType(candArgListType);
 
@@ -1064,16 +1126,28 @@ CalleeCandidateInfo::evaluateCloseness(Type candArgListType,
       // more parameters.
       // We can still do something more sophisticated with this.
       // FIXME: Use TC.isConvertibleTo?
-      if (rArgType->isEqual(paramType))
+
+      SmallVector<ArchetypeType *, 4> archetypes;
+      SmallVector<Type, 4> substitutions;
+      bool matched;
+      if (paramType->is<UnresolvedType>())
+        matched = false;
+      else
+        matched = findGenericSubstitutions(dc, paramType, rArgType,
+                                           archetypes, substitutions);
+      
+      if (matched && archetypes.size() == 0)
         continue;
-      if (auto genericParam = paramType->getAs<GenericTypeParamType>())
-        paramType = genericParam->getDecl()->getArchetype();
-      if (paramType->is<ArchetypeType>() && !rArgType->hasTypeVariable()) {
+      if (matched && archetypes.size() == 1 && !rArgType->hasTypeVariable()) {
+        auto archetype = archetypes[0];
+        auto substitution = substitutions[0];
+        
         if (singleArchetype) {
-          if (!paramType->isEqual(singleArchetype))
+          if (!archetype->isEqual(singleArchetype))
             // Multiple archetypes, too complicated.
             return { CC_ArgumentMismatch, {}};
-          if (rArgType->isEqual(matchingArgType)) {
+          
+          if (substitution->isEqual(matchingArgType)) {
             if (nonSubstitutableArgs == 0)
               continue;
             ++nonSubstitutableArgs;
@@ -1087,20 +1161,18 @@ CalleeCandidateInfo::evaluateCloseness(Type candArgListType,
               // If we have only one nonSubstitutableArg so far, then this different
               // type might be the one that we should be substituting for instead.
               // Note that failureInfo is already set correctly for that case.
-              auto archetype = paramType->castTo<ArchetypeType>();
-              if (CS->TC.isSubstitutableFor(rArgType, archetype, CS->DC)) {
-                mismatchesAreNearMisses = argumentMismatchIsNearMiss(matchingArgType, rArgType);
-                matchingArgType = rArgType;
+              if (CS->TC.isSubstitutableFor(substitution, archetype, CS->DC)) {
+                mismatchesAreNearMisses = argumentMismatchIsNearMiss(matchingArgType, substitution);
+                matchingArgType = substitution;
                 continue;
               }
             }
           }
         } else {
-          matchingArgType = rArgType;
-          singleArchetype = paramType;
+          matchingArgType = substitution;
+          singleArchetype = archetype;
 
-          auto archetype = paramType->getAs<ArchetypeType>();
-          if (CS->TC.isSubstitutableFor(rArgType, archetype, CS->DC)) {
+          if (CS->TC.isSubstitutableFor(substitution, archetype, CS->DC)) {
             continue;
           }
           ++nonSubstitutableArgs;
@@ -1325,7 +1397,9 @@ void CalleeCandidateInfo::filterList(ArrayRef<CallArgParam> actualArgs) {
     // If this isn't a function or isn't valid at this uncurry level, treat it
     // as a general mismatch.
     if (!inputType) return { CC_GeneralMismatch, {}};
-    return evaluateCloseness(inputType, actualArgs);
+    Decl *decl = candidate.getDecl();
+    return evaluateCloseness(decl ? decl->getInnermostDeclContext() : nullptr,
+                             inputType, actualArgs);
   });
 }
 
@@ -3540,8 +3614,10 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
     }
 
     // Explode out multi-index subscripts to find the best match.
+    Decl *decl = cand.getDecl();
     auto indexResult =
-      calleeInfo.evaluateCloseness(cand.getArgumentType(), decomposedIndexType);
+      calleeInfo.evaluateCloseness(decl ? decl->getInnermostDeclContext() : nullptr,
+                                   cand.getArgumentType(), decomposedIndexType);
     if (selfConstraint > indexResult.first)
       return {selfConstraint, {}};
     return indexResult;

test/Constraints/diagnostics.swift

@@ -71,6 +71,7 @@ i.wobble() // expected-error{{value of type 'Int' has no member 'wobble'}}
 // Generic member does not conform.
 extension Int {
   func wibble<T: P2>(x: T, _ y: T) -> T { return x }
+  func wubble<T>(x: Int -> T) -> T { return x(self) }
 }
 i.wibble(3, 4) // expected-error {{argument type 'Int' does not conform to expected type 'P2'}}
 
@@ -82,6 +83,15 @@ let a = A()
 for j in i.wibble(a, a) { // expected-error {{type 'A' does not conform to protocol 'SequenceType'}}
 }
 
+// Generic as part of function/tuple types
+func f6<T:P2>(g: Void -> T) -> (c: Int, i: T) {
+  return (c: 0, i: g())
+}
+func f7() -> (c: Int, v: A) {
+  let g: Void -> A = { return A() }
+  return f6(g) // expected-error {{cannot convert return expression of type '(c: Int, i: A)' to return type '(c: Int, v: A)'}}
+}
+
 // <rdar://problem/19658691> QoI: Incorrect diagnostic for calling nonexistent members on literals
 1.doesntExist(0)  // expected-error {{value of type 'Int' has no member 'doesntExist'}}
 [1, 2, 3].doesntExist(0)  // expected-error {{value of type '[Int]' has no member 'doesntExist'}}

test/Generics/function_defs.swift

@@ -17,9 +17,7 @@ func doCompare<T : EqualComparable, U : EqualComparable>(t1: T, t2: T, u: U) ->
     return true
   }
 
-  // FIXME: This is not ambiguous, the actual problem is that 'u' has the wrong
-  // type - "U" is not the same as "T".
-  return t1.isEqual(u) // expected-error {{type of expression is ambiguous without more context}}
+  return t1.isEqual(u) // expected-error {{cannot invoke 'isEqual' with an argument list of type '(U)'}} expected-note {{expected an argument list of type '(T)'}}
 }
 
 protocol MethodLessComparable {
@@ -171,9 +169,7 @@ func staticEqCheck<T : StaticEq, U : StaticEq>(t: T, u: U) {
   if T.isEqual(t, y: t) { return }
   if U.isEqual(u, y: u) { return }
 
-  // FIXME: This is not ambiguous, the actual problem is that 'u' has the wrong
-  // type - "U" is not the same as "T".
-  T.isEqual(t, y: u) // expected-error{{type of expression is ambiguous without more context}}
+  T.isEqual(t, y: u) // expected-error{{cannot invoke 'isEqual' with an argument list of type '(T, y: U)'}} expected-note {{expected an argument list of type '(T, y: T)'}}
 }
 
 //===----------------------------------------------------------------------===//

test/Misc/misc_diagnostics.swift

@@ -91,7 +91,7 @@ func testIS1() -> Int { return 0 }
 let _: String = testIS1() // expected-error {{cannot convert value of type 'Int' to specified type 'String'}}
 
 func insertA<T>(inout array : [T], elt : T) {
-  array.append(T); // expected-error {{ambiguous reference to member 'append'}}
+  array.append(T); // expected-error {{cannot invoke 'append' with an argument list of type '((T).Type)'}} expected-note {{expected an argument list of type '(T)'}}
 }
 
 // <rdar://problem/17875634> can't append to array of tuples
@@ -130,6 +130,6 @@ func test17875634() {
 
 // <rdar://problem/20770032> Pattern matching ranges against tuples crashes the compiler
 func test20770032() {
-  if case let 1...10 = (1, 1) { // expected-warning{{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}} expected-error{{cannot convert value of type '(Int, Int)' to expected argument type 'Range<Int>'}}
+  if case let 1...10 = (1, 1) { // expected-warning{{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}} expected-error{{expression pattern of type 'Range<Int>' cannot match values of type '(Int, Int)'}}
   }
 }

validation-test/stdlib/StdlibUnittestStaticAssertions.swift

@@ -21,7 +21,6 @@ func test_expectType() {
 
 func test_expectEqualType() {
   expectEqualType(S1.self, S1.self)
-  expectEqualType(S1.self, S2.self) // expected-error {{cannot invoke 'expectEqualType' with an argument list of type '(S1.Type, S2.Type)'}}
-  // expected-note @-1 {{expected an argument list of type '(T.Type, T.Type)'}}
+  expectEqualType(S1.self, S2.self) // expected-error {{cannot convert value of type 'S2.Type' to expected argument type 'S1'}}
 }