Skip to content

Commit

Permalink
[ConstraintSystem] Allow simplifyRestrictedConstraintImpl to diagno…
Browse files Browse the repository at this point in the history
…se contextual failures with optionals

Since `simplifyRestrictedConstraintImpl` has both parent types and
does nested type matching it's a good place to diagnose top-level
contextual problems like mismatches in underlying types of optionals.
  • Loading branch information
xedin committed Feb 18, 2020
1 parent 0516c3b commit 0a8de8b
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 21 deletions.
60 changes: 44 additions & 16 deletions lib/Sema/CSSimplify.cpp
Expand Up @@ -3248,6 +3248,11 @@ bool ConstraintSystem::repairFailures(
}))
break;

if (hasConversionOrRestriction(
ConversionRestrictionKind::ValueToOptional) ||
hasConversionOrRestriction(ConversionRestrictionKind::PointerToPointer))
break;

// If this is something like `[A] argument conv [B]` where `A` and `B`
// are unrelated types, let's give `matchTypes` a chance to consider
// element types.
Expand All @@ -3259,6 +3264,9 @@ bool ConstraintSystem::repairFailures(
if (hasConversionOrRestriction(ConversionRestrictionKind::Existential))
break;

if (hasConversionOrRestriction(ConversionRestrictionKind::Superclass))
break;

// If there implicit 'something-to-pointer' conversions involved,
// such conversions are going to be diagnosed by specialized fix
// which deals with generic argument mismatches.
Expand Down Expand Up @@ -8280,16 +8288,27 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
auto fixContextualFailure = [&](Type fromType, Type toType,
ConstraintLocatorBuilder locator) -> bool {
auto *loc = getConstraintLocator(locator);
// Since this is a contextual type mismatch, let's start from higher
// impact than regular fix to avoid ambiguities.
auto impact = 2;
if (loc->isForAssignment() || loc->isForCoercion() ||
loc->isForContextualType()) {
loc->isForContextualType() ||
loc->isLastElement<LocatorPathElt::ApplyArgToParam>()) {
if (restriction == ConversionRestrictionKind::Superclass) {
if (auto *fix =
CoerceToCheckedCast::attempt(*this, fromType, toType, loc))
return !recordFix(fix);
return !recordFix(fix, impact);
}

auto *fix = ContextualMismatch::create(*this, fromType, toType, loc);
return !recordFix(fix);
if (restriction == ConversionRestrictionKind::ValueToOptional ||
restriction == ConversionRestrictionKind::OptionalToOptional)
++impact;

auto *fix =
loc->isLastElement<LocatorPathElt::ApplyArgToParam>()
? AllowArgumentMismatch::create(*this, fromType, toType, loc)
: ContextualMismatch::create(*this, fromType, toType, loc);
return !recordFix(fix, impact);
}

return false;
Expand Down Expand Up @@ -8378,14 +8397,18 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
assert(matchKind >= ConstraintKind::Subtype);
if (auto generic2 = type2->getAs<BoundGenericType>()) {
if (generic2->getDecl()->isOptionalDecl()) {
return matchTypes(type1, generic2->getGenericArgs()[0],
matchKind, subflags,
locator.withPathElement(
ConstraintLocator::OptionalPayload));
auto result = matchTypes(
type1, generic2->getGenericArgs()[0], matchKind, subflags,
locator.withPathElement(ConstraintLocator::OptionalPayload));

if (!(shouldAttemptFixes() && result.isFailure()))
return result;
}
}

return SolutionKind::Error;
return shouldAttemptFixes() && fixContextualFailure(type1, type2, locator)
? SolutionKind::Solved
: SolutionKind::Error;
}

// for $< in { <, <c, <oc }:
Expand All @@ -8401,16 +8424,21 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
if (auto generic1 = type1->getAs<BoundGenericType>()) {
if (auto generic2 = type2->getAs<BoundGenericType>()) {
if (generic1->getDecl()->isOptionalDecl() &&
generic2->getDecl()->isOptionalDecl())
return matchTypes(generic1->getGenericArgs()[0],
generic2->getGenericArgs()[0],
matchKind, subflags,
locator.withPathElement(
LocatorPathElt::GenericArgument(0)));
generic2->getDecl()->isOptionalDecl()) {
auto result = matchTypes(
generic1->getGenericArgs()[0], generic2->getGenericArgs()[0],
matchKind, subflags,
locator.withPathElement(LocatorPathElt::GenericArgument(0)));

if (!(shouldAttemptFixes() && result.isFailure()))
return result;
}
}
}

return SolutionKind::Error;
return shouldAttemptFixes() && fixContextualFailure(type1, type2, locator)
? SolutionKind::Solved
: SolutionKind::Error;
}

case ConversionRestrictionKind::ClassMetatypeToAnyObject:
Expand Down
8 changes: 6 additions & 2 deletions test/ClangImporter/cf.swift
Expand Up @@ -118,10 +118,14 @@ func testOutParametersBad() {
CCRefrigeratorCreateIndirect(fridge) // expected-error {{cannot convert value of type 'CCRefrigerator?' to expected argument type 'UnsafeMutablePointer<CCRefrigerator?>?'}}

let power: CCPowerSupply?
CCRefrigeratorGetPowerSupplyIndirect(0, power) // expected-error {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator?'}}
CCRefrigeratorGetPowerSupplyIndirect(0, power)
// expected-error@-1:40 {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator?'}}
// expected-error@-2:43 {{cannot convert value of type 'CCPowerSupply?' to expected argument type 'AutoreleasingUnsafeMutablePointer<CCPowerSupply?>'}}

let item: CCItem?
CCRefrigeratorGetItemUnaudited(0, 0, item) // expected-error {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator?'}}
CCRefrigeratorGetItemUnaudited(0, 0, item)
// expected-error@-1:34 {{cannot convert value of type 'Int' to expected argument type 'CCRefrigerator?'}}
// expected-error@-2:40 {{cannot convert value of type 'CCItem?' to expected argument type 'UnsafeMutablePointer<Unmanaged<CCItem>?>?'}}
}

func nameCollisions() {
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/tuple_arguments.swift
Expand Up @@ -1679,7 +1679,7 @@ do {
f(a: log() as ((()) -> Void)?) // expected-error {{cannot convert value of type '((()) -> Void)?' to expected argument type '(() -> Void)?'}}

func logNoOptional<T>() -> (T) -> Void { }
f(a: logNoOptional() as ((()) -> Void)) // expected-error {{cannot convert value of type '(()) -> Void' to expected argument type '(() -> Void)?'}}
f(a: logNoOptional() as ((()) -> Void)) // expected-error {{cannot convert value of type '(()) -> Void' to expected argument type '() -> Void'}}

func g() {}
g(()) // expected-error {{argument passed to call that takes no arguments}}
Expand Down
2 changes: 1 addition & 1 deletion test/expr/closure/trailing.swift
Expand Up @@ -108,7 +108,7 @@ func labeledArgumentAndTrailingClosure() {

// Trailing closure binds to last parameter, always.
takeTwoFuncsWithDefaults { "Hello, " + $0 }
takeTwoFuncsWithDefaults { $0 + 1 } // expected-error {{cannot convert value of type '(Int) -> Int' to expected argument type '((String) -> String)?'}}
takeTwoFuncsWithDefaults { $0 + 1 } // expected-error {{cannot convert value of type 'Int' to expected argument type 'String'}}
takeTwoFuncsWithDefaults(f1: {$0 + 1 })
}

Expand Down
3 changes: 2 additions & 1 deletion test/expr/expressions.swift
Expand Up @@ -776,7 +776,8 @@ func testNilCoalescePrecedence(cond: Bool, a: Int?, r: ClosedRange<Int>?) {
// ?? should have higher precedence than logical operators like || and comparisons.
if cond || (a ?? 42 > 0) {} // Ok.
if (cond || a) ?? 42 > 0 {} // expected-error {{cannot be used as a boolean}} {{15-15=(}} {{16-16= != nil)}}
// expected-error@-1:12 {{cannot convert value of type 'Bool' to expected argument type 'Int?'}}
// expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
// expected-error@-2 {{binary operator '>' cannot be applied to operands of type 'Bool' and 'Int'}}
if (cond || a) ?? (42 > 0) {} // expected-error {{cannot be used as a boolean}} {{15-15=(}} {{16-16= != nil)}}

if cond || a ?? 42 > 0 {} // Parses as the first one, not the others.
Expand Down

0 comments on commit 0a8de8b

Please sign in to comment.