LifetimeDepenenceDiagnostics improvements

Better handling of @_unsafeNonescapableResult

Improve diagnostic clarity.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

@@ -52,8 +52,9 @@ let lifetimeDependenceDiagnosticsPass = FunctionPass(
     if let apply = instruction as? FullApplySite {
       // Handle ~Escapable results that do not have a lifetime
       // dependence (@_unsafeNonescapableResult).
-      apply.dependentValues.forEach {
-        if let lifetimeDep = LifetimeDependence(applyResult: $0, context) {
+      apply.resultOrYields.forEach {
+        if let lifetimeDep = LifetimeDependence(unsafeApplyResult: $0,
+                                                context) {
           analyze(dependence: lifetimeDep, context)
         }
       }
@@ -75,23 +76,35 @@ private func analyze(dependence: LifetimeDependence,
   var range = dependence.computeRange(context)
   defer { range?.deinitialize() }
 
+  var error = false
   let diagnostics =
-    DiagnoseDependence(dependence: dependence, range: range, context: context)
+    DiagnoseDependence(dependence: dependence, range: range,
+                       onError: { error = true }, context: context)
 
   // Check each lifetime-dependent use via a def-use visitor
   var walker = DiagnoseDependenceWalker(diagnostics, context)
   defer { walker.deinitialize() }
-  _ = walker.walkDown(root: dependence.parentValue)
+  _ = walker.walkDown(root: dependence.dependentValue)
+
+  if !error {
+    dependence.resolve(context)
+  }
 }
 
 /// Analyze and diagnose a single LifetimeDependence.
 private struct DiagnoseDependence {
   let dependence: LifetimeDependence
   let range: InstructionRange?
+  let onError: ()->()
   let context: FunctionPassContext
 
   var function: Function { dependence.function }
 
+  func diagnose(_ position: SourceLoc?, _ id: DiagID,
+                _ args: DiagnosticArgument...) {
+    context.diagnosticEngine.diagnose(position, id, args)
+  }
+
   /// Check that this use is inside the dependence scope.
   func checkInScope(operand: Operand) -> WalkResult {
     if let range, !range.inclusiveRangeContains(operand.instruction) {
@@ -114,45 +127,73 @@ private struct DiagnoseDependence {
   }
 
   func checkFunctionResult(operand: Operand) -> WalkResult {
-    // TODO: Get the argument dependence for this result. Check that it is the
-    // same as the current dependence scope
 
     if function.hasUnsafeNonEscapableResult {
       return .continueWalk
     }
-    // TODO: Take ResultInfo as an argument and provide better
-    // diagnostics for missing lifetime dependencies.
+    // Allow returning an apply result (@_unsafeNonescapableResult) if
+    // the calling function has a dependence. This implicitly makes
+    // the unsafe nonescapable result dependent on the calling
+    // function's lifetime dependence arguments.
+    if dependence.isUnsafeApplyResult, function.hasResultDependence {
+      return .continueWalk
+    }
+    // Check that the argument dependence for this result is the same
+    // as the current dependence scope.
+    if let arg = dependence.scope.parentValue as? FunctionArgument,
+       function.argumentConventions[resultDependsOn: arg.index] != nil {
+      // The returned value depends on a lifetime that is inherited or
+      // borrowed in the caller. The lifetime of the argument value
+      // itself is irrelevant here.
+      return .continueWalk
+    }
     reportEscaping(operand: operand)
     return .abortWalk
   }
 
   func reportError(operand: Operand, diagID: DiagID) {
+    onError()
+
     // Identify the escaping variable.
     let escapingVar = LifetimeVariable(dependent: operand.value, context)
     let varName = escapingVar.name
     if let varName {
-      context.diagnosticEngine.diagnose(escapingVar.sourceLoc,
-        .lifetime_variable_outside_scope,
-        varName)
+      diagnose(escapingVar.sourceLoc, .lifetime_variable_outside_scope,
+               varName)
     } else {
-      context.diagnosticEngine.diagnose(escapingVar.sourceLoc,
-        .lifetime_value_outside_scope)
-    }
-    // Identify the dependence scope.
-    //
-    // TODO: add bridging for function argument locations
-    // [SILArgument.getDecl().getLoc()]
-    //
-    // TODO: For clear diagnostics: switch on dependence.scope.
-    // For an access, report both the accessed variable, and the access.
-    if let parentSourceLoc =
-         dependence.parentValue.definingInstruction?.location.sourceLoc {
-      context.diagnosticEngine.diagnose(parentSourceLoc,
-                                        .lifetime_outside_scope_parent)
+      diagnose(escapingVar.sourceLoc, .lifetime_value_outside_scope)
     }
+    reportScope()
     // Identify the use point.
     let userSourceLoc = operand.instruction.location.sourceLoc
-    context.diagnosticEngine.diagnose(userSourceLoc, diagID)
+    diagnose(userSourceLoc, diagID)
+  }
+
+  // Identify the dependence scope.
+  func reportScope() {
+    if case let .access(beginAccess) = dependence.scope {
+      let parentVar = LifetimeVariable(dependent: beginAccess, context)
+      if let sourceLoc = beginAccess.location.sourceLoc ?? parentVar.sourceLoc {
+        diagnose(sourceLoc, .lifetime_outside_scope_access,
+                 parentVar.name ?? "")
+      }
+      return
+    }
+    if let arg = dependence.parentValue as? Argument,
+       let varDecl = arg.varDecl,
+       let sourceLoc = arg.sourceLoc {
+      diagnose(sourceLoc, .lifetime_outside_scope_argument,
+               varDecl.userFacingName)
+      return
+    }
+    let parentVar = LifetimeVariable(dependent: dependence.parentValue, context)
+    if let parentLoc = parentVar.sourceLoc {
+      if let parentName = parentVar.name {
+        diagnose(parentLoc, .lifetime_outside_scope_variable, parentName)
+      } else {
+        diagnose(parentLoc, .lifetime_outside_scope_value)
+      }
+    }
   }
 }
 
@@ -182,18 +223,41 @@ private struct LifetimeVariable {
     return varDecl?.userFacingName
   }
 
-  init(introducer: Value) {
-    if introducer.type.isAddress {
-      switch introducer.enclosingAccessScope {
-      case let .scope(beginAccess):
-        // TODO: report both the access point and original variable.
-        self = LifetimeVariable(introducer: beginAccess.operand.value)
-        return
-      case .base(_):
-        // TODO: use an address walker to get the allocation point.
-        break
-      }
+  init(dependent value: Value, _ context: some Context) {
+    if value.type.isAddress {
+      self = Self(accessBase: value.accessBase, context)
+      return
     }
+    if let firstIntroducer = getFirstBorrowIntroducer(of: value, context) {
+      self = Self(introducer: firstIntroducer)
+      return
+    }
+    self.varDecl = nil
+    self.sourceLoc = nil
+  }
+
+  // FUTURE: consider diagnosing multiple variable introducers. It's
+  // unclear how more than one can happen.
+  private func getFirstBorrowIntroducer(of value: Value,
+                                        _ context: some Context)
+    -> Value? {
+    var introducers = Stack<Value>(context)
+    gatherBorrowIntroducers(for: value, in: &introducers, context)
+    return introducers.pop()
+  }
+
+  private func getFirstLifetimeIntroducer(of value: Value,
+                                          _ context: some Context)
+    -> Value? {
+    var introducer: Value?
+    _ = visitLifetimeIntroducers(for: value, context) {
+      introducer = $0
+      return .abortWalk
+    }
+    return introducer
+  }
+
+  private init(introducer: Value) {
     if let arg = introducer as? Argument {
       self.varDecl = arg.varDecl
     } else {
@@ -205,17 +269,42 @@ private struct LifetimeVariable {
     }
   }
 
-  init(dependent value: Value, _ context: Context) {
-    // TODO: consider diagnosing multiple variable introducers. It's
-    // unclear how more than one can happen.
-    var introducers = Stack<Value>(context)
-    gatherBorrowIntroducers(for: value, in: &introducers, context)
-    if let firstIntroducer = introducers.pop() {
-      self = LifetimeVariable(introducer: firstIntroducer)
-      return
+  // Record the source location of the variable decl if possible. The
+  // caller will already have a source location for the formal access,
+  // which is more relevant for diagnostics.
+  private init(accessBase: AccessBase, _ context: some Context) {
+    switch accessBase {
+    case .box(let projectBox):
+      if let box = getFirstLifetimeIntroducer(of: projectBox.box, context) {
+        self = Self(introducer: box)
+      }
+      // We should always find an introducer since boxes are nontrivial.
+      self.varDecl = nil
+      self.sourceLoc = nil
+    case .stack(let allocStack):
+      self = Self(introducer: allocStack)
+    case .global(let globalVar):
+      self.varDecl = globalVar.varDecl
+      self.sourceLoc = nil
+    case .class(let refAddr):
+      self.varDecl = refAddr.varDecl
+      self.sourceLoc = refAddr.location.sourceLoc
+    case .tail(let refTail):
+      self = Self(introducer: refTail.instance)
+    case .argument(let arg):
+      self.varDecl = arg.varDecl
+      self.sourceLoc = arg.sourceLoc
+    case .yield(let result):
+      // TODO: bridge VarDecl for FunctionConvention.Yields
+      self.varDecl = nil
+      self.sourceLoc = result.parentInstruction.location.sourceLoc
+    case .pointer(let ptrToAddr):
+      self.varDecl = nil
+      self.sourceLoc = ptrToAddr.location.sourceLoc
+    case .unidentified:
+      self.varDecl = nil
+      self.sourceLoc = nil
     }
-    self.varDecl = nil
-    self.sourceLoc = nil
   }
 }
 
@@ -229,8 +318,8 @@ private struct LifetimeVariable {
 ///
 /// TODO: handle stores to singly initialized temporaries like copies using a standard reaching-def analysis.
 private struct DiagnoseDependenceWalker {
-  let diagnostics: DiagnoseDependence
   let context: Context
+  var diagnostics: DiagnoseDependence
   var visitedValues: ValueSet
 
   var function: Function { diagnostics.function }

include/swift/AST/DiagnosticsSIL.def

@@ -958,9 +958,6 @@ NOTE(regionbasedisolation_named_stronglytransferred_binding, none,
 ERROR(deinit_not_visible, none,
       "deinit of non-copyable type not visible in the current module", ())
 
-ERROR(lifetime_value_outside_scope, none,
-      "lifetime-dependent value escapes its scope", ())
-
 ERROR(vector_capacity_not_constant, none,
       "vector capacity needs to be constant", ())
 
@@ -969,7 +966,15 @@ ERROR(fixed_arrays_not_available, none,
 
 ERROR(lifetime_variable_outside_scope, none,
       "lifetime-dependent variable '%0' escapes its scope", (Identifier))
-NOTE(lifetime_outside_scope_parent, none,
+ERROR(lifetime_value_outside_scope, none,
+      "lifetime-dependent value escapes its scope", ())
+NOTE(lifetime_outside_scope_argument, none,
+     "it depends on the lifetime of argument '%0'", (Identifier))
+NOTE(lifetime_outside_scope_access, none,
+     "it depends on this scoped access to variable '%0'", (Identifier))
+NOTE(lifetime_outside_scope_variable, none,
+     "it depends on the lifetime of variable '%0'", (Identifier))
+NOTE(lifetime_outside_scope_value, none,
      "it depends on the lifetime of this parent value", ())
 NOTE(lifetime_outside_scope_use, none,
      "this use of the lifetime-dependent value is out of scope", ())