Skip to content

Commit bfa991d

Browse files
authored
Merge pull request #81910 from DougGregor/unsafe-string-interpolation
[Strict memory safety] Adjust "unsafe" location for string interpolations
2 parents 911e9f9 + 6ba48f2 commit bfa991d

File tree

2 files changed

+28
-8
lines changed

2 files changed

+28
-8
lines changed

lib/Sema/TypeCheckEffects.cpp

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3645,9 +3645,9 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
36453645
}
36463646

36473647
/// Find the top location where we should put the await
3648-
static Expr *walkToAnchor(Expr *e, llvm::DenseMap<Expr *, Expr *> &parentMap,
3649-
bool isInterpolatedString,
3650-
bool stopAtAutoClosure) {
3648+
Expr *walkToAnchor(Expr *e, llvm::DenseMap<Expr *, Expr *> &parentMap,
3649+
InterpolatedStringLiteralExpr *interpolatedString,
3650+
bool stopAtAutoClosure, EffectKind effect) {
36513651
llvm::SmallPtrSet<Expr *, 4> visited;
36523652
Expr *parent = e;
36533653
Expr *lastParent = e;
@@ -3662,8 +3662,20 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
36623662
if (parent && !isAnchorTooEarly(parent)) {
36633663
return parent;
36643664
}
3665-
if (isInterpolatedString) {
3665+
if (interpolatedString) {
36663666
assert(parent == nullptr && "Expected to be at top of expression");
3667+
3668+
// If the last parent we found is a call to appendInterpolation, adjust
3669+
// the anchor location to the interpolated string itself.
3670+
if (effect == EffectKind::Unsafe) {
3671+
if (auto callExpr = dyn_cast<CallExpr>(lastParent)) {
3672+
if (auto calleeDecl = callExpr->getCalledValue()) {
3673+
if (calleeDecl->getName().matchesRef(Ctx.Id_appendInterpolation))
3674+
return interpolatedString;
3675+
}
3676+
}
3677+
}
3678+
36673679
if (ArgumentList *args = lastParent->getArgs()) {
36683680
if (Expr *unaryArg = args->getUnlabeledUnaryExpr())
36693681
return unaryArg;
@@ -4322,8 +4334,9 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
43224334
Flags.has(ContextFlags::InAsyncLet))) {
43234335
Expr *expr = E.dyn_cast<Expr*>();
43244336
Expr *anchor = walkToAnchor(expr, parentMap,
4325-
CurContext.isWithinInterpolatedString(),
4326-
/*stopAtAutoClosure=*/true);
4337+
CurContext.getInterpolatedString(),
4338+
/*stopAtAutoClosure=*/true,
4339+
EffectKind::Async);
43274340
if (Flags.has(ContextFlags::StmtExprCoversAwait))
43284341
classification.setDowngradeToWarning(true);
43294342
if (uncoveredAsync.find(anchor) == uncoveredAsync.end())
@@ -4348,8 +4361,9 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
43484361
if (!Flags.has(ContextFlags::IsUnsafeCovered)) {
43494362
Expr *expr = E.dyn_cast<Expr*>();
43504363
Expr *anchor = walkToAnchor(expr, parentMap,
4351-
CurContext.isWithinInterpolatedString(),
4352-
/*stopAtAutoClosure=*/false);
4364+
CurContext.getInterpolatedString(),
4365+
/*stopAtAutoClosure=*/false,
4366+
EffectKind::Unsafe);
43534367

43544368
// We don't diagnose uncovered unsafe uses within the next/nextElement
43554369
// call, because they're handled already by the for-in loop checking.

test/Unsafe/safe.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,3 +376,9 @@ protocol CustomAssociated: Associated { }
376376
extension SomeClass: CustomAssociated {
377377
typealias Associated = SomeClassWrapper // expected-note{{unsafe type 'SomeClass.Associated' (aka 'SomeClassWrapper') cannot satisfy safe associated type 'Associated'}}
378378
}
379+
380+
func testInterpolation(ptr: UnsafePointer<Int>) {
381+
_ = "Hello \(unsafe ptr)" // expected-warning{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{7-7=unsafe }}
382+
// expected-note@-1{{reference to unsafe type 'UnsafePointer<Int>'}}
383+
// expected-note@-2{{argument #0 in call to instance method 'appendInterpolation' has unsafe type 'UnsafePointer<Int>'}}
384+
}

0 commit comments

Comments
 (0)