Skip to content

[Sema] Do not diagnose contextual type mismatches for malformed key path expressions #33230

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
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
8 changes: 7 additions & 1 deletion include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -5648,7 +5648,13 @@ class KeyPathExpr : public Expr {
/// components from the argument array.
void resolveComponents(ASTContext &C,
ArrayRef<Component> resolvedComponents);


/// Indicates if the key path expression is composed by a single invalid
/// component. e.g. missing component `\Root`
bool hasSingleInvalidComponent() const {
return Components.size() == 1 && !Components.front().isValid();
}

/// Retrieve the string literal expression, which will be \c NULL prior to
/// type checking and a string literal after type checking for an
/// @objc key path.
Expand Down
6 changes: 6 additions & 0 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,12 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
} else if (srcLocator->directlyAt<ObjectLiteralExpr>()) {
fix = SpecifyObjectLiteralTypeImport::create(cs, dstLocator);
} else if (srcLocator->isKeyPathRoot()) {
// If we recorded an invalid key path fix, let's skip this specify root
// type fix because it wouldn't produce a useful diagnostic.
auto *kpLocator = cs.getConstraintLocator(srcLocator->getAnchor());
if (cs.hasFixFor(kpLocator, FixKind::AllowKeyPathWithoutComponents))
return true;

fix = SpecifyKeyPathRootType::create(cs, dstLocator);
}

Expand Down
16 changes: 16 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6760,3 +6760,19 @@ void TrailingClosureRequiresExplicitLabel::fixIt(
diagnostic.fixItInsert(newRParenLoc,
isExpr<SubscriptExpr>(anchor) ? "]" : ")");
}

bool InvalidEmptyKeyPathFailure::diagnoseAsError() {
auto *KPE = getAsExpr<KeyPathExpr>(getAnchor());
assert(KPE && KPE->hasSingleInvalidComponent() &&
"Expected a malformed key path expression");

// If we have a string interpolation represented as key path expressions
// e.g. \(x), \(x, a: 1). Let's skip it because this would be already
// diagnosed and it is not the case for an extra empty key path diagnostic.
auto *root = KPE->getParsedRoot();
if (root && (isa<ParenExpr>(root) || isa<TupleExpr>(root)))
return true;

emitDiagnostic(diag::expr_swift_keypath_empty);
return true;
}
14 changes: 14 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -2248,6 +2248,20 @@ class TrailingClosureRequiresExplicitLabel final : public FailureDiagnostic {
const FunctionArgApplyInfo &info) const;
};

/// Diagnose situations where we have a key path with no components.
///
/// \code
/// let _ : KeyPath<A, B> = \A
/// \endcode
class InvalidEmptyKeyPathFailure final : public FailureDiagnostic {
public:
InvalidEmptyKeyPathFailure(const Solution &solution,
ConstraintLocator *locator)
: FailureDiagnostic(solution, locator) {}

bool diagnoseAsError() override;
};

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

Expand Down
12 changes: 12 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1539,3 +1539,15 @@ SpecifyLabelToAssociateTrailingClosure::create(ConstraintSystem &cs,
return new (cs.getAllocator())
SpecifyLabelToAssociateTrailingClosure(cs, locator);
}

bool AllowKeyPathWithoutComponents::diagnose(const Solution &solution,
bool asNote) const {
InvalidEmptyKeyPathFailure failure(solution, getLocator());
return failure.diagnose(asNote);
}

AllowKeyPathWithoutComponents *
AllowKeyPathWithoutComponents::create(ConstraintSystem &cs,
ConstraintLocator *locator) {
return new (cs.getAllocator()) AllowKeyPathWithoutComponents(cs, locator);
}
24 changes: 23 additions & 1 deletion lib/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,16 @@ enum class FixKind : uint8_t {

/// Specify key path root type when it cannot be infered from context.
SpecifyKeyPathRootType,

/// Unwrap optional base on key path application.
UnwrapOptionalBaseKeyPathApplication,

/// Explicitly specify a label to match trailing closure to a certain
/// parameter in the call.
SpecifyLabelToAssociateTrailingClosure,

/// Allow key path expressions with no components.
AllowKeyPathWithoutComponents,
};

class ConstraintFix {
Expand Down Expand Up @@ -1958,6 +1961,25 @@ class SpecifyLabelToAssociateTrailingClosure final : public ConstraintFix {
create(ConstraintSystem &cs, ConstraintLocator *locator);
};

/// Diagnose situations where we have a key path with no components.
///
/// \code
/// let _ : KeyPath<A, B> = \A
/// \endcode
class AllowKeyPathWithoutComponents final : public ConstraintFix {
AllowKeyPathWithoutComponents(ConstraintSystem &cs,
ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::AllowKeyPathWithoutComponents, locator) {}

public:
std::string getName() const override { return "key path missing component"; }

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

static AllowKeyPathWithoutComponents *create(ConstraintSystem &cs,
ConstraintLocator *locator);
};

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

Expand Down
11 changes: 10 additions & 1 deletion lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8127,6 +8127,14 @@ ConstraintSystem::simplifyKeyPathConstraint(
if (keyPathTy->isHole())
return SolutionKind::Solved;

// If we have a malformed KeyPathExpr e.g. let _: KeyPath<A, C> = \A
// let's record a AllowKeyPathMissingComponent fix.
if (keyPath->hasSingleInvalidComponent()) {
auto *fix = AllowKeyPathWithoutComponents::create(
*this, getConstraintLocator(locator));
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
}

// If the root type has been bound to a hole, we cannot infer it.
if (getFixedTypeRecursive(rootTy, /*wantRValue*/ true)->isHole())
return SolutionKind::Solved;
Expand Down Expand Up @@ -9988,7 +9996,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::UnwrapOptionalBaseKeyPathApplication:
case FixKind::AllowCoercionToForceCast:
case FixKind::SpecifyKeyPathRootType:
case FixKind::SpecifyLabelToAssociateTrailingClosure: {
case FixKind::SpecifyLabelToAssociateTrailingClosure:
case FixKind::AllowKeyPathWithoutComponents: {
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
}

Expand Down
1 change: 0 additions & 1 deletion lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1900,7 +1900,6 @@ void PreCheckExpression::resolveKeyPathExpr(KeyPathExpr *KPE) {

// Key paths must be spelled with at least one component.
if (components.empty()) {
DE.diagnose(KPE->getLoc(), diag::expr_swift_keypath_empty);
// Passes further down the pipeline expect keypaths to always have at least
// one component, so stuff an invalid component in the AST for recovery.
components.push_back(KeyPathExpr::Component());
Expand Down
12 changes: 7 additions & 5 deletions test/expr/unary/keypath/keypath.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct A: Hashable {
func hash(into hasher: inout Hasher) { fatalError() }
}
struct B {}
struct C<T> { // expected-note 3 {{'T' declared as parameter to type 'C'}}
struct C<T> { // expected-note 4 {{'T' declared as parameter to type 'C'}}
var value: T
subscript() -> T { get { return value } }
subscript(sub: Sub) -> T { get { return value } set { } }
Expand Down Expand Up @@ -224,15 +224,17 @@ func testKeyPathInGenericContext<H: Hashable, X>(hashable: H, anything: X) {
}

func testDisembodiedStringInterpolation(x: Int) {
\(x) // expected-error{{string interpolation}} expected-error{{}}
\(x, radix: 16) // expected-error{{string interpolation}} expected-error{{}}
\(x) // expected-error{{string interpolation can only appear inside a string literal}}
\(x, radix: 16) // expected-error{{string interpolation can only appear inside a string literal}}
}

func testNoComponents() {
let _: KeyPath<A, A> = \A // expected-error{{must have at least one component}}
let _: KeyPath<C, A> = \C // expected-error{{must have at least one component}} expected-error{{}}
let _: KeyPath<C, A> = \C // expected-error{{must have at least one component}}
// expected-error@-1 {{generic parameter 'T' could not be inferred}}
// expected-error@-2 {{cannot convert value of type 'KeyPath<Root, Value>' to specified type 'KeyPath<C<T>, A>'}}
let _: KeyPath<A, C> = \A // expected-error{{must have at least one component}}
// expected-error@-1 {{generic parameter 'T' could not be inferred}}
_ = \A // expected-error {{key path must have at least one component}}
}

struct TupleStruct {
Expand Down