Merge pull request #78199 from atrick/lifedep-cleanup

LifetimeDependence: minor diagnostic quality fixes and related utilities

Author: atrick

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

@@ -9,6 +9,15 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+///
+/// Pass dependencies:
+///
+/// - After MoveOnly checking fixes non-Copyable lifetimes.
+///
+/// - Before MoveOnlyTypeEliminator removes ownership operations on trivial types, which loses variable information
+/// required for diagnostics.
+///
+//===----------------------------------------------------------------------===//
 
 import AST
 import SIL
@@ -250,22 +259,6 @@ private struct DiagnoseDependence {
   }
 }
 
-private extension Instruction {
-  func findVarDecl() -> VarDecl? {
-    if let varDeclInst = self as? VarDeclInstruction {
-      return varDeclInst.varDecl
-    }
-    for result in results {
-      for use in result.uses {
-        if let debugVal = use.instruction as? DebugValueInst {
-          return debugVal.varDecl
-        }
-      }
-    }
-    return nil
-  }
-}
-
 // Identify a best-effort variable declaration based on a defining SIL
 // value or any lifetime dependent use of that SIL value.
 private struct LifetimeVariable {
@@ -327,7 +320,7 @@ private struct LifetimeVariable {
       self = Self(introducer: allocStack)
     case .global(let globalVar):
       self.varDecl = globalVar.varDecl
-      self.sourceLoc = nil
+      self.sourceLoc = varDecl?.nameLoc
     case .class(let refAddr):
       self.varDecl = refAddr.varDecl
       self.sourceLoc = refAddr.location.sourceLoc

SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift

@@ -564,3 +564,22 @@ extension AddressOwnershipLiveRange {
     return .local(allocation, range)
   }
 }
+
+let addressOwnershipLiveRangeTest = FunctionTest("address_ownership_live_range") {
+  function, arguments, context in
+  let address = arguments.takeValue()
+  print("Address: \(address)")
+  print("Base: \(address.accessBase)")
+  let begin = address.definingInstructionOrTerminator ?? {
+    assert(address is FunctionArgument)
+    return function.instructions.first!
+  }()
+  let localReachabilityCache = LocalVariableReachabilityCache()
+  guard var ownershipRange = AddressOwnershipLiveRange.compute(for: address, at: begin,
+                                                               localReachabilityCache, context) else {
+    print("Error: indeterminate live range")
+    return
+  }
+  defer { ownershipRange.deinitialize() }
+  print(ownershipRange)
+}

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -52,6 +52,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import AST
 import SIL
 
 private let verbose = false
@@ -593,17 +594,13 @@ struct VariableIntroducerUseDefWalker : LifetimeDependenceUseDefWalker {
   }
 
   mutating func walkUp(value: Value, _ owner: Value?) -> WalkResult {
-    switch value.definingInstruction {
-    case let moveInst as MoveValueInst:
-      if moveInst.isFromVarDecl {
-        return introducer(moveInst, owner)
-      }
-    case let borrow as BeginBorrowInst:
-      if borrow.isFromVarDecl {
-        return introducer(borrow, owner)
-      }
-    default:
-      break
+    if let inst = value.definingInstruction, VariableScopeInstruction(inst) != nil {
+      return introducer(value, owner)
+    }
+    // Finding a variable introducer requires following the mark_dependence forwarded value, not the base value like the
+    // default LifetimeDependenceUseDefWalker.
+    if value is MarkDependenceInst {
+      return walkUpDefault(forwarded: value, owner)
     }
     return walkUpDefault(dependent: value, owner: owner)
   }
@@ -749,6 +746,11 @@ extension LifetimeDependenceUseDefWalker {
         return walkUp(newLifetime: store.source)
       case let srcDestInst as SourceDestAddrInstruction:
         return walkUp(address: srcDestInst.sourceOperand.value)
+      case let apply as FullApplySite:
+        if let f = apply.referencedFunction,
+           f.isConvertPointerToPointerArgument {
+          return walkUp(address: apply.parameterOperands[0].value)
+        }
       default:
         break
       }

SwiftCompilerSources/Sources/Optimizer/Utilities/Test.swift

@@ -154,6 +154,7 @@ public func registerOptimizerTests() {
   // Register each test.
   registerFunctionTests(
     getAccessBaseTest,
+    addressOwnershipLiveRangeTest,
     argumentConventionsTest,
     borrowIntroducersTest,
     enclosingValuesTest,

SwiftCompilerSources/Sources/SIL/Function.swift

@@ -40,6 +40,8 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
 
   public var isAutodiffVJP: Bool { bridged.isAutodiffVJP() }
 
+  public var isConvertPointerToPointerArgument: Bool { bridged.isConvertPointerToPointerArgument() }
+
   public var specializationLevel: Int { bridged.specializationLevel() }
 
   public var hasOwnership: Bool { bridged.hasOwnership() }

SwiftCompilerSources/Sources/SIL/Instruction.swift

@@ -401,6 +401,88 @@ public protocol VarDeclInstruction {
   var varDecl: VarDecl? { get }
 }
 
+/// A scoped instruction whose single result introduces a variable scope.
+///
+/// The scope-ending uses represent the end of the variable scope. This allows trivial 'let' variables to be treated
+/// like a value with ownership. 'var' variables are primarily represented as addressable allocations via alloc_box or
+/// alloc_stack, but may have redundant VariableScopeInstructions.
+public enum VariableScopeInstruction {
+  case beginBorrow(BeginBorrowInst)
+  case moveValue(MoveValueInst)
+
+  public init?(_ inst: Instruction?) {
+    switch inst {
+    case let bbi as BeginBorrowInst:
+      guard bbi.isFromVarDecl else {
+        return nil
+      }
+      self = .beginBorrow(bbi)
+    case let mvi as MoveValueInst:
+      guard mvi.isFromVarDecl else {
+        return nil
+      }
+      self = .moveValue(mvi)
+    default:
+      return nil
+    }
+  }
+
+  public var instruction: Instruction {
+    switch self {
+    case let .beginBorrow(bbi):
+      return bbi
+    case let .moveValue(mvi):
+      return mvi
+    }
+  }
+
+  public var scopeBegin: Value {
+    instruction as! SingleValueInstruction
+  }
+
+  public var endOperands: LazyFilterSequence<UseList> {
+    return scopeBegin.uses.endingLifetime
+  }
+
+  // TODO: with SIL verification, we might be able to make varDecl non-Optional.
+  public var varDecl: VarDecl? {
+    if let debugVarDecl = instruction.debugVarDecl {
+      return debugVarDecl
+    }
+    // SILGen may produce double var_decl instructions for the same variable:
+    //   %box = alloc_box [var_decl] "x"
+    //   begin_borrow %box [var_decl]
+    //
+    // Assume that, if the begin_borrow or move_value does not have its own debug_value, then it must actually be
+    // associated with its operand's var_decl.
+    return instruction.operands[0].value.definingInstruction?.findVarDecl()
+  }
+}
+
+extension Instruction {
+  /// Find a variable declaration assoicated with this instruction.
+  public func findVarDecl() -> VarDecl? {
+    if let varDeclInst = self as? VarDeclInstruction {
+      return varDeclInst.varDecl
+    }
+    if let varScopeInst = VariableScopeInstruction(self) {
+      return varScopeInst.varDecl
+    }
+    return debugVarDecl
+  }
+
+  var debugVarDecl: VarDecl? {
+    for result in results {
+      for use in result.uses {
+        if let debugVal = use.instruction as? DebugValueInst {
+          return debugVal.varDecl
+        }
+      }
+    }
+    return nil
+  }
+}
+
 public protocol DebugVariableInstruction : VarDeclInstruction {
   typealias DebugVariable = OptionalBridgedSILDebugVariable
 

include/swift/SIL/SILBridging.h

@@ -525,6 +525,7 @@ struct BridgedFunction {
   BRIDGED_INLINE void setLinkage(BridgedLinkage linkage) const;
   BRIDGED_INLINE void setIsSerialized(bool isSerialized) const;
   bool isTrapNoReturn() const;
+  bool isConvertPointerToPointerArgument() const;
   bool isAutodiffVJP() const;
   SwiftInt specializationLevel() const;
   SWIFT_IMPORT_UNSAFE BridgedSubstitutionMap getMethodSubstitutions(BridgedSubstitutionMap contextSubs) const;

lib/AST/LifetimeDependence.cpp

@@ -17,6 +17,7 @@
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/ParameterList.h"
+#include "swift/AST/SourceFile.h"
 #include "swift/AST/Type.h"
 #include "swift/AST/TypeRepr.h"
 #include "swift/Basic/Assertions.h"
@@ -538,10 +539,18 @@ LifetimeDependenceInfo::infer(AbstractFunctionDecl *afd) {
                              : afd->getParameters()->size();
 
   auto *cd = dyn_cast<ConstructorDecl>(afd);
-  if (cd && cd->isImplicit()) {
-    if (cd->getParameters()->size() == 0) {
+  if (cd && cd->getParameters()->size() == 0) {
+    if (cd->isImplicit()) {
       return std::nullopt;
     }
+    if (auto *sf = afd->getParentSourceFile()) {
+      // The AST printer makes implicit initializers explicit, but does not
+      // print the @lifetime annotations. Until that is fixed, avoid diagnosing
+      // this as an error.
+      if (sf->Kind == SourceFileKind::SIL) {
+        return std::nullopt;
+      }
+    }
   }
 
   if (!ctx.LangOpts.hasFeature(Feature::LifetimeDependence)) {

lib/Parse/ParseDecl.cpp

@@ -5069,8 +5069,7 @@ ParserResult<LifetimeEntry> Parser::parseLifetimeEntry(SourceLoc loc) {
   auto lParenLoc = consumeAttributeLParen(); // consume the l_paren
 
   std::optional<LifetimeDescriptor> targetDescriptor;
-  if (!isInSILMode() &&
-      Tok.isAny(tok::identifier, tok::integer_literal, tok::kw_self) &&
+  if (Tok.isAny(tok::identifier, tok::integer_literal, tok::kw_self) &&
       peekToken().is(tok::colon)) {
     targetDescriptor = parseLifetimeDescriptor(*this);
     if (!targetDescriptor) {

lib/SILOptimizer/PassManager/PassManager.cpp

@@ -1608,6 +1608,15 @@ bool BridgedFunction::isTrapNoReturn() const {
   return swift::isTrapNoReturnFunction(getFunction());
 }
 
+bool BridgedFunction::isConvertPointerToPointerArgument() const {
+  if (auto declRef = getFunction()->getDeclRef()) {
+    auto *conversionDecl =
+      declRef.getASTContext().getConvertPointerToPointerArgument();
+    return declRef.getFuncDecl() == conversionDecl;
+  }
+  return false;
+}
+
 bool BridgedFunction::isAutodiffVJP() const {
   return swift::isDifferentiableFuncComponent(
       getFunction(), swift::AutoDiffFunctionComponent::VJP);