Skip to content

Commit 8793970

Browse files
committed
Sema: Don't crash when recovering type errors from malformed keypath expressions.
It's particularly likely someone will try to type `\(foo)`, which looks like a string interpolation segment, outside of a string literal, so give that case a special diagnostic. Fixes rdar://problem/32315365.
1 parent 8501af3 commit 8793970

File tree

12 files changed

+53
-6
lines changed

12 files changed

+53
-6
lines changed

include/swift/AST/DiagnosticsSema.def

+2
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,8 @@ ERROR(expr_swift_keypath_unimplemented_component,none,
450450
ERROR(expr_smart_keypath_value_covert_to_contextual_type,none,
451451
"KeyPath value type %0 cannot be converted to contextual type %1",
452452
(Type, Type))
453+
ERROR(expr_string_interpolation_outside_string,none,
454+
"string interpolation can only appear inside a string literal", ())
453455

454456
// Selector expressions.
455457
ERROR(expr_selector_no_objc_runtime,none,

include/swift/AST/Expr.h

+9-1
Original file line numberDiff line numberDiff line change
@@ -4607,6 +4607,7 @@ class KeyPathExpr : public Expr {
46074607
class Component {
46084608
public:
46094609
enum class Kind: unsigned {
4610+
Invalid,
46104611
UnresolvedProperty,
46114612
UnresolvedSubscript,
46124613
Property,
@@ -4641,7 +4642,7 @@ class KeyPathExpr : public Expr {
46414642
{}
46424643

46434644
public:
4644-
Component() : Component({}, nullptr, (Kind)0, Type(), SourceLoc()) {}
4645+
Component() : Component({}, nullptr, Kind::Invalid, Type(), SourceLoc()) {}
46454646

46464647
/// Create an unresolved component for a property.
46474648
static Component forUnresolvedProperty(DeclName UnresolvedName,
@@ -4745,12 +4746,17 @@ class KeyPathExpr : public Expr {
47454746
return SubscriptIndexExprAndKind.getInt();
47464747
}
47474748

4749+
bool isValid() const {
4750+
return getKind() != Kind::Invalid;
4751+
}
4752+
47484753
Expr *getIndexExpr() const {
47494754
switch (getKind()) {
47504755
case Kind::Subscript:
47514756
case Kind::UnresolvedSubscript:
47524757
return SubscriptIndexExprAndKind.getPointer();
47534758

4759+
case Kind::Invalid:
47544760
case Kind::OptionalChain:
47554761
case Kind::OptionalWrap:
47564762
case Kind::OptionalForce:
@@ -4765,6 +4771,7 @@ class KeyPathExpr : public Expr {
47654771
case Kind::UnresolvedProperty:
47664772
return Decl.UnresolvedName;
47674773

4774+
case Kind::Invalid:
47684775
case Kind::Subscript:
47694776
case Kind::UnresolvedSubscript:
47704777
case Kind::OptionalChain:
@@ -4781,6 +4788,7 @@ class KeyPathExpr : public Expr {
47814788
case Kind::Subscript:
47824789
return Decl.ResolvedDecl;
47834790

4791+
case Kind::Invalid:
47844792
case Kind::UnresolvedProperty:
47854793
case Kind::UnresolvedSubscript:
47864794
case Kind::OptionalChain:

lib/AST/ASTDumper.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -2429,6 +2429,10 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
24292429
OS.indent(Indent + 2);
24302430
OS << "(component=";
24312431
switch (component.getKind()) {
2432+
case KeyPathExpr::Component::Kind::Invalid:
2433+
OS << "invalid ";
2434+
break;
2435+
24322436
case KeyPathExpr::Component::Kind::OptionalChain:
24332437
OS << "optional_chain ";
24342438
break;

lib/AST/ASTWalker.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,7 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
969969
case KeyPathExpr::Component::Kind::OptionalForce:
970970
case KeyPathExpr::Component::Kind::Property:
971971
case KeyPathExpr::Component::Kind::UnresolvedProperty:
972+
case KeyPathExpr::Component::Kind::Invalid:
972973
// No subexpr to visit.
973974
break;
974975
}

lib/SILGen/SILGenExpr.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -2753,7 +2753,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
27532753
case KeyPathExpr::Component::Kind::OptionalWrap:
27542754
return unsupported("non-property key path component");
27552755

2756-
2756+
case KeyPathExpr::Component::Kind::Invalid:
27572757
case KeyPathExpr::Component::Kind::UnresolvedProperty:
27582758
case KeyPathExpr::Component::Kind::UnresolvedSubscript:
27592759
llvm_unreachable("not resolved");

lib/Sema/CSApply.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ static bool buildObjCKeyPathString(KeyPathExpr *E,
305305
// when indexing a Dictionary or NSDictionary by string, or when applying
306306
// a mapping subscript operation to Array/Set or NSArray/NSSet.
307307
return false;
308+
case KeyPathExpr::Component::Kind::Invalid:
308309
case KeyPathExpr::Component::Kind::UnresolvedProperty:
309310
case KeyPathExpr::Component::Kind::UnresolvedSubscript:
310311
// Don't bother building the key path string if the key path didn't even
@@ -4123,6 +4124,7 @@ namespace {
41234124
case KeyPathExpr::Component::Kind::Property:
41244125
case KeyPathExpr::Component::Kind::Subscript:
41254126
case KeyPathExpr::Component::Kind::OptionalWrap:
4127+
case KeyPathExpr::Component::Kind::Invalid:
41264128
llvm_unreachable("already resolved");
41274129
}
41284130

lib/Sema/CSDiag.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -6953,6 +6953,7 @@ static bool diagnoseKeyPathComponents(ConstraintSystem *CS, KeyPathExpr *KPE,
69536953
componentName = TC.Context.Id_subscript;
69546954
break;
69556955

6956+
case KeyPathExpr::Component::Kind::Invalid:
69566957
case KeyPathExpr::Component::Kind::OptionalChain:
69576958
case KeyPathExpr::Component::Kind::OptionalForce:
69586959
// FIXME: Diagnose optional chaining and forcing properly.

lib/Sema/CSGen.cpp

+10-1
Original file line numberDiff line numberDiff line change
@@ -2755,7 +2755,13 @@ namespace {
27552755
CS.TC.diagnose(E->getLoc(), diag::expr_keypath_no_keypath_type);
27562756
return ErrorType::get(CS.getASTContext());
27572757
}
2758-
2758+
2759+
// If the key path contained any syntactically invalid components, bail
2760+
// out.
2761+
for (auto &c : E->getComponents())
2762+
if (!c.isValid())
2763+
return ErrorType::get(CS.getASTContext());
2764+
27592765
// For native key paths, traverse the key path components to set up
27602766
// appropriate type relationships at each level.
27612767
auto locator = CS.getConstraintLocator(E);
@@ -2778,6 +2784,9 @@ namespace {
27782784
for (unsigned i : indices(E->getComponents())) {
27792785
auto &component = E->getComponents()[i];
27802786
switch (auto kind = component.getKind()) {
2787+
case KeyPathExpr::Component::Kind::Invalid:
2788+
llvm_unreachable("should have bailed out");
2789+
27812790
case KeyPathExpr::Component::Kind::UnresolvedProperty: {
27822791
auto memberTy = CS.createTypeVariable(locator, TVO_CanBindToLValue);
27832792
auto refKind = component.getUnresolvedDeclName().isSimpleName()

lib/Sema/CSSimplify.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -3823,6 +3823,9 @@ ConstraintSystem::simplifyKeyPathConstraint(Type keyPathTy,
38233823
auto &component = keyPath->getComponents()[i];
38243824

38253825
switch (component.getKind()) {
3826+
case KeyPathExpr::Component::Kind::Invalid:
3827+
return SolutionKind::Error;
3828+
38263829
case KeyPathExpr::Component::Kind::UnresolvedProperty:
38273830
case KeyPathExpr::Component::Kind::UnresolvedSubscript: {
38283831
// If no choice was made, leave the constraint unsolved.

lib/Sema/TypeCheckConstraints.cpp

+12-3
Original file line numberDiff line numberDiff line change
@@ -1511,9 +1511,18 @@ void PreCheckExpression::resolveKeyPathExpr(KeyPathExpr *KPE) {
15111511
assert(OEE == outermostExpr);
15121512
expr = OEE->getSubExpr();
15131513
} else {
1514-
if (emitErrors)
1515-
TC.diagnose(expr->getLoc(),
1516-
diag::expr_swift_keypath_invalid_component);
1514+
if (emitErrors) {
1515+
// \(<expr>) may be an attempt to write a string interpolation outside
1516+
// of a string literal; diagnose this case specially.
1517+
if (isa<ParenExpr>(expr) || isa<TupleExpr>(expr)) {
1518+
TC.diagnose(expr->getLoc(),
1519+
diag::expr_string_interpolation_outside_string);
1520+
} else {
1521+
TC.diagnose(expr->getLoc(),
1522+
diag::expr_swift_keypath_invalid_component);
1523+
}
1524+
}
1525+
components.push_back(KeyPathExpr::Component());
15171526
return;
15181527
}
15191528
}

lib/Sema/TypeCheckExprObjC.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ Optional<Type> TypeChecker::checkObjCKeyPathExpr(DeclContext *dc,
199199
// ObjC keypaths only support named segments.
200200
// TODO: Perhaps we can map subscript components to dictionary keys.
201201
switch (auto kind = component.getKind()) {
202+
case KeyPathExpr::Component::Kind::Invalid:
203+
continue;
204+
202205
case KeyPathExpr::Component::Kind::UnresolvedProperty:
203206
break;
204207
case KeyPathExpr::Component::Kind::UnresolvedSubscript:

test/expr/unary/keypath/keypath.swift

+5
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,11 @@ func testKeyPath(sub: Sub, optSub: OptSub, x: Int) {
131131
let _: ReferenceWritableKeyPath<Prop, B> = \.nonMutatingProperty
132132
}
133133

134+
func testDisembodiedStringInterpolation(x: Int) {
135+
\(x) // expected-error{{string interpolation}}
136+
\(x, radix: 16) // expected-error{{string interpolation}}
137+
}
138+
134139
struct TupleStruct {
135140
var unlabeled: (Int, String)
136141
var labeled: (foo: Int, bar: String)

0 commit comments

Comments
 (0)