Skip to content

Commit e9baa3f

Browse files
committed
Fix interpolated string effect placement
This fixes the placement of the await fix-it inside of interpolated strings. We were putting the effect between the `\` and `(`, which doesn't exactly work out very well. I'm intentionally being relatively gentle with the casting when grabbing the body of the interpolated string. I can't think of a valid expression that doesn't result in that structure, so it's probably safe to do a straight cast, and probably should switch it over eventually. In the current state-of-affairs, it will put the fix-it placement in the wrong place if my assumptions are broken about the structure of the code.
1 parent 3da0a54 commit e9baa3f

File tree

1 file changed

+33
-4
lines changed

1 file changed

+33
-4
lines changed

Diff for: lib/Sema/TypeCheckEffects.cpp

+33-4
Original file line numberDiff line numberDiff line change
@@ -1436,6 +1436,11 @@ class Context {
14361436
DiagnoseErrorOnTry = b;
14371437
}
14381438

1439+
/// Return true when the current context is under an interpolated string
1440+
bool isWithinInterpolatedString() const {
1441+
return InterpolatedString != nullptr;
1442+
}
1443+
14391444
/// Stores the location of the innermost await
14401445
SourceLoc awaitLoc = SourceLoc();
14411446

@@ -2084,15 +2089,38 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
20842089
}
20852090

20862091
/// Find the top location where we should put the await
2087-
static Expr* walkToAnchor(Expr *e,
2088-
llvm::DenseMap<Expr *, Expr*> &parentMap) {
2092+
static Expr *walkToAnchor(Expr *e, llvm::DenseMap<Expr *, Expr *> &parentMap,
2093+
bool isInterpolatedString) {
20892094
Expr *parent = e;
20902095
Expr *lastParent = e;
20912096
while (parent && !isEffectAnchor(parent)) {
20922097
lastParent = parent;
20932098
parent = parentMap[parent];
20942099
}
2095-
return parent && !isAnchorTooEarly(parent) ? parent : lastParent;
2100+
2101+
if (parent && !isAnchorTooEarly(parent)) {
2102+
return parent;
2103+
}
2104+
2105+
if (isInterpolatedString) {
2106+
// TODO: I'm being gentle with the casts to avoid breaking things
2107+
// If we see incorrect fix-it locations in string interpolations
2108+
// we need to change how this behaves
2109+
// Assert builds will crash giving us a bug to fix, non-asserts will
2110+
// quietly "just work".
2111+
assert(parent == nullptr && "Expected to be at top of expression");
2112+
assert(isa<CallExpr>(lastParent) &&
2113+
"Expected top of string interpolation to be CalExpr");
2114+
assert(isa<ParenExpr>(dyn_cast<CallExpr>(lastParent)->getArg()) &&
2115+
"Expected paren expr in string interpolation call");
2116+
if (CallExpr *callExpr = dyn_cast<CallExpr>(lastParent)) {
2117+
if (ParenExpr *body = dyn_cast<ParenExpr>(callExpr->getArg())) {
2118+
return body->getSubExpr();
2119+
}
2120+
}
2121+
}
2122+
2123+
return lastParent;
20962124
}
20972125

20982126
void flagInvalidCode() {
@@ -2620,7 +2648,8 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
26202648
// Diagnose async calls that are outside of an await context.
26212649
else if (!Flags.has(ContextFlags::IsAsyncCovered)) {
26222650
Expr *expr = E.dyn_cast<Expr*>();
2623-
Expr *anchor = walkToAnchor(expr, parentMap);
2651+
Expr *anchor = walkToAnchor(expr, parentMap,
2652+
CurContext.isWithinInterpolatedString());
26242653

26252654
auto key = uncoveredAsync.find(anchor);
26262655
if (key == uncoveredAsync.end()) {

0 commit comments

Comments
 (0)