Merge pull request #28783 from xedin/cut-down-fn-retypecheck-in-visit-applye-expr

xedin · web-flow · commit a96c40d23b85 · 2019-12-16T10:09:05.000-08:00
[CSDiag] Remove obsolete function diagnostics from `visitApplyExpr`
diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp
@@ -201,12 +201,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
                                          ContextualTypePurpose CTP,
                                          Type suggestedType = Type());
 
-  /// Attempt to produce a diagnostic for a mismatch between a call's
-  /// type and its assumed contextual type.
-  bool diagnoseCallContextualConversionErrors(ApplyExpr *callEpxr,
-                                              Type contextualType,
-                                              ContextualTypePurpose CTP);
-
   bool diagnoseImplicitSelfErrors(Expr *fnExpr, Expr *argExpr,
                                   CalleeCandidateInfo &CCI,
                                   ArrayRef<Identifier> argLabels);
@@ -1671,36 +1665,6 @@ namespace {
   };
 } // end anonymous namespace
 
-/// Check if there failure associated with expression is related
-/// to given contextual type.
-bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
-    ApplyExpr *callExpr, Type contextualType, ContextualTypePurpose CTP) {
-  if (!contextualType || contextualType->hasUnresolvedType())
-    return false;
-
-  auto typeCheckExpr = [&](Expr *expr, DeclContext *DC,
-                           SmallPtrSetImpl<TypeBase *> &types) {
-    getPossibleTypesOfExpressionWithoutApplying(
-        expr, DC, types, FreeTypeVariableBinding::Disallow);
-  };
-
-  // First let's type-check expression without contextual type, and
-  // see if that's going to produce a type, if so, let's type-check
-  // again, this time using given contextual type.
-  SmallPtrSet<TypeBase *, 4> withoutContextual;
-  typeCheckExpr(callExpr, CS.DC, withoutContextual);
-
-  // If there are no types returned, it means that problem was
-  // nothing to do with contextual information, probably parameter/argument
-  // mismatch.
-  if (withoutContextual.empty())
-    return false;
-
-  Type exprType = withoutContextual.size() == 1 ? *withoutContextual.begin() : Type();
-  return diagnoseContextualConversionError(callExpr, contextualType, CTP,
-                                           exprType);
-}
-
 // 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
@@ -1731,45 +1695,8 @@ static bool shouldTypeCheckFunctionExpr(FailureDiagnosis &FD, DeclContext *DC,
   return true;
 }
 
-// Check if any candidate of the overload set can accept a specified
-// number of arguments, regardless of parameter type or label information.
-static bool isViableOverloadSet(const CalleeCandidateInfo &CCI,
-                                size_t numArgs) {
-  for (unsigned i = 0; i < CCI.size(); ++i) {
-    auto &&cand = CCI[i];
-    auto funcDecl = dyn_cast_or_null<AbstractFunctionDecl>(cand.getDecl());
-
-    // If we don't have a func decl or we haven't resolved its parameters,
-    // continue. The latter case can occur with `type(of:)`, which is introduced
-    // as a type variable.
-    if (!funcDecl || !cand.hasParameters())
-      continue;
-
-    auto params = cand.getParameters();
-    bool hasVariadicParameter = false;
-    auto pairMatcher = [&](unsigned argIdx, unsigned paramIdx) {
-      hasVariadicParameter |= params[paramIdx].isVariadic();
-      return true;
-    };
-
-    auto paramInfo = cand.getParameterListInfo(params);
-    InputMatcher IM(params, paramInfo);
-    auto result = IM.match(numArgs, pairMatcher);
-    if (result == InputMatcher::IM_Succeeded)
-      return true;
-    if (result == InputMatcher::IM_HasUnclaimedInput && hasVariadicParameter)
-      return true;
-  }
-  return false;
-}
-
 bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
-  if (diagnoseCallContextualConversionErrors(callExpr, CS.getContextualType(),
-                                             CS.getContextualTypePurpose()))
-    return true;
-
   auto *fnExpr = callExpr->getFn();
-  auto originalFnType = CS.getType(callExpr->getFn());
 
   if (shouldTypeCheckFunctionExpr(*this, CS.DC, fnExpr)) {
     // Type check the function subexpression to resolve a type for it if
@@ -1792,162 +1719,12 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
 
   auto fnType = getFuncType(CS.getType(fnExpr));
 
-  // Let's see if this has to do with member vs. property error
-  // because sometimes when there is a member and a property declared
-  // on the nominal type with the same name. Type-checking function
-  // expression separately from arguments might produce solution for
-  // the property instead of the member.
-  if (!fnType->is<AnyFunctionType>() &&
-    isa<UnresolvedDotExpr>(callExpr->getFn())) {
-    fnExpr = callExpr->getFn();
-
-    SmallPtrSet<TypeBase *, 4> types;
-    getPossibleTypesOfExpressionWithoutApplying(fnExpr, CS.DC, types);
-
-    auto isFunctionType = [getFuncType](Type type) -> bool {
-      return type && getFuncType(type)->is<AnyFunctionType>();
-    };
-
-    auto fnTypes = std::find_if(types.begin(), types.end(), isFunctionType);
-    if (fnTypes != types.end()) {
-      auto funcType = getFuncType(*fnTypes);
-      // If there is only one function type, let's use it.
-      if (std::none_of(std::next(fnTypes), types.end(), isFunctionType))
-        fnType = funcType;
-    } else {
-      fnType = getFuncType(originalFnType);
-    }
-  }
-
-  // If we have a contextual type, and if we have an ambiguously typed function
-  // result from our previous check, we re-type-check it using this contextual
-  // type to inform the result type of the callee.
-  //
-  // We only do this as a second pass because the first pass we just did may
-  // return something of obviously non-function-type.  If this happens, we
-  // produce better diagnostics below by diagnosing this here rather than trying
-  // to peel apart the failed conversion to function type.
-  if (CS.getContextualType() &&
-      (isUnresolvedOrTypeVarType(fnType) ||
-       (fnType->is<AnyFunctionType>() && fnType->hasUnresolvedType()))) {
-    // FIXME: Prevent typeCheckChildIndependently from transforming expressions,
-    // because if we try to typecheck OSR expression with contextual type,
-    // it'll end up converting it into DeclRefExpr based on contextual info,
-    // instead let's try to get a type without applying and filter callee
-    // candidates later on.
-    CalleeListener listener(CS.getContextualType());
-
-    if (isa<OverloadSetRefExpr>(fnExpr)) {
-      assert(!cast<OverloadSetRefExpr>(fnExpr)->getReferencedDecl() &&
-             "unexpected declaration reference");
-
-      ConcreteDeclRef decl = nullptr;
-      Type type = TypeChecker::getTypeOfExpressionWithoutApplying(
-          fnExpr, CS.DC, decl, FreeTypeVariableBinding::UnresolvedType,
-          &listener);
-
-      if (type)
-        fnType = getFuncType(type);
-    } else {
-      fnExpr = typeCheckChildIndependently(callExpr->getFn(), Type(),
-                                           CTP_CalleeResult, TCC_ForceRecheck,
-                                           &listener);
-      if (!fnExpr)
-        return true;
-
-      fnType = getFuncType(CS.getType(fnExpr));
-    }
-  }
-
-  // If we resolved a concrete expression for the callee, and it has
-  // non-function/non-metatype type, then we cannot call it!
-  if (!isUnresolvedOrTypeVarType(fnType) &&
-      !fnType->is<AnyFunctionType>() && !fnType->is<MetatypeType>()) {
-    auto arg = callExpr->getArg();
-
-    // If the argument is a trailing ClosureExpr (i.e. {....}) and it is on
-    // the line after the callee, then it's likely the user forgot to
-    // write "do" before their brace stmt.
-    // Note that line differences of more than 1 are diagnosed during parsing.
-    if (auto *PE = dyn_cast<ParenExpr>(arg)) {
-      if (PE->hasTrailingClosure() && isa<ClosureExpr>(PE->getSubExpr())) {
-        auto *closure = cast<ClosureExpr>(PE->getSubExpr());
-        auto &SM = CS.getASTContext().SourceMgr;
-        if (closure->hasAnonymousClosureVars() &&
-            closure->getParameters()->size() == 0 &&
-            1 + SM.getLineNumber(callExpr->getFn()->getEndLoc()) ==
-                SM.getLineNumber(closure->getStartLoc())) {
-          diagnose(closure->getStartLoc(), diag::brace_stmt_suggest_do)
-              .fixItInsert(closure->getStartLoc(), "do ");
-          return true;
-        }
-      }
-    }
-
-    auto isExistentialMetatypeType = fnType->is<ExistentialMetatypeType>();
-    if (isExistentialMetatypeType) {
-      auto diag = diagnose(arg->getStartLoc(),
-                           diag::missing_init_on_metatype_initialization);
-      diag.highlight(fnExpr->getSourceRange());
-      return true;
-    } else {
-      auto diag = diagnose(arg->getStartLoc(),
-                           diag::cannot_call_non_function_value, fnType);
-      diag.highlight(fnExpr->getSourceRange());
-
-      // If the argument is an empty tuple, then offer a
-      // fix-it to remove the empty tuple and use the value
-      // directly.
-      if (auto tuple = dyn_cast<TupleExpr>(arg)) {
-        if (tuple->getNumElements() == 0) {
-          diag.fixItRemove(arg->getSourceRange());
-        }
-      }
-      return true;
-    }
-  }
-  
   bool hasTrailingClosure = callArgHasTrailingClosure(callExpr->getArg());
   
   // Collect a full candidate list of callees based on the partially type
   // checked function.
   CalleeCandidateInfo calleeInfo(fnExpr, hasTrailingClosure, CS);
 
-  // In the case that function subexpression was resolved independently in
-  // the first place, the resolved type may not provide the best diagnostic.
-  // We consider the number of arguments to decide whether we'd go with it or
-  // stay with the original one.
-  if (fnExpr != callExpr->getFn()) {
-    bool isInstanceMethodAsCurriedMemberOnType = false;
-    if (!calleeInfo.empty()) {
-      auto &&cand = calleeInfo[0];
-      auto decl = cand.getDecl();
-      if (decl && decl->isInstanceMember() && !cand.skipCurriedSelf &&
-          cand.getParameters().size() == 1)
-        isInstanceMethodAsCurriedMemberOnType = true;
-    }
-
-    // In terms of instance method as curried member on type, we should not
-    // take the number of arguments into account.
-    if (!isInstanceMethodAsCurriedMemberOnType) {
-      size_t numArgs = 1;
-      auto arg = callExpr->getArg();
-      if (auto tuple = dyn_cast<TupleExpr>(arg)) {
-        numArgs = tuple->getNumElements();
-      }
-
-      if (!isViableOverloadSet(calleeInfo, numArgs)) {
-        CalleeCandidateInfo calleeInfoOrig(callExpr->getFn(),
-                                           hasTrailingClosure, CS);
-        if (isViableOverloadSet(calleeInfoOrig, numArgs)) {
-          fnExpr = callExpr->getFn();
-          fnType = getFuncType(CS.getType(fnExpr));
-          calleeInfo = calleeInfoOrig;
-        }
-      }
-    }
-  }
-
   // Filter list of the candidates based on the known function type.
   if (auto fn = fnType->getAs<AnyFunctionType>()) {
     using Closeness = CalleeCandidateInfo::ClosenessResultTy;
diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp
@@ -5598,6 +5598,19 @@ bool ExtraneousCallFailure::diagnoseAsError() {
     }
   }
 
+  if (auto *UDE = dyn_cast<UnresolvedDotExpr>(anchor)) {
+    auto *baseExpr = UDE->getBase();
+    auto *call = cast<CallExpr>(getRawAnchor());
+
+    if (getType(baseExpr)->isAnyObject()) {
+      emitDiagnostic(anchor->getLoc(), diag::cannot_call_with_params,
+                     UDE->getName().getBaseName().userFacingName(),
+                     getType(call->getArg())->getString(),
+                     isa<TypeExpr>(baseExpr));
+      return true;
+    }
+  }
+
   auto diagnostic = emitDiagnostic(
       anchor->getLoc(), diag::cannot_call_non_function_value, getType(anchor));
   removeParensFixIt(diagnostic);
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -7427,6 +7427,26 @@ ConstraintSystem::simplifyApplicableFnConstraint(
       // Track how many times we do this so that we can record a fix for each.
       ++unwrapCount;
     }
+
+    // Let's account for optional members concept from Objective-C
+    // which forms a disjunction for member type to check whether
+    // it would be possible to use optional type directly or it has
+    // to be force unwrapped (because such types are imported as IUO).
+    if (unwrapCount > 0 && desugar2->is<TypeVariableType>()) {
+      auto *typeVar = desugar2->castTo<TypeVariableType>();
+      auto *locator = typeVar->getImpl().getLocator();
+      if (locator->isLastElement<LocatorPathElt::Member>()) {
+        auto *fix = ForceOptional::create(*this, origType2, desugar2,
+                                          getConstraintLocator(locator));
+        if (recordFix(fix, /*impact=*/unwrapCount))
+          return SolutionKind::Error;
+
+        // Since the right-hand side of the constraint has been changed
+        // we have to re-generate this constraint to use new type.
+        flags |= TMF_GenerateConstraints;
+        return formUnsolved();
+      }
+    }
   }
 
   // For a function, bind the output and convert the argument to the input.
@@ -7500,11 +7520,6 @@ ConstraintSystem::simplifyApplicableFnConstraint(
         desugar2->is<AnyMetatypeType>())
       return SolutionKind::Error;
 
-    if (auto objectTy = desugar2->lookThroughAllOptionalTypes()) {
-      if (objectTy->isAny() || objectTy->isAnyObject())
-        return SolutionKind::Error;
-    }
-
     // If there are any type variables associated with arguments/result
     // they have to be marked as "holes".
     type1.visit([&](Type subType) {
diff --git a/test/Constraints/iuo_objc.swift b/test/Constraints/iuo_objc.swift
@@ -9,23 +9,29 @@ func iuo_error(prop: IUOProperty) {
   // expected-note@-2{{coalesce}}
   // expected-note@-3{{force-unwrap}}
   let _: Coat? = prop.iuo.optional()!
-  // expected-error@-1 {{cannot invoke 'optional' with no arguments}}
+  // expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped to a value of type '() -> Coat?'}}
+  // expected-note@-2{{coalesce}}
+  // expected-note@-3{{force-unwrap}}
   let _: Coat? = prop.iuo.optional!()
   let _: Coat? = prop.iuo.optional!()!
   let _: Coat? = prop.iuo!.optional()
   // expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped}}
   // expected-note@-2{{coalesce}}
   // expected-note@-3{{force-unwrap}}
   let _: Coat? = prop.iuo!.optional()!
-  // expected-error@-1 {{cannot invoke 'optional' with no arguments}}
+  // expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped to a value of type '() -> Coat?'}}
+  // expected-note@-2{{coalesce}}
+  // expected-note@-3{{force-unwrap}}
   let _: Coat? = prop.iuo!.optional!()
   let _: Coat? = prop.iuo!.optional!()!
   let _: Coat = prop.iuo.optional()
   // expected-error@-1 {{value of optional type '(() -> Coat)?' must be unwrapped}}
   // expected-note@-2{{coalesce}}
   // expected-note@-3{{force-unwrap}}
   let _: Coat = prop.iuo.optional()!
-  // expected-error@-1 {{cannot invoke 'optional' with no arguments}}
+  // expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped to a value of type '() -> Coat?'}}
+  // expected-note@-2{{coalesce}}
+  // expected-note@-3{{force-unwrap}}
   let _: Coat = prop.iuo.optional!()
   let _: Coat = prop.iuo.optional!()!
   let _: Coat = prop.iuo!.optional()
@@ -34,7 +40,9 @@ func iuo_error(prop: IUOProperty) {
   // expected-note@-3{{force-unwrap}}
   
   let _: Coat = prop.iuo!.optional()!
-  // expected-error@-1 {{cannot invoke 'optional' with no arguments}}
+  // expected-error@-1 {{value of optional type '(() -> Coat?)?' must be unwrapped to a value of type '() -> Coat?'}}
+  // expected-note@-2{{coalesce}}
+  // expected-note@-3{{force-unwrap}}
   let _: Coat = prop.iuo!.optional!()
   let _: Coat = prop.iuo!.optional!()!
 
