Skip to content

[5.3][SR-12425] Improving diagnostics for key path application on non-convertible root type #31233

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.

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
Apr 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,9 @@ ERROR(expr_keypath_subscript_index_not_hashable, none,
ERROR(expr_smart_keypath_application_type_mismatch,none,
"key path of type %0 cannot be applied to a base of type %1",
(Type, Type))
ERROR(expr_keypath_root_type_mismatch, none,
"key path with root type %0 cannot be applied to a base of type %1",
(Type, Type))
ERROR(expr_swift_keypath_anyobject_root,none,
"the root type of a Swift key path cannot be 'AnyObject'", ())
WARNING(expr_deprecated_writable_keypath,none,
Expand Down
12 changes: 12 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6224,3 +6224,15 @@ bool CoercionAsForceCastFailure::diagnoseAsError() {
.highlight(getSourceRange());
return true;
}

bool KeyPathRootTypeMismatchFailure::diagnoseAsError() {
auto locator = getLocator();
assert(locator->isKeyPathRoot() && "Expected a key path root");

auto baseType = getFromType();
auto rootType = getToType();

emitDiagnostic(diag::expr_keypath_root_type_mismatch,
rootType, baseType);
return true;
}
18 changes: 18 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -2022,6 +2022,24 @@ class CoercionAsForceCastFailure final : public ContextualFailure {

bool diagnoseAsError() override;
};

/// Diagnose a key path root type that cannot be applied to an instance
/// base that has another type.
///
/// \code
/// func f(_ bar: Bar , keyPath: KeyPath<Foo, Int> ) {
/// bar[keyPath: keyPath]
/// }
/// \endcode
class KeyPathRootTypeMismatchFailure final : public ContextualFailure {
public:
KeyPathRootTypeMismatchFailure(const Solution &solution, Type lhs, Type rhs,
ConstraintLocator *locator)
: ContextualFailure(solution, lhs, rhs, locator) {}

bool diagnoseAsError() override;
};

} // end namespace constraints
} // end namespace swift

Expand Down
14 changes: 14 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1293,3 +1293,17 @@ AllowCoercionToForceCast::create(ConstraintSystem &cs, Type fromType,
return new (cs.getAllocator())
AllowCoercionToForceCast(cs, fromType, toType, locator);
}

bool AllowKeyPathRootTypeMismatch::diagnose(const Solution &solution,
bool asNote) const {
KeyPathRootTypeMismatchFailure failure(solution, getFromType(), getToType(),
getLocator());
return failure.diagnose(asNote);
}

AllowKeyPathRootTypeMismatch *
AllowKeyPathRootTypeMismatch::create(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator) {
return new (cs.getAllocator())
AllowKeyPathRootTypeMismatch(cs, lhs, rhs, locator);
}
30 changes: 30 additions & 0 deletions lib/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,10 @@ enum class FixKind : uint8_t {

/// A warning fix that allows a coercion to perform a force-cast.
AllowCoercionToForceCast,

/// Allow key path root type mismatch when applying a key path that has a
/// root type not convertible to the type of the base instance.
AllowKeyPathRootTypeMismatch,
};

class ConstraintFix {
Expand Down Expand Up @@ -1768,6 +1772,32 @@ class AllowCoercionToForceCast final : public ContextualMismatch {
ConstraintLocator *locator);
};

/// Attempt to fix a key path application where the key path type cannot be
/// applied to a base instance of another type.
///
/// \code
/// func f(_ bar: Bar , keyPath: KeyPath<Foo, Int> ) {
/// bar[keyPath: keyPath]
/// }
/// \endcode
class AllowKeyPathRootTypeMismatch : public ContextualMismatch {
protected:
AllowKeyPathRootTypeMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator)
: ContextualMismatch(cs, FixKind::AllowKeyPathRootTypeMismatch, lhs, rhs,
locator) {}

public:
std::string getName() const override {
return "allow key path root type mismatch";
}

bool diagnose(const Solution &solution, bool asNote = false) const override;

static AllowKeyPathRootTypeMismatch *
create(ConstraintSystem &cs, Type lhs, Type rhs, ConstraintLocator *locator);
};

} // end namespace constraints
} // end namespace swift

Expand Down
13 changes: 11 additions & 2 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3577,6 +3577,13 @@ bool ConstraintSystem::repairFailures(
break;
}

case ConstraintLocator::KeyPathRoot: {
conversionsOrFixes.push_back(AllowKeyPathRootTypeMismatch::create(
*this, lhs, rhs, getConstraintLocator(locator)));

break;
}

case ConstraintLocator::FunctionArgument: {
auto *argLoc = getConstraintLocator(
locator.withPathElement(LocatorPathElt::SynthesizedArgument(0)));
Expand Down Expand Up @@ -7829,8 +7836,9 @@ ConstraintSystem::simplifyKeyPathApplicationConstraint(
rootTy = getFixedTypeRecursive(rootTy, flags, /*wantRValue=*/false);

auto matchRoot = [&](ConstraintKind kind) -> bool {
auto rootMatches = matchTypes(rootTy, kpRootTy, kind,
subflags, locator);
auto rootMatches =
matchTypes(rootTy, kpRootTy, kind, subflags,
locator.withPathElement(LocatorPathElt::KeyPathRoot()));
switch (rootMatches) {
case SolutionKind::Error:
return false;
Expand Down Expand Up @@ -9392,6 +9400,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::SpecifyBaseTypeForContextualMember:
case FixKind::CoerceToCheckedCast:
case FixKind::SpecifyObjectLiteralTypeImport:
case FixKind::AllowKeyPathRootTypeMismatch:
case FixKind::AllowCoercionToForceCast: {
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
}
Expand Down
15 changes: 11 additions & 4 deletions test/attr/attr_dynamic_member_lookup.swift
Original file line number Diff line number Diff line change
Expand Up @@ -811,9 +811,16 @@ internal var rightStructInstance: SR12425_R = SR12425_R()

public extension SR12425_R {
subscript<T>(dynamicMember member: WritableKeyPath<SR12425_S, T>) -> T {
// TODO(Diagnostics): bad diagnostic for member assign.
// A better diagnostic would be: key path of type WritableKeyPath<SR12425_S, T> cannot be applied to a base of type SR12425_R
get { rightStructInstance[keyPath: member] } // expected-error {{cannot convert return expression of type 'Any?' to return type 'T'}}
set { rightStructInstance[keyPath: member] = newValue } // expected-error {{type of expression is ambiguous without more context}}
get { rightStructInstance[keyPath: member] } // expected-error {{key path with root type 'SR12425_S' cannot be applied to a base of type 'SR12425_R'}}
set { rightStructInstance[keyPath: member] = newValue } // expected-error {{key path with root type 'SR12425_S' cannot be applied to a base of type 'SR12425_R'}}
}
}

@dynamicMemberLookup
public struct SR12425_R1 {}

public extension SR12425_R1 {
subscript<T>(dynamicMember member: KeyPath<SR12425_R1, T>) -> T {
get { rightStructInstance[keyPath: member] } // expected-error {{key path with root type 'SR12425_R1' cannot be applied to a base of type 'SR12425_R'}}
}
}