Merge pull request #63904 from LucianoPAlmeida/ambiguous-overload

[Sema] Diagnose function coercion ambiguity

Author: LucianoPAlmeida

include/swift/AST/DiagnosticsSema.def

@@ -1403,10 +1403,13 @@ WARNING(unlabeled_trailing_closure_deprecated,Deprecation,
 NOTE(decl_multiple_defaulted_closure_parameters,none,
  "%0 contains defaulted closure parameters %1 and %2",
  (DeclName, Identifier, Identifier))
+NOTE(candidate_with_extraneous_args_closure,none,
+ "candidate %0 requires %1 argument%s1, "
+ "but %2 %select{were|was}3 used in closure body",
+ (Type, unsigned, unsigned, bool))
 NOTE(candidate_with_extraneous_args,none,
- "candidate %0 requires %1 argument%s1, "
- "but %2 %select{were|was}3 %select{provided|used in closure body}4",
- (Type, unsigned, unsigned, bool, bool))
+ "candidate %0 has %1 parameter%s1, but context %2 has %3",
+ (Type, unsigned, Type, unsigned))
 
 ERROR(no_accessible_initializers,none,
  "%0 cannot be constructed because it has no accessible initializers",

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -265,6 +265,9 @@ ABSTRACT_LOCATOR_PATH_ELT(PatternDecl)
 /// A function type global actor.
 SIMPLE_LOCATOR_PATH_ELT(GlobalActorType)
 
+/// A type coercion operand.
+SIMPLE_LOCATOR_PATH_ELT(CoercionOperand)
+
 #undef LOCATOR_PATH_ELT
 #undef CUSTOM_LOCATOR_PATH_ELT
 #undef SIMPLE_LOCATOR_PATH_ELT

lib/Sema/CSApply.cpp

@@ -4349,7 +4349,8 @@ namespace {
  // If we weren't explicitly told by the caller which disjunction choice,
  // get it from the solution to determine whether we've picked a coercion
  // or a bridging conversion.
- auto *locator = cs.getConstraintLocator(expr);
+ auto *locator =
+ cs.getConstraintLocator(expr, ConstraintLocator::CoercionOperand);
  auto choice = solution.getDisjunctionChoice(locator);
 
  // Handle the coercion/bridging of the underlying subexpression, where

lib/Sema/CSDiagnostics.cpp

@@ -282,6 +282,9 @@ ValueDecl *RequirementFailure::getDeclRef() const {
  return getAffectedDeclFromType(contextualTy);
  }
 
+ if (getLocator()->isFirstElement<LocatorPathElt::CoercionOperand>())
+ return getAffectedDeclFromType(getOwnerType());
+
  if (auto overload = getCalleeOverloadChoiceIfAvailable(getLocator())) {
  // If there is a declaration associated with this
  // failure e.g. an overload choice of the call
@@ -786,7 +789,8 @@ bool GenericArgumentsMismatchFailure::diagnoseAsError() {
  //
  // `value` has to get implicitly wrapped into 2 optionals
  // before pointer types could be compared.
- auto path = getLocator()->getPath();
+ auto locator = getLocator();
+ auto path = locator->getPath();
  unsigned toDrop = 0;
  for (const auto &elt : llvm::reverse(path)) {
  if (!elt.is<LocatorPathElt::OptionalPayload>())
@@ -802,7 +806,7 @@ bool GenericArgumentsMismatchFailure::diagnoseAsError() {
  if (path.empty()) {
  if (isExpr<AssignExpr>(anchor)) {
  diagnostic = getDiagnosticFor(CTP_AssignSource);
- } else if (isExpr<CoerceExpr>(anchor)) {
+ } else if (locator->isForCoercion()) {
  diagnostic = getDiagnosticFor(CTP_CoerceOperand);
  } else {
  return false;
@@ -887,6 +891,11 @@ bool GenericArgumentsMismatchFailure::diagnoseAsError() {
  break;
  }
 
+ case ConstraintLocator::CoercionOperand: {
+ diagnostic = getDiagnosticFor(CTP_CoerceOperand);
+ break;
+ }
+
  default:
  break;
  }
@@ -1219,6 +1228,14 @@ ASTNode InvalidCoercionFailure::getAnchor() const {
  return anchor;
 }
 
+SourceLoc InvalidCoercionFailure::getLoc() const {
+ if (getLocator()->isForCoercion()) {
+ auto *CE = castToExpr<CoerceExpr>(getRawAnchor());
+ return CE->getAsLoc();
+ }
+ return FailureDiagnostic::getLoc();
+}
+
 bool InvalidCoercionFailure::diagnoseAsError() {
  auto fromType = getFromType();
  auto toType = getToType();
@@ -2487,7 +2504,7 @@ bool ContextualFailure::diagnoseAsError() {
  diagnostic = diag::cannot_convert_condition_value;
  break;
  }
-  
+ case ConstraintLocator::CoercionOperand:
  case ConstraintLocator::InstanceType: {
  if (diagnoseCoercionToUnrelatedType())
  return true;
@@ -2809,7 +2826,7 @@ bool ContextualFailure::diagnoseConversionToNil() const {
  emitDiagnostic(diag::unresolved_nil_literal);
  return true;
  }
- } else if (isa<CoerceExpr>(parentExpr)) {
+ } else if (locator->isForCoercion()) {
  // `nil` is passed as a left-hand side of the coercion
  // operator e.g. `nil as Foo`
  CTP = CTP_CoerceOperand;
@@ -2891,23 +2908,23 @@ bool ContextualFailure::diagnoseExtraneousAssociatedValues() const {
 }
 
 bool ContextualFailure::diagnoseCoercionToUnrelatedType() const {
- auto anchor = getAnchor();
+ auto anchor = getRawAnchor();
+ auto *coerceExpr = getAsExpr<CoerceExpr>(anchor);
+ if (!coerceExpr) {
+ return false;
+ }
 
- if (auto *coerceExpr = getAsExpr<CoerceExpr>(anchor)) {
- const auto fromType = getType(coerceExpr->getSubExpr());
- const auto toType = getType(coerceExpr->getCastTypeRepr());
+ const auto fromType = getType(coerceExpr->getSubExpr());
+ const auto toType = getType(coerceExpr->getCastTypeRepr());
 
-  auto diagnostic = getDiagnosticFor(CTP_CoerceOperand, toType);
+ auto diagnostic = getDiagnosticFor(CTP_CoerceOperand, toType);
 
-  auto diag = emitDiagnostic(*diagnostic, fromType, toType);
-  diag.highlight(getSourceRange());
+ auto diag = emitDiagnostic(*diagnostic, fromType, toType);
+ diag.highlight(getSourceRange());
 
- (void)tryFixIts(diag);
- 
- return true;
- }
+ (void)tryFixIts(diag);
 
- return false;
+ return true;
 }
 
 bool ContextualFailure::diagnoseConversionToBool() const {
@@ -3178,9 +3195,6 @@ bool ContextualFailure::trySequenceSubsequenceFixIts(
  if (getFromType()->isSubstring()) {
  if (getToType()->isString()) {
  auto *anchor = castToExpr(getAnchor())->getSemanticsProvidingExpr();
- if (auto *CE = dyn_cast<CoerceExpr>(anchor)) {
- anchor = CE->getSubExpr();
- }
 
  if (auto *call = dyn_cast<CallExpr>(anchor)) {
  auto *fnExpr = call->getFn();
@@ -5860,9 +5874,15 @@ bool ExtraneousArgumentsFailure::diagnoseAsNote() {
 
  auto *decl = overload->choice.getDecl();
  auto numArgs = getTotalNumArguments();
- emitDiagnosticAt(decl, diag::candidate_with_extraneous_args, ContextualType,
- ContextualType->getNumParams(), numArgs, (numArgs == 1),
- isExpr<ClosureExpr>(getAnchor()));
+ if (isExpr<ClosureExpr>(getAnchor())) {
+ emitDiagnosticAt(decl, diag::candidate_with_extraneous_args_closure,
+ ContextualType, ContextualType->getNumParams(), numArgs,
+ (numArgs == 1));
+ } else {
+ emitDiagnosticAt(decl, diag::candidate_with_extraneous_args,
+ overload->adjustedOpenedType, numArgs, ContextualType,
+ ContextualType->getNumParams());
+ }
  return true;
 }
 
@@ -8878,12 +8898,7 @@ GlobalActorFunctionMismatchFailure::getDiagnosticMessage() const {
  auto path = locator->getPath();
 
  if (path.empty()) {
- auto anchor = getAnchor();
- if (isExpr<CoerceExpr>(anchor)) {
- return diag::cannot_convert_global_actor_coercion;
- } else {
- return diag::cannot_convert_global_actor;
- }
+ return diag::cannot_convert_global_actor;
  }
 
  auto last = path.back();
@@ -8901,6 +8916,9 @@ GlobalActorFunctionMismatchFailure::getDiagnosticMessage() const {
  case ConstraintLocator::TernaryBranch: {
  return diag::ternary_expr_cases_global_actor_mismatch;
  }
+ case ConstraintLocator::CoercionOperand: {
+ return diag::cannot_convert_global_actor_coercion;
+ }
  default:
  break;
  }

lib/Sema/CSDiagnostics.h

@@ -2177,6 +2177,8 @@ class InvalidCoercionFailure final : public ContextualFailure {
 
  ASTNode getAnchor() const override;
 
+ SourceLoc getLoc() const override;
+
  bool diagnoseAsError() override;
 };
 

lib/Sema/CSGen.cpp

@@ -3307,7 +3307,8 @@ namespace {
  if (repr) CS.setType(repr, toType);
 
  auto fromType = CS.getType(expr->getSubExpr());
- auto locator = CS.getConstraintLocator(expr);
+ auto locator =
+ CS.getConstraintLocator(expr, ConstraintLocator::CoercionOperand);
 
  // Literal initialization (e.g. `UInt32(0)`) doesn't require
  // a conversion because the literal is supposed to assume the
@@ -3327,8 +3328,11 @@ namespace {
 
  // If the result type was declared IUO, add a disjunction for
  // bindings for the result of the coercion.
- if (repr && repr->getKind() == TypeReprKind::ImplicitlyUnwrappedOptional)
- return createTypeVariableAndDisjunctionForIUOCoercion(toType, locator);
+ if (repr &&
+ repr->getKind() == TypeReprKind::ImplicitlyUnwrappedOptional) {
+ return createTypeVariableAndDisjunctionForIUOCoercion(
+ toType, CS.getConstraintLocator(expr));
+ }
 
  return toType;
  }

lib/Sema/CSSimplify.cpp

@@ -5058,63 +5058,6 @@ bool ConstraintSystem::repairFailures(
  if (!anchor)
  return false;
 
- if (auto *coercion = getAsExpr<CoerceExpr>(anchor)) {
- // Coercion from T.Type to T.Protocol.
- if (hasConversionOrRestriction(
- ConversionRestrictionKind::MetatypeToExistentialMetatype))
- return false;
-
- if (hasConversionOrRestriction(ConversionRestrictionKind::Superclass))
- return false;
-
- // Let's check whether the sub-expression is an optional type which
- // is possible to unwrap (either by force or `??`) to satisfy the cast,
- // otherwise we'd have to fallback to force downcast.
- if (repairViaOptionalUnwrap(*this, lhs, rhs, matchKind,
- conversionsOrFixes,
- getConstraintLocator(coercion->getSubExpr())))
- return true;
-
- // If the result type of the coercion has an value to optional conversion
- // we can instead suggest the conditional downcast as it is safer in
- // situations like conditional binding.
- auto useConditionalCast =
- llvm::any_of(ConstraintRestrictions, [&](const auto &restriction) {
- Type type1, type2;
- std::tie(type1, type2) = restriction.first;
- auto restrictionKind = restriction.second;
-
- if (restrictionKind != ConversionRestrictionKind::ValueToOptional)
- return false;
-
- return rhs->isEqual(type1);
- });
-
- // Repair a coercion ('as') with a runtime checked cast ('as!' or 'as?').
- if (auto *coerceToCheckCastFix =
- CoerceToCheckedCast::attempt(*this, lhs, rhs, useConditionalCast,
- getConstraintLocator(locator))) {
- conversionsOrFixes.push_back(coerceToCheckCastFix);
- return true;
- }
-
- // If it has a deep equality restriction, defer the diagnostic to
- // GenericMismatch.
- if (hasConversionOrRestriction(ConversionRestrictionKind::DeepEquality) &&
- !hasConversionOrRestriction(
- ConversionRestrictionKind::OptionalToOptional)) {
- return false;
- }
-
- if (hasConversionOrRestriction(ConversionRestrictionKind::Existential))
- return false;
-
- auto *fix = ContextualMismatch::create(*this, lhs, rhs,
- getConstraintLocator(locator));
- conversionsOrFixes.push_back(fix);
- return true;
- }
-
  // This could be:
  // - `InOutExpr` used with r-value e.g. `foo(&x)` where `x` is a `let`.
  // - `ForceValueExpr` e.g. `foo.bar! = 42` where `bar` or `foo` are
@@ -6449,6 +6392,64 @@ bool ConstraintSystem::repairFailures(
  break;
  }
 
+ case ConstraintLocator::CoercionOperand: {
+ auto *coercion = castToExpr<CoerceExpr>(anchor);
+
+ // Coercion from T.Type to T.Protocol.
+ if (hasConversionOrRestriction(
+ ConversionRestrictionKind::MetatypeToExistentialMetatype))
+ return false;
+
+ if (hasConversionOrRestriction(ConversionRestrictionKind::Superclass))
+ return false;
+
+ // Let's check whether the sub-expression is an optional type which
+ // is possible to unwrap (either by force or `??`) to satisfy the cast,
+ // otherwise we'd have to fallback to force downcast.
+ if (repairViaOptionalUnwrap(*this, lhs, rhs, matchKind,
+ conversionsOrFixes,
+ getConstraintLocator(coercion->getSubExpr())))
+ return true;
+
+ // If the result type of the coercion has an value to optional conversion
+ // we can instead suggest the conditional downcast as it is safer in
+ // situations like conditional binding.
+ auto useConditionalCast =
+ llvm::any_of(ConstraintRestrictions, [&](const auto &restriction) {
+ Type type1, type2;
+ std::tie(type1, type2) = restriction.first;
+ auto restrictionKind = restriction.second;
+
+ if (restrictionKind != ConversionRestrictionKind::ValueToOptional)
+ return false;
+
+ return rhs->isEqual(type1);
+ });
+
+ // Repair a coercion ('as') with a runtime checked cast ('as!' or 'as?').
+ if (auto *coerceToCheckCastFix =
+ CoerceToCheckedCast::attempt(*this, lhs, rhs, useConditionalCast,
+ getConstraintLocator(locator))) {
+ conversionsOrFixes.push_back(coerceToCheckCastFix);
+ return true;
+ }
+
+ // If it has a deep equality restriction, defer the diagnostic to
+ // GenericMismatch.
+ if (hasConversionOrRestriction(ConversionRestrictionKind::DeepEquality) &&
+ !hasConversionOrRestriction(
+ ConversionRestrictionKind::OptionalToOptional)) {
+ return false;
+ }
+
+ if (hasConversionOrRestriction(ConversionRestrictionKind::Existential))
+ return false;
+
+ auto *fix = ContextualMismatch::create(*this, lhs, rhs,
+ getConstraintLocator(locator));
+ conversionsOrFixes.push_back(fix);
+ return true;
+ }
  default:
  break;
  }
@@ -6963,7 +6964,8 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
  ArrayRef<LocatorPathElt> path) {
  // E.g. contextual conversion from coercion/cast
  // to some other type.
- if (!path.empty())
+ if (!(path.empty() ||
+ path.back().is<LocatorPathElt::CoercionOperand>()))
  return false;
 
  return isExpr<CoerceExpr>(anchor) || isExpr<IsExpr>(anchor) ||
@@ -11328,7 +11330,7 @@ ConstraintSystem::simplifyBridgingConstraint(Type type1,
 
  SmallVector<LocatorPathElt, 4> elts;
  auto anchor = locator.getLocatorParts(elts);
- if (!elts.empty())
+ if (elts.empty() || !elts.back().is<LocatorPathElt::CoercionOperand>())
  return false;
 
  auto *coercion = getAsExpr<CoerceExpr>(anchor);