Skip to content

Commit 0d5351b

Browse files
authored
Merge pull request #81993 from atrick/62-fix-lifedep-indirect-closure
[6.2] LifetimeDependenceDiagnostics: diagnose indirect closure results.
2 parents 608a37a + 4caca64 commit 0d5351b

File tree

3 files changed

+71
-19
lines changed

3 files changed

+71
-19
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,16 @@ let lifetimeDependenceDiagnosticsPass = FunctionPass(
7272
markDep.settleToEscaping()
7373
continue
7474
}
75-
if let apply = instruction as? FullApplySite {
76-
// Handle ~Escapable results that do not have a lifetime dependence. This includes implicit initializers and
77-
// @_unsafeNonescapableResult.
75+
if let apply = instruction as? FullApplySite, !apply.hasResultDependence {
76+
// Handle ~Escapable results that do not have a lifetime dependence. This includes implicit initializers, calls to
77+
// closures, and @_unsafeNonescapableResult.
7878
apply.resultOrYields.forEach {
79-
if let lifetimeDep = LifetimeDependence(unsafeApplyResult: $0,
80-
context) {
79+
if let lifetimeDep = LifetimeDependence(unsafeApplyResult: $0, apply: apply, context) {
80+
_ = analyze(dependence: lifetimeDep, context)
81+
}
82+
}
83+
apply.indirectResultOperands.forEach {
84+
if let lifetimeDep = LifetimeDependence(unsafeApplyResult: $0.value, apply: apply, context) {
8185
_ = analyze(dependence: lifetimeDep, context)
8286
}
8387
}
@@ -237,10 +241,12 @@ private struct DiagnoseDependence {
237241
onError()
238242

239243
// Identify the escaping variable.
240-
let escapingVar = LifetimeVariable(dependent: operand.value, context)
244+
let escapingVar = LifetimeVariable(usedBy: operand, context)
241245
if let varDecl = escapingVar.varDecl {
242246
// Use the variable location, not the access location.
243-
diagnose(varDecl.nameLoc, .lifetime_variable_outside_scope, escapingVar.name ?? "")
247+
// Variable names like $return_value and $implicit_value don't have source locations.
248+
let sourceLoc = varDecl.nameLoc ?? escapingVar.sourceLoc
249+
diagnose(sourceLoc, .lifetime_variable_outside_scope, escapingVar.name ?? "")
244250
} else if let sourceLoc = escapingVar.sourceLoc {
245251
diagnose(sourceLoc, .lifetime_value_outside_scope)
246252
} else {
@@ -263,7 +269,7 @@ private struct DiagnoseDependence {
263269

264270
// Identify the dependence scope. If no source location is found, bypass this diagnostic.
265271
func reportScope() {
266-
let parentVar = LifetimeVariable(dependent: dependence.parentValue, context)
272+
let parentVar = LifetimeVariable(definedBy: dependence.parentValue, context)
267273
// First check if the dependency is limited to an access scope. If the access has no source location then
268274
// fall-through to report possible dependence on an argument.
269275
if parentVar.isAccessScope, let accessLoc = parentVar.sourceLoc {
@@ -310,7 +316,22 @@ private struct LifetimeVariable {
310316
return varDecl?.userFacingName
311317
}
312318

313-
init(dependent value: Value, _ context: some Context) {
319+
init(usedBy operand: Operand, _ context: some Context) {
320+
self = .init(dependent: operand.value, context)
321+
// variable names like $return_value and $implicit_value don't have source locations.
322+
// For @out arguments, the operand's location is the best answer.
323+
// Otherwise, fall back to the function's location.
324+
self.sourceLoc = self.sourceLoc ?? operand.instruction.location.sourceLoc
325+
?? operand.instruction.parentFunction.location.sourceLoc
326+
}
327+
328+
init(definedBy value: Value, _ context: some Context) {
329+
self = .init(dependent: value, context)
330+
// Fall back to the function's location.
331+
self.sourceLoc = self.sourceLoc ?? value.parentFunction.location.sourceLoc
332+
}
333+
334+
private init(dependent value: Value, _ context: some Context) {
314335
guard let introducer = getFirstVariableIntroducer(of: value, context) else {
315336
return
316337
}

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -165,16 +165,12 @@ extension LifetimeDependence {
165165
//
166166
// This is necessary because inserting a mark_dependence placeholder for such an unsafe dependence would illegally
167167
// have the same base and value operand.
168-
//
169-
// TODO: handle indirect results
170-
init?(unsafeApplyResult value: Value, _ context: some Context) {
168+
init?(unsafeApplyResult value: Value, apply: FullApplySite, _ context: some Context) {
171169
if value.isEscapable {
172170
return nil
173171
}
174-
if (value.definingInstructionOrTerminator as! FullApplySite).hasResultDependence {
175-
return nil
176-
}
177-
assert(value.ownership == .owned, "unsafe apply result must be owned")
172+
assert(!apply.hasResultDependence, "mark_dependence should be used instead")
173+
assert(value.ownership == .owned || value.type.isAddress, "unsafe apply result must be owned")
178174
self.scope = Scope(base: value, context)
179175
self.dependentValue = value
180176
self.markDepInst = nil
@@ -578,8 +574,8 @@ extension LifetimeDependenceDefUseWalker {
578574
}
579575
let root = dependence.dependentValue
580576
if root.type.isAddress {
581-
// The root address may be an escapable mark_dependence that guards its address uses (unsafeAddress), or an
582-
// allocation or incoming argument. In all these cases, it is sufficient to walk down the address uses.
577+
// The root address may be an escapable mark_dependence that guards its address uses (unsafeAddress), an
578+
// allocation, an incoming argument, or an outgoing argument. In all these cases, walk down the address uses.
583579
return walkDownAddressUses(of: root)
584580
}
585581
return walkDownUses(of: root, using: nil)
@@ -974,7 +970,8 @@ extension LifetimeDependenceDefUseWalker {
974970

975971
private mutating func visitAppliedUse(of operand: Operand, by apply: FullApplySite) -> WalkResult {
976972
if let conv = apply.convention(of: operand), conv.isIndirectOut {
977-
return leafUse(of: operand)
973+
// This apply initializes an allocation.
974+
return dependentUse(of: operand, dependentAddress: operand.value)
978975
}
979976
if apply.isCallee(operand: operand) {
980977
return leafUse(of: operand)

test/SILOptimizer/lifetime_dependence/verify_diagnostics.swift

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ struct Holder {
4949
var c: C? = nil
5050
}
5151

52+
// Generic non-Escapable for indirect values.
53+
struct GNE<T> : ~Escapable {
54+
let t: T
55+
@lifetime(borrow t)
56+
init(t: borrowing T) { self.t = copy t }
57+
}
58+
59+
@_silgen_name("forward")
60+
@lifetime(copy arg)
61+
func forward<T>(_ arg: GNE<T>) -> GNE<T>
62+
5263
@_silgen_name("getGeneric")
5364
@lifetime(borrow holder)
5465
func getGeneric<T: ~Escapable>(_ holder: borrowing Holder, _: T.Type) -> T
@@ -156,3 +167,26 @@ func testClosureCapture1(_ a: HasMethods) {
156167
}
157168
*/
158169
}
170+
171+
// =============================================================================
172+
// Indirect ~Escapable results
173+
// =============================================================================
174+
175+
@lifetime(copy arg1)
176+
func testIndirectForwardedResult<T>(arg1: GNE<T>) -> GNE<T> {
177+
forward(arg1)
178+
}
179+
180+
@lifetime(copy arg1)
181+
func testIndirectNonForwardedResult<T>(arg1: GNE<T>, arg2: GNE<T>) -> GNE<T> {
182+
// expected-error @-1{{lifetime-dependent variable 'arg2' escapes its scope}}
183+
// expected-note @-2{{it depends on the lifetime of argument 'arg2'}}
184+
forward(arg2) // expected-note {{this use causes the lifetime-dependent value to escape}}
185+
}
186+
187+
func testIndirectClosureResult<T>(f: () -> GNE<T>) -> GNE<T> {
188+
f()
189+
// expected-error @-1{{lifetime-dependent variable '$return_value' escapes its scope}}
190+
// expected-note @-3{{it depends on the lifetime of argument '$return_value'}}
191+
// expected-note @-3{{this use causes the lifetime-dependent value to escape}}
192+
}

0 commit comments

Comments
 (0)