Skip to content

[Diagnostics] Centralize requirement failure impact assessment #27390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 28, 2019
Merged

[Diagnostics] Centralize requirement failure impact assessment #27390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 61 additions & 17 deletions lib/Sema/CSSimplify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,56 @@ static ConstraintFix *fixRequirementFailure(ConstraintSystem &cs, Type type1,
return nullptr;
}

static unsigned
assessRequirementFailureImpact(ConstraintSystem &cs, Type requirementType,
ConstraintLocatorBuilder locator) {
auto *anchor = locator.getAnchor();
if (!anchor)
return 1;

// If this requirement is associated with an overload choice let's
// tie impact to how many times this requirement type is mentioned.
if (auto *ODRE = dyn_cast<OverloadedDeclRefExpr>(anchor)) {
if (!(requirementType && requirementType->is<TypeVariableType>()))
return 1;

unsigned choiceImpact = 0;
if (auto *choice = cs.findSelectedOverloadFor(ODRE)) {
auto *typeVar = requirementType->castTo<TypeVariableType>();
choice->ImpliedType.visit([&](Type type) {
if (type->isEqual(typeVar))
++choiceImpact;
});
}

return choiceImpact == 0 ? 1 : choiceImpact;
}

// If this requirement is associated with a member reference and it
// was possible to check it before overload choice is bound, that means
// types came from the context (most likely Self, or associated type(s))
// and failing this constraint makes member unrelated/inaccessible, so
// the impact has to be adjusted accordingly in order for this fix not to
// interfere with other overload choices.
//
// struct S<T> {}
// extension S where T == AnyObject { func foo() {} }
//
// func bar(_ s: S<Int>) { s.foo() }
//
// In this case `foo` is only accessible if T == `AnyObject`, which makes
// fix for same-type requirement higher impact vs. requirement associated
// with method itself e.g. `func foo<U>() -> U where U : P {}` because
// `foo` is accessible from any `S` regardless of what `T` is.
if (isa<UnresolvedDotExpr>(anchor) || isa<UnresolvedMemberExpr>(anchor)) {
auto *calleeLoc = cs.getCalleeLocator(cs.getConstraintLocator(locator));
if (!cs.findSelectedOverloadFor(calleeLoc))
return 10;
}

return 1;
}

/// Attempt to fix missing arguments by introducing type variables
/// and inferring their types from parameters.
static bool fixMissingArguments(ConstraintSystem &cs, Expr *anchor,
Expand Down Expand Up @@ -4130,20 +4180,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(

if (auto *fix =
fixRequirementFailure(*this, type, protocolTy, anchor, path)) {
unsigned choiceImpact = 0;
// If this requirement is associated with overload choice let's
// tie impact to how many times this non-conforming type is mentioned.
if (auto *ODRE = dyn_cast_or_null<OverloadedDeclRefExpr>(anchor)) {
auto *choice = findSelectedOverloadFor(ODRE);
if (typeVar && choice) {
choice->ImpliedType.visit([&](Type type) {
if (type->isEqual(typeVar))
++choiceImpact;
});
}
}

if (!recordFix(fix, choiceImpact == 0 ? 1 : choiceImpact)) {
auto impact = assessRequirementFailureImpact(*this, typeVar, locator);
if (!recordFix(fix, impact)) {
// Record this conformance requirement as "fixed".
recordFixedRequirement(type, RequirementKind::Conformance,
protocolTy);
Expand Down Expand Up @@ -7639,11 +7677,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(

case FixKind::InsertCall:
case FixKind::RemoveReturn:
case FixKind::AddConformance:
case FixKind::RemoveAddressOf:
case FixKind::TreatRValueAsLValue:
case FixKind::SkipSameTypeRequirement:
case FixKind::SkipSuperclassRequirement:
case FixKind::AddMissingArguments:
case FixKind::SkipUnhandledConstructInFunctionBuilder:
case FixKind::UsePropertyWrapper:
Expand All @@ -7654,6 +7689,15 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
}

case FixKind::AddConformance:
case FixKind::SkipSameTypeRequirement:
case FixKind::SkipSuperclassRequirement: {
return recordFix(fix, assessRequirementFailureImpact(*this, type1,
fix->getLocator()))
? SolutionKind::Error
: SolutionKind::Solved;
}

case FixKind::AllowArgumentTypeMismatch: {
increaseScore(SK_Fix);
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
Expand Down
6 changes: 6 additions & 0 deletions test/Constraints/generics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,3 +830,9 @@ func test_identification_of_key_path_component_callees() {
_ = \SR11435<Int>.foo // expected-error {{property 'foo' requires that 'Int' conform to 'P'}}
_ = \SR11435<Int>.[x: 5] // expected-error {{subscript 'subscript(x:)' requires that 'Int' conform to 'P'}}
}

func sr_11491(_ value: [String]) {
var arr: Set<String> = []
arr.insert(value)
// expected-error@-1 {{cannot convert value of type '[String]' to expected argument type 'String'}}
}
25 changes: 16 additions & 9 deletions test/stdlib/KeyPathAppending.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,31 +50,38 @@ func mismatchedAppends<T, U, V>(readOnlyLeft: KeyPath<T, U>,
writableRight: WritableKeyPath<U, V>,
referenceRight: ReferenceWritableKeyPath<U, V>){
_ = readOnlyRight.appending(path: readOnlyLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'KeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{cannot convert value of type 'KeyPath<T, U>' to expected argument type 'KeyPath<V, U>'}}
// expected-note@-2 {{arguments to generic parameter 'Root' ('T' and 'V') are expected to be equal}}

_ = readOnlyRight.appending(path: writableLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'KeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{cannot convert value of type 'KeyPath<T, U>' to expected argument type 'KeyPath<V, U>'}}
// expected-note@-2 {{arguments to generic parameter 'Root' ('T' and 'V') are expected to be equal}}

_ = readOnlyRight.appending(path: referenceLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'KeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{cannot convert value of type 'ReferenceWritableKeyPath<T, U>' to expected argument type 'ReferenceWritableKeyPath<V, U>'}}
// expected-note@-2 {{arguments to generic parameter 'Root' ('T' and 'V') are expected to be equal}}

_ = writableRight.appending(path: readOnlyLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'WritableKeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{instance method 'appending(path:)' requires that 'KeyPath<U, V>' inherit from 'KeyPath<U, T>'}}

_ = writableRight.appending(path: writableLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'WritableKeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{cannot convert value of type 'WritableKeyPath<T, U>' to expected argument type 'WritableKeyPath<V, U>'}}
// expected-note@-2 {{arguments to generic parameter 'Root' ('T' and 'V') are expected to be equal}}

_ = writableRight.appending(path: referenceLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'WritableKeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{cannot convert value of type 'ReferenceWritableKeyPath<T, U>' to expected argument type 'ReferenceWritableKeyPath<V, U>'}}
// expected-note@-2 {{arguments to generic parameter 'Root' ('T' and 'V') are expected to be equal}}

_ = referenceRight.appending(path: readOnlyLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'ReferenceWritableKeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{instance method 'appending(path:)' requires that 'KeyPath<U, V>' inherit from 'KeyPath<U, T>'}}

_ = referenceRight.appending(path: writableLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'ReferenceWritableKeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{cannot convert value of type 'WritableKeyPath<T, U>' to expected argument type 'WritableKeyPath<V, U>'}}
// expected-note@-2 {{arguments to generic parameter 'Root' ('T' and 'V') are expected to be equal}}

_ = referenceRight.appending(path: referenceLeft)
// expected-error@-1 {{referencing instance method 'appending(path:)' on '_AppendKeyPath' requires the types 'ReferenceWritableKeyPath<U, V>' and 'AnyKeyPath' be equivalent}}
// expected-error@-1 {{cannot convert value of type 'WritableKeyPath<T, U>' to expected argument type 'WritableKeyPath<V, U>'}}
// expected-note@-2 {{arguments to generic parameter 'Root' ('T' and 'V') are expected to be equal}}
}

func partialAppends<T, U, V>(partial: PartialKeyPath<T>,
Expand Down