From bd72b8fb047268d5f00a083b57bc013caaec5eaf Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 2 Jun 2025 12:36:36 -0700 Subject: [PATCH] [Strict memory safety] Adjust "unsafe" location for string interpolations String interpolations can end up being unsafe in the call to appendInterpolation when it's provided with unsafe types. Move the location of the proposed "unsafe" out to the string interpolation itself in these cases, which properly suppresses the warning. Fixes rdar://151799777. --- lib/Sema/TypeCheckEffects.cpp | 30 ++++++++++++++++++++++-------- test/Unsafe/safe.swift | 6 ++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/lib/Sema/TypeCheckEffects.cpp b/lib/Sema/TypeCheckEffects.cpp index 5f952112c27bd..5906a45021e29 100644 --- a/lib/Sema/TypeCheckEffects.cpp +++ b/lib/Sema/TypeCheckEffects.cpp @@ -3626,9 +3626,9 @@ class CheckEffectsCoverage : public EffectsHandlingWalker } /// Find the top location where we should put the await - static Expr *walkToAnchor(Expr *e, llvm::DenseMap &parentMap, - bool isInterpolatedString, - bool stopAtAutoClosure) { + Expr *walkToAnchor(Expr *e, llvm::DenseMap &parentMap, + InterpolatedStringLiteralExpr *interpolatedString, + bool stopAtAutoClosure, EffectKind effect) { llvm::SmallPtrSet visited; Expr *parent = e; Expr *lastParent = e; @@ -3643,8 +3643,20 @@ class CheckEffectsCoverage : public EffectsHandlingWalker if (parent && !isAnchorTooEarly(parent)) { return parent; } - if (isInterpolatedString) { + if (interpolatedString) { assert(parent == nullptr && "Expected to be at top of expression"); + + // If the last parent we found is a call to appendInterpolation, adjust + // the anchor location to the interpolated string itself. + if (effect == EffectKind::Unsafe) { + if (auto callExpr = dyn_cast(lastParent)) { + if (auto calleeDecl = callExpr->getCalledValue()) { + if (calleeDecl->getName().matchesRef(Ctx.Id_appendInterpolation)) + return interpolatedString; + } + } + } + if (ArgumentList *args = lastParent->getArgs()) { if (Expr *unaryArg = args->getUnlabeledUnaryExpr()) return unaryArg; @@ -4279,8 +4291,9 @@ class CheckEffectsCoverage : public EffectsHandlingWalker Flags.has(ContextFlags::InAsyncLet))) { Expr *expr = E.dyn_cast(); Expr *anchor = walkToAnchor(expr, parentMap, - CurContext.isWithinInterpolatedString(), - /*stopAtAutoClosure=*/true); + CurContext.getInterpolatedString(), + /*stopAtAutoClosure=*/true, + EffectKind::Async); if (Flags.has(ContextFlags::StmtExprCoversAwait)) classification.setDowngradeToWarning(true); if (uncoveredAsync.find(anchor) == uncoveredAsync.end()) @@ -4305,8 +4318,9 @@ class CheckEffectsCoverage : public EffectsHandlingWalker if (!Flags.has(ContextFlags::IsUnsafeCovered)) { Expr *expr = E.dyn_cast(); Expr *anchor = walkToAnchor(expr, parentMap, - CurContext.isWithinInterpolatedString(), - /*stopAtAutoClosure=*/false); + CurContext.getInterpolatedString(), + /*stopAtAutoClosure=*/false, + EffectKind::Unsafe); // We don't diagnose uncovered unsafe uses within the next/nextElement // call, because they're handled already by the for-in loop checking. diff --git a/test/Unsafe/safe.swift b/test/Unsafe/safe.swift index a164d82ecc161..5131dfbfee172 100644 --- a/test/Unsafe/safe.swift +++ b/test/Unsafe/safe.swift @@ -366,3 +366,9 @@ protocol CustomAssociated: Associated { } extension SomeClass: CustomAssociated { typealias Associated = SomeClassWrapper // expected-note{{unsafe type 'SomeClass.Associated' (aka 'SomeClassWrapper') cannot satisfy safe associated type 'Associated'}} } + +func testInterpolation(ptr: UnsafePointer) { + _ = "Hello \(unsafe ptr)" // expected-warning{{expression uses unsafe constructs but is not marked with 'unsafe'}}{{7-7=unsafe }} + // expected-note@-1{{reference to unsafe type 'UnsafePointer'}} + // expected-note@-2{{argument #0 in call to instance method 'appendInterpolation' has unsafe type 'UnsafePointer'}} +}