Merge pull request #29490 from xedin/remove-more-code-from-visitapplyexpr

[Diagnostics] Don't re-typecheck function expression associated with various calls

Author: xedin

lib/Sema/CSDiag.cpp

@@ -1202,32 +1202,6 @@ static Expr *getFailedArgumentExpr(CalleeCandidateInfo CCI, Expr *argExpr) {
 bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI,
                                                Expr *fnExpr, Expr *argExpr,
                                                ArrayRef<Identifier> argLabels) {
-  if (auto *MTT = CS.getType(fnExpr)->getAs<MetatypeType>()) {
-    auto instTy = MTT->getInstanceType();
-    auto &DE = CS.getASTContext().Diags;
-    if (instTy->getAnyNominal()) {
-      // If we are invoking a constructor on a nominal type and there are
-      // absolutely no candidates, then they must all be private.
-      if (CCI.empty() || (CCI.size() == 1 && CCI.candidates[0].getDecl() &&
-                              isa<ProtocolDecl>(CCI.candidates[0].getDecl()))) {
-        DE.diagnose(fnExpr->getLoc(), diag::no_accessible_initializers,
-                    instTy);
-        return true;
-      }
-      // continue below
-    } else if (!instTy->is<TupleType>()) {
-      // If we are invoking a constructor on a non-nominal type, the expression
-      // is malformed.
-      SourceRange initExprRange(fnExpr->getSourceRange().Start,
-                                argExpr->getSourceRange().End);
-      DE.diagnose(fnExpr->getLoc(), instTy->isExistentialType() ?
-                  diag::construct_protocol_by_name :
-                  diag::non_nominal_no_initializers, instTy)
-          .highlight(initExprRange);
-      return true;
-    }
-  }
-
   // Try to diagnose errors related to the use of implicit self reference.
   if (diagnoseImplicitSelfErrors(fnExpr, argExpr, CCI, argLabels))
     return true;
@@ -1268,59 +1242,9 @@ bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI,
   return false;
 }
 
-// Check if there is a structural problem in the function expression
-// by performing type checking with the option to allow unresolved
-// type variables. If that is going to produce a function type with
-// unresolved result let's not re-typecheck the function expression,
-// because it might produce unrelated diagnostics due to lack of
-// contextual information.
-static bool shouldTypeCheckFunctionExpr(FailureDiagnosis &FD, DeclContext *DC,
-                                        Expr *fnExpr) {
-  if (!isa<UnresolvedDotExpr>(fnExpr))
-    return true;
-
-  SmallPtrSet<TypeBase *, 4> fnTypes;
-  FD.getPossibleTypesOfExpressionWithoutApplying(
-      fnExpr, DC, fnTypes, FreeTypeVariableBinding::UnresolvedType);
-
-  if (fnTypes.size() == 1) {
-    // Some member types depend on the arguments to produce a result type,
-    // type-checking such expressions without associated arguments is
-    // going to produce unrelated diagnostics.
-    if (auto fn = (*fnTypes.begin())->getAs<AnyFunctionType>()) {
-      auto resultType = fn->getResult();
-      if (resultType->hasUnresolvedType() || resultType->hasTypeVariable())
-        return false;
-    }
-  }
-
-  // Might be a structural problem related to the member itself.
-  return true;
-}
-
 bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
   auto *fnExpr = callExpr->getFn();
-
-  if (shouldTypeCheckFunctionExpr(*this, CS.DC, fnExpr)) {
-    // Type check the function subexpression to resolve a type for it if
-    // possible.
-    fnExpr = typeCheckChildIndependently(callExpr->getFn());
-    if (!fnExpr) {
-      return CS.getASTContext().Diags.hadAnyError();
-    }
-  }
-
-  SWIFT_DEFER {
-    if (!fnExpr) return;
-
-    // If it's a member operator reference, put the operator back.
-    if (auto operatorRef = fnExpr->getMemberOperatorRef())
-      callExpr->setFn(operatorRef);
-  };
-
-  auto getFuncType = [](Type type) -> Type { return type->getRValueType(); };
-
-  auto fnType = getFuncType(CS.getType(fnExpr));
+  auto fnType = CS.getType(fnExpr)->getRValueType();
 
   bool hasTrailingClosure = callArgHasTrailingClosure(callExpr->getArg());
 
@@ -1489,24 +1413,17 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
   if (CS.getType(argExpr)->hasUnresolvedType())
     return false;
 
-  SmallVector<AnyFunctionType::Param, 8> params;
-  AnyFunctionType::decomposeInput(CS.getType(argExpr), params);
-  auto argString = AnyFunctionType::getParamListAsString(params);
-
-  if (auto MTT = fnType->getAs<MetatypeType>()) {
-    if (MTT->getInstanceType()->isExistentialType()) {
-      diagnose(fnExpr->getLoc(), diag::construct_protocol_value, fnType);
-      return true;
-    }
-  }
-  
   bool isInitializer = isa<TypeExpr>(fnExpr);
   if (isa<TupleExpr>(argExpr) &&
       cast<TupleExpr>(argExpr)->getNumElements() == 0) {
     // Emit diagnostics that say "no arguments".
     diagnose(fnExpr->getLoc(), diag::cannot_call_with_no_params,
              overloadName, isInitializer);
   } else {
+    SmallVector<AnyFunctionType::Param, 8> params;
+    AnyFunctionType::decomposeInput(CS.getType(argExpr), params);
+    auto argString = AnyFunctionType::getParamListAsString(params);
+
     diagnose(fnExpr->getLoc(), diag::cannot_call_with_params,
              overloadName, argString, isInitializer);
   }

lib/Sema/CSSimplify.cpp

@@ -2249,8 +2249,17 @@ ConstraintSystem::matchTypesBindTypeVar(
   // Simplify the right-hand type and perform the "occurs" check.
   typeVar = getRepresentative(typeVar);
   type = simplifyType(type, flags);
-  if (!isBindable(typeVar, type))
+  if (!isBindable(typeVar, type)) {
+    if (shouldAttemptFixes()) {
+      // If type variable is allowed to be a hole and it can't be bound to
+      // a particular (full resolved) type, just ignore this binding
+      // instead of re-trying it and failing later.
+      if (typeVar->getImpl().canBindToHole() && !type->hasTypeVariable())
+        return getTypeMatchSuccess();
+    }
+
     return formUnsolvedResult();
+  }
 
   // Since member lookup doesn't check requirements
   // it might sometimes return types which are not
@@ -7509,6 +7518,18 @@ ConstraintSystem::simplifyApplicableFnConstraint(
   // following: $T1 -> $T2.
   auto func1 = type1->castTo<FunctionType>();
 
+  // If a type variable representing "function type" is a hole
+  // or it could be bound to some concrete type with a help of
+  // a fix, let's propagate holes to the "input" type. Doing so
+  // provides more information to upcoming argument and result matching.
+  if (shouldAttemptFixes()) {
+    if (auto *typeVar = type2->getAs<TypeVariableType>()) {
+      auto *locator = typeVar->getImpl().getLocator();
+      if (typeVar->isHole() || hasFixFor(locator))
+        recordPotentialHole(func1);
+    }
+  }
+
   // Before stripping lvalue-ness and optional types, save the original second
   // type for handling `func callAsFunction` and `@dynamicCallable`
   // applications. This supports the following cases:
@@ -7721,10 +7742,7 @@ ConstraintSystem::simplifyApplicableFnConstraint(
 
     // If there are any type variables associated with arguments/result
     // they have to be marked as "holes".
-    type1.visit([&](Type subType) {
-      if (auto *typeVar = subType->getAs<TypeVariableType>())
-        recordPotentialHole(typeVar);
-    });
+    recordPotentialHole(func1);
 
     if (desugar2->isHole())
       return SolutionKind::Solved;
@@ -7998,11 +8016,7 @@ ConstraintSystem::simplifyDynamicCallableApplicableFnConstraint(
       return SolutionKind::Error;
 
     recordPotentialHole(tv);
-
-    Type(func1).visit([&](Type type) {
-      if (auto *typeVar = type->getAs<TypeVariableType>())
-        recordPotentialHole(typeVar);
-    });
+    recordPotentialHole(func1);
 
     return SolutionKind::Solved;
   }
@@ -8563,6 +8577,14 @@ void ConstraintSystem::recordPotentialHole(TypeVariableType *typeVar) {
   typeVar->getImpl().enableCanBindToHole(getSavedBindings());
 }
 
+void ConstraintSystem::recordPotentialHole(FunctionType *fnType) {
+  assert(fnType);
+  Type(fnType).visit([&](Type type) {
+    if (auto *typeVar = type->getAs<TypeVariableType>())
+      recordPotentialHole(typeVar);
+  });
+}
+
 ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
     ConstraintFix *fix, Type type1, Type type2, ConstraintKind matchKind,
     TypeMatchOptions flags, ConstraintLocatorBuilder locator) {

lib/Sema/ConstraintSystem.h

@@ -2359,6 +2359,7 @@ class ConstraintSystem {
   bool recordFix(ConstraintFix *fix, unsigned impact = 1);
 
   void recordPotentialHole(TypeVariableType *typeVar);
+  void recordPotentialHole(FunctionType *fnType);
 
   /// Determine whether constraint system already has a fix recorded
   /// for a particular location.

test/Constraints/bridging.swift

@@ -265,7 +265,7 @@ func rdar19831698() {
   var v71 = true + 1.0 // expected-error{{binary operator '+' cannot be applied to operands of type 'Bool' and 'Double'}}
 // expected-note@-1{{overloads for '+'}}
   var v72 = true + true // expected-error{{binary operator '+' cannot be applied to two 'Bool' operands}}
-  var v73 = true + [] // expected-error@:13 {{cannot convert value of type 'Bool' to expected argument type 'Array<Bool>'}}
+  var v73 = true + [] // expected-error@:18 {{binary operator '+' cannot be applied to operands of type 'Bool' and '[Any]'}}
   var v75 = true + "str" // expected-error@:13 {{cannot convert value of type 'Bool' to expected argument type 'String'}}
 }
 

test/Constraints/members.swift

@@ -405,17 +405,17 @@ func bar_32854314() -> Int {
 extension Array where Element == Int {
   func foo() {
     let _ = min(foo_32854314(), bar_32854314()) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}}
-    // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min()'}}
+    // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}}
   }
 
   func foo(_ x: Int, _ y: Double) {
     let _ = min(x, y) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}}
-    // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min()'}}
+    // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}}
   }
 
   func bar() {
     let _ = min(1.0, 2) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}}
-    // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min()'}}
+    // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}}
   }
 }
 

test/expr/expressions.swift

@@ -748,10 +748,8 @@ func invalidDictionaryLiteral() {
 }
 
 
-[4].joined(separator: [1]) // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
-// expected-error@-1 {{cannot convert value of type '[Int]' to expected argument type 'String'}}
-[4].joined(separator: [[[1]]]) // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}}
-// expected-error@-1 {{cannot convert value of type '[[[Int]]]' to expected argument type 'String'}}
+[4].joined(separator: [1]) // expected-error {{referencing instance method 'joined(separator:)' on 'Sequence' requires that 'Int' conform to 'Sequence'}}
+[4].joined(separator: [[[1]]]) // expected-error {{referencing instance method 'joined(separator:)' on 'Sequence' requires that 'Int' conform to 'Sequence'}}
 
 //===----------------------------------------------------------------------===//
 // nil/metatype comparisons

test/type/protocol_composition.swift

@@ -173,7 +173,9 @@ takesP1AndP2([Swift.AnyObject & P1 & P2]())
 takesP1AndP2([AnyObject & protocol_composition.P1 & P2]())
 takesP1AndP2([AnyObject & P1 & protocol_composition.P2]())
 takesP1AndP2([DoesNotExist & P1 & P2]()) // expected-error {{use of unresolved identifier 'DoesNotExist'}}
-takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{module 'Swift' has no member named 'DoesNotExist'}}
+// TODO(diagnostics): The problem here is that `&` is interpreted as a binary operator, we need to re-think
+// how "missing member" fix is implemented because currently it finds N solutions with multiple fixes.
+takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{cannot invoke '' with no arguments}}
 
 typealias T08 = P1 & inout P2 // expected-error {{'inout' may only be used on parameters}}
 typealias T09 = P1 & __shared P2 // expected-error {{'__shared' may only be used on parameters}}