Merge pull request #60240 from hamishknight/functional-filter

[CS] Filter out uncallable vars when simplifying applied overloads

Author: hamishknight

include/swift/AST/ArgumentList.h

@@ -527,13 +527,6 @@ class alignas(Argument) ArgumentList final
       ASTContext &ctx,
       llvm::function_ref<Type(Expr *)> getType = __Expr_getType) const;
 
-  /// Avoid adding new usages of this. Creates a TupleType or ParenType
-  /// representing the types in the argument list. A ParenType will be returned
-  /// for a single argument, otherwise a TupleType.
-  Type composeTupleOrParenType(
-      ASTContext &ctx,
-      llvm::function_ref<Type(Expr *)> getType = __Expr_getType) const;
-
   /// Whether the argument list matches a given parameter list. This will return
   /// \c false if the arity doesn't match, or any of the canonical types or
   /// labels don't match. Note that this expects types to be present for the

include/swift/AST/DiagnosticsSema.def

@@ -256,10 +256,6 @@ ERROR(cannot_pass_rvalue_inout,none,
 ERROR(cannot_provide_default_value_inout,none,
       "cannot provide default value to inout parameter %0", (Identifier))
 
-ERROR(cannot_call_with_params, none,
-      "cannot invoke %select{|initializer for type }2'%0' with an argument list"
-      " of type '%1'", (StringRef, StringRef, bool))
-
 ERROR(cannot_call_non_function_value,none,
       "cannot call value of non-function type %0", (Type))
 

include/swift/AST/Types.h

@@ -918,6 +918,11 @@ class alignas(1 << TypeAlignInBits) TypeBase
             getAnyNominal());
   }
 
+  /// Checks whether this type may potentially be callable. This returns true
+  /// for function types, metatypes, nominal types that support being called,
+  /// and types that have not been inferred yet.
+  bool mayBeCallable(DeclContext *dc);
+
   /// Checks whether this is a type that supports being called through the
   /// implementation of a \c callAsFunction method. Note that this does not
   /// check access control.

lib/AST/ArgumentList.cpp

@@ -262,32 +262,6 @@ Expr *ArgumentList::packIntoImplicitTupleOrParen(
   return tuple;
 }
 
-Type ArgumentList::composeTupleOrParenType(
-    ASTContext &ctx, llvm::function_ref<Type(Expr *)> getType) const {
-  if (auto *unary = getUnlabeledUnaryExpr()) {
-    auto ty = getType(unary);
-    assert(ty);
-    ParameterTypeFlags flags;
-    if (get(0).isInOut()) {
-      ty = ty->getInOutObjectType();
-      flags = flags.withInOut(true);
-    }
-    return ParenType::get(ctx, ty, flags);
-  }
-  SmallVector<TupleTypeElt, 4> elts;
-  for (auto arg : *this) {
-    auto ty = getType(arg.getExpr());
-    assert(ty);
-    ParameterTypeFlags flags;
-    if (arg.isInOut()) {
-      ty = ty->getInOutObjectType();
-      flags = flags.withInOut(true);
-    }
-    elts.emplace_back(ty, arg.getLabel(), flags);
-  }
-  return TupleType::get(elts, ctx);
-}
-
 bool ArgumentList::matches(ArrayRef<AnyFunctionType::Param> params,
                            llvm::function_ref<Type(Expr *)> getType) const {
   if (size() != params.size())

lib/AST/Type.cpp

@@ -1992,6 +1992,26 @@ const llvm::fltSemantics &BuiltinFloatType::getAPFloatSemantics() const {
   llvm::report_fatal_error("Unknown FP semantics");
 }
 
+bool TypeBase::mayBeCallable(DeclContext *dc) {
+  if (is<AnyFunctionType>())
+    return true;
+
+  // Callable for construction.
+  if (is<AnyMetatypeType>())
+    return true;
+
+  // Unresolved types that could potentially be callable.
+  if (isPlaceholder() || is<UnresolvedType>() ||
+      isTypeParameter() || isTypeVariableOrMember()) {
+    return true;
+  }
+  // Callable nominal types.
+  if (isCallAsFunctionType(dc) || hasDynamicCallableAttribute())
+    return true;
+
+  return false;
+}
+
 bool TypeBase::mayHaveSuperclass() {
   if (getClassOrBoundGenericClass())
     return true;

lib/Sema/CSApply.cpp

@@ -7747,9 +7747,14 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
     callee = resolveConcreteDeclRef(decl, calleeLoc);
   }
 
+  // Make sure we have a function type that is callable. This helps ensure
+  // Type::mayBeCallable stays up-to-date.
+  auto fnRValueTy = cs.getType(fn)->getRValueType();
+  assert(fnRValueTy->mayBeCallable(dc));
+
   // If this is an implicit call to a `callAsFunction` method, build the
   // appropriate member reference.
-  if (cs.getType(fn)->getRValueType()->isCallAsFunctionType(dc)) {
+  if (fnRValueTy->isCallAsFunctionType(dc)) {
     fn = buildCallAsFunctionMethodRef(*this, apply, *overload, calleeLoc);
     if (!fn)
       return nullptr;

lib/Sema/CSDiagnostics.cpp

@@ -6991,20 +6991,6 @@ bool ExtraneousCallFailure::diagnoseAsError() {
     }
   }
 
-  if (auto *UDE = getAsExpr<UnresolvedDotExpr>(anchor)) {
-    auto *baseExpr = UDE->getBase();
-    auto *call = castToExpr<CallExpr>(getRawAnchor());
-
-    if (getType(baseExpr)->isAnyObject()) {
-      auto argsTy = call->getArgs()->composeTupleOrParenType(
-          getASTContext(), [&](Expr *E) { return getType(E); });
-      emitDiagnostic(diag::cannot_call_with_params,
-                     UDE->getName().getBaseName().userFacingName(),
-                     argsTy.getString(), isa<TypeExpr>(baseExpr));
-      return true;
-    }
-  }
-
   auto diagnostic =
       emitDiagnostic(diag::cannot_call_non_function_value, getType(anchor));
   removeParensFixIt(diagnostic);

lib/Sema/CSSimplify.cpp

@@ -135,7 +135,8 @@ bool constraints::doesMemberRefApplyCurriedSelf(Type baseTy,
 }
 
 static bool areConservativelyCompatibleArgumentLabels(
-    OverloadChoice choice, SmallVectorImpl<FunctionType::Param> &args,
+    ConstraintSystem &cs, OverloadChoice choice,
+    SmallVectorImpl<FunctionType::Param> &args,
     MatchCallArgumentListener &listener,
     Optional<unsigned> unlabeledTrailingClosureArgIndex) {
   ValueDecl *decl = nullptr;
@@ -155,23 +156,35 @@ static bool areConservativelyCompatibleArgumentLabels(
     return true;
   }
 
-  if (!decl->hasParameterList())
-    return true;
-
-  // This is a member lookup, which generally means that the call arguments
-  // (if we have any) will apply to the second level of parameters, with
-  // the member lookup applying the curried self at the first level. But there
-  // are cases where we can get an unapplied declaration reference back.
+  // If this is a member lookup, the call arguments (if we have any) will
+  // generally be applied to the second level of parameters, with the member
+  // lookup applying the curried self at the first level. But there are cases
+  // where we can get an unapplied declaration reference back.
   auto hasAppliedSelf =
       decl->hasCurriedSelf() &&
       doesMemberRefApplyCurriedSelf(choice.getBaseType(), decl);
 
-  auto *fnType = decl->getInterfaceType()->castTo<AnyFunctionType>();
-  if (hasAppliedSelf) {
-    fnType = fnType->getResult()->getAs<AnyFunctionType>();
-    assert(fnType && "Parameter list curry level does not match type");
+  AnyFunctionType *fnType = nullptr;
+  if (decl->hasParameterList()) {
+    fnType = decl->getInterfaceType()->castTo<AnyFunctionType>();
+    if (hasAppliedSelf) {
+      fnType = fnType->getResult()->getAs<AnyFunctionType>();
+      assert(fnType && "Parameter list curry level does not match type");
+    }
+  } else if (auto *VD = dyn_cast<VarDecl>(decl)) {
+    // For variables, we can reject any type that we know cannot be callable.
+    auto varTy = VD->getValueInterfaceType()->lookThroughAllOptionalTypes();
+    if (!varTy->mayBeCallable(cs.DC))
+      return false;
+    fnType = varTy->getAs<AnyFunctionType>();
   }
 
+  // Given we want to be conservative with this checking, if there's any case
+  // we can't match arguments for (e.g callable nominals, type parameters),
+  // default to returning true.
+  if (!fnType)
+    return true;
+
   auto params = fnType->getParams();
   ParameterListInfo paramInfo(params, decl, hasAppliedSelf);
 
@@ -11014,7 +11027,7 @@ bool ConstraintSystem::simplifyAppliedOverloadsImpl(
 
           auto labelsMatch = [&](MatchCallArgumentListener &listener) {
             if (areConservativelyCompatibleArgumentLabels(
-                    choice, argsWithLabels, listener,
+                    *this, choice, argsWithLabels, listener,
                     argList->getFirstTrailingClosureIndex()))
               return true;
 

lib/Sema/ConstraintSystem.cpp

@@ -4403,19 +4403,17 @@ static bool diagnoseAmbiguity(
             type->lookThroughAllOptionalTypes()->getAs<AnyFunctionType>();
         assert(fn);
 
-        if (fn->getNumParams() == 1) {
-          auto *argList =
-              solution.getArgumentList(solution.Fixes.front()->getLocator());
-          assert(argList);
+        auto *argList =
+            solution.getArgumentList(solution.Fixes.front()->getLocator());
+        assert(argList);
 
+        if (fn->getNumParams() == 1 && argList->isUnary()) {
           const auto &param = fn->getParams()[0];
-          auto argType = argList->composeTupleOrParenType(
-              cs.getASTContext(),
-              [&](Expr *E) { return solution.getResolvedType(E); });
+          auto argTy = solution.getResolvedType(argList->getUnaryExpr());
 
           DE.diagnose(noteLoc, diag::candidate_has_invalid_argument_at_position,
                       solution.simplifyType(param.getPlainType()),
-                      /*position=*/1, param.isInOut(), argType);
+                      /*position=*/1, param.isInOut(), argTy);
         } else {
           DE.diagnose(noteLoc, diag::candidate_partial_match,
                       fn->getParamListAsString(fn->getParams()));

test/ClangImporter/objc_parse.swift

@@ -506,7 +506,7 @@ class IncompleteProtocolAdopter : Incomplete, IncompleteOptional { // expected-e
 
 func testNullarySelectorPieces(_ obj: AnyObject) {
   obj.foo(1, bar: 2, 3) // no-warning
-  obj.foo(1, 2, bar: 3) // expected-error{{cannot invoke 'foo' with an argument list of type '(Int, Int, bar: Int)'}}
+  obj.foo(1, 2, bar: 3) // expected-error{{argument 'bar' must precede unnamed argument #2}}
 }
 
 func testFactoryMethodAvailability() {

test/Constraints/diagnostics.swift

@@ -1303,9 +1303,14 @@ func f11(_ n: Int) {}
 func f11<T : P2>(_ n: T, _ f: @escaping (T) -> T) {}  // expected-note {{where 'T' = 'Int'}}
 f11(3, f4) // expected-error {{global function 'f11' requires that 'Int' conform to 'P2'}}
 
-let f12: (Int) -> Void = { _ in } // expected-note {{candidate '(Int) -> Void' requires 1 argument, but 2 were provided}}
-func f12<T : P2>(_ n: T, _ f: @escaping (T) -> T) {} // expected-note {{candidate requires that 'Int' conform to 'P2' (requirement specified as 'T' : 'P2')}}
-f12(3, f4)// expected-error {{no exact matches in call to global function 'f12'}}
+let f12: (Int) -> Void = { _ in }
+func f12<T : P2>(_ n: T, _ f: @escaping (T) -> T) {} // expected-note {{where 'T' = 'Int'}}
+f12(3, f4)// expected-error {{global function 'f12' requires that 'Int' conform to 'P2'}}
+
+// SR-15293: Bad diagnostic for var + func overload with mismatched call
+func f13(x: Int, y: Int) {}
+var f13: Any = 0
+f13(0, x: 0) // expected-error {{incorrect argument labels in call (have '_:x:', expected 'x:y:')}}
 
 // SR-12242
 struct SR_12242_R<Value> {}

test/Constraints/generics.swift

@@ -965,3 +965,15 @@ do {
   let g2: Gift<Sweets> = Gift<Chocolate>(box)
   // expected-error@-1 {{cannot assign value of type 'Gift<Chocolate>' to type 'Gift<Sweets>'}}
 }
+
+func testOverloadGenericVarFn() {
+  struct S<T> {
+    var foo: T
+    func foo(_ y: Int) {}
+    init() { fatalError() }
+  }
+  // Make sure we can pick the variable overload over the function.
+  S<(String) -> Void>().foo("")
+  S<((String) -> Void)?>().foo?("")
+  S<((String) -> Void)?>().foo!("")
+}