[silgen] When emitting lvalue gets into temporaries, cleanup the temporaries as early as possible.

Previously, we were emitting these cleanups at the end of the lexical scope
instead of at the end of the formal evaluation scope. This change ensures that
we always emit the cleanup immediately at the end of the formal evaluation
scope.

Previously in most cases we got away with this due to the +0 self
hack. Basically we would emit a get for a self parameter and then immediately
use that self parameter as a guaranteed parameter. Then the hack would insert
the destroy value forwarding the lexical scope level cleanup at the same time.

rdar://29791263

Author: gottesmm

lib/SILGen/Cleanup.cpp

@@ -179,7 +179,7 @@ void CleanupManager::setCleanupState(Cleanup &cleanup, CleanupState state) {
   // Do the transition now to avoid doing it in N places below.
   CleanupState oldState = cleanup.getState();
   (void)oldState;
-  cleanup.setState(state);
+  cleanup.setState(Gen, state);
 
   assert(state != oldState && "trivial cleanup state change");
   assert(oldState != CleanupState::Dead && "changing state of dead cleanup");
@@ -201,7 +201,7 @@ void CleanupStateRestorationScope::pushCleanupState(CleanupHandle handle,
          "changing state of dead cleanup");
 
   CleanupState oldState = cleanup.getState();
-  cleanup.setState(newState);
+  cleanup.setState(Cleanups.Gen, newState);
 
   SavedStates.push_back({handle, oldState});
 }
@@ -229,7 +229,7 @@ void CleanupStateRestorationScope::pop() {
     Cleanup &cleanup = *iter;
     assert(cleanup.getState() != CleanupState::Dead &&
            "changing state of dead cleanup");
-    cleanup.setState(stateToRestore);
+    cleanup.setState(Cleanups.Gen, stateToRestore);
   }
 }
 

lib/SILGen/Cleanup.h

@@ -33,6 +33,7 @@ class JumpDest;
 class SILGenFunction;
 class ManagedValue;
 class SharedBorrowFormalAccess;
+class FormalEvaluationScope;
 
 /// The valid states that a cleanup can be in.
 enum class CleanupState {
@@ -56,10 +57,11 @@ llvm::raw_ostream &operator<<(raw_ostream &os, CleanupState state);
 
 class LLVM_LIBRARY_VISIBILITY Cleanup {
   unsigned allocatedSize;
-  CleanupState state;
 
   friend class CleanupManager;
 protected:
+  CleanupState state;
+
   Cleanup() {}
   virtual ~Cleanup() {}
 
@@ -69,7 +71,9 @@ class LLVM_LIBRARY_VISIBILITY Cleanup {
   size_t allocated_size() const { return allocatedSize; }
 
   CleanupState getState() const { return state; }
-  void setState(CleanupState newState) { state = newState; }
+  virtual void setState(SILGenFunction &gen, CleanupState newState) {
+    state = newState;
+  }
   bool isActive() const { return state >= CleanupState::Active; }
   bool isDead() const { return state == CleanupState::Dead; }
 
@@ -121,6 +125,7 @@ class LLVM_LIBRARY_VISIBILITY CleanupManager {
 
   friend class CleanupStateRestorationScope;
   friend class SharedBorrowFormalEvaluation;
+  friend class FormalEvaluationScope;
 
 public:
   CleanupManager(SILGenFunction &Gen)

lib/SILGen/FormalEvaluation.cpp

@@ -27,11 +27,23 @@ void FormalAccess::_anchor() {}
 //                      Shared Borrow Formal Evaluation
 //===----------------------------------------------------------------------===//
 
-void SharedBorrowFormalAccess::finish(SILGenFunction &gen) {
+void SharedBorrowFormalAccess::finishImpl(SILGenFunction &gen) {
   gen.B.createEndBorrow(CleanupLocation::get(loc), borrowedValue,
                         originalValue);
 }
 
+//===----------------------------------------------------------------------===//
+//                             OwnedFormalAccess
+//===----------------------------------------------------------------------===//
+
+void OwnedFormalAccess::finishImpl(SILGenFunction &gen) {
+  auto cleanupLoc = CleanupLocation::get(loc);
+  if (value->getType().isAddress())
+    gen.B.createDestroyAddr(cleanupLoc, value);
+  else
+    gen.B.emitDestroyValueOperation(cleanupLoc, value);
+}
+
 //===----------------------------------------------------------------------===//
 //                          Formal Evaluation Scope
 //===----------------------------------------------------------------------===//
@@ -76,7 +88,21 @@ void FormalEvaluationScope::popImpl() {
     // Grab the next evaluation...
     FormalAccess &access = *iter;
 
-    // and deactivate the cleanup.
+    // If this access was already finished, continue. This can happen if an
+    // owned formal access was forwarded.
+    if (access.isFinished()) {
+      assert(access.getKind() == FormalAccess::Owned &&
+             "Only owned formal accesses should be forwarded.");
+      // We can not check that our cleanup is actually dead since the cleanup
+      // may have been popped at this point and the stack may have new values.
+      continue;
+    }
+
+    assert(!access.isFinished() && "Can not finish a formal access cleanup "
+                                   "twice");
+
+    // and deactivate the cleanup. This will set the isFinished bit for owned
+    // FormalAccess.
     gen.Cleanups.setCleanupState(access.getCleanup(), CleanupState::Dead);
 
     // Attempt to diagnose problems where obvious aliasing introduces illegal
@@ -112,3 +138,14 @@ void FormalEvaluationScope::popImpl() {
   // And then pop off all stack elements until we reach the savedDepth.
   context.pop(savedDepth.getValue());
 }
+
+//===----------------------------------------------------------------------===//
+//                         Formal Evaluation Context
+//===----------------------------------------------------------------------===//
+
+void FormalEvaluationContext::dump(SILGenFunction &gen) {
+  for (auto II = begin(), IE = end(); II != IE; ++II) {
+    FormalAccess &access = *II;
+    gen.Cleanups.dump(access.getCleanup());
+  }
+}

lib/SILGen/FormalEvaluation.h

@@ -26,7 +26,7 @@ class LogicalPathComponent;
 
 class FormalAccess {
 public:
-  enum Kind { Shared, Exclusive };
+  enum Kind { Shared, Exclusive, Owned };
 
 private:
   unsigned allocatedSize;
@@ -35,10 +35,12 @@ class FormalAccess {
 protected:
   SILLocation loc;
   CleanupHandle cleanup;
+  bool finished;
 
   FormalAccess(unsigned allocatedSize, Kind kind, SILLocation loc,
                CleanupHandle cleanup)
-      : allocatedSize(allocatedSize), kind(kind), loc(loc), cleanup(cleanup) {}
+      : allocatedSize(allocatedSize), kind(kind), loc(loc), cleanup(cleanup),
+        finished(false) {}
 
 public:
   virtual ~FormalAccess() {}
@@ -57,7 +59,14 @@ class FormalAccess {
 
   Kind getKind() const { return kind; }
 
-  virtual void finish(SILGenFunction &gen) = 0;
+  void finish(SILGenFunction &gen) { finishImpl(gen); }
+
+  void setFinished() { finished = true; }
+
+  bool isFinished() const { return finished; }
+
+protected:
+  virtual void finishImpl(SILGenFunction &gen) = 0;
 };
 
 class SharedBorrowFormalAccess : public FormalAccess {
@@ -69,10 +78,26 @@ class SharedBorrowFormalAccess : public FormalAccess {
                            SILValue originalValue, SILValue borrowedValue)
       : FormalAccess(sizeof(*this), FormalAccess::Shared, loc, cleanup),
         originalValue(originalValue), borrowedValue(borrowedValue) {}
-  void finish(SILGenFunction &gen) override;
 
   SILValue getBorrowedValue() const { return borrowedValue; }
   SILValue getOriginalValue() const { return originalValue; }
+
+private:
+  void finishImpl(SILGenFunction &gen) override;
+};
+
+class OwnedFormalAccess : public FormalAccess {
+  SILValue value;
+
+public:
+  OwnedFormalAccess(SILLocation loc, CleanupHandle cleanup, SILValue value)
+      : FormalAccess(sizeof(*this), FormalAccess::Owned, loc, cleanup),
+        value(value) {}
+
+  SILValue getValue() const { return value; }
+
+private:
+  void finishImpl(SILGenFunction &gen) override;
 };
 
 class FormalEvaluationContext {
@@ -113,6 +138,8 @@ class FormalEvaluationContext {
   /// Pop objects off of the stack until \p the object pointed to by stable_iter
   /// is the top element of the stack.
   void pop(stable_iterator stable_iter) { stack.pop(stable_iter); }
+
+  void dump(SILGenFunction &gen);
 };
 
 class FormalEvaluationScope {

lib/SILGen/LValue.h

@@ -508,7 +508,7 @@ struct LLVM_LIBRARY_VISIBILITY ExclusiveBorrowFormalAccess : FormalAccess {
     component->writeback(gen, loc, base, materialized, isFinal);
   }
 
-  void finish(SILGenFunction &gen) override {
+  void finishImpl(SILGenFunction &gen) override {
     performWriteback(gen, /*isFinal*/ true);
   }
 };

lib/SILGen/SILGenDecl.cpp

@@ -1353,6 +1353,16 @@ SILGenFunction::emitTemporary(SILLocation loc, const TypeLowering &tempTL) {
   return useBufferAsTemporary(addr, tempTL);
 }
 
+std::unique_ptr<TemporaryInitialization>
+SILGenFunction::emitFormalAccessTemporary(SILLocation loc,
+                                          const TypeLowering &tempTL) {
+  SILValue addr = emitTemporaryAllocation(loc, tempTL.getLoweredType());
+  CleanupHandle cleanup =
+      enterDormantFormalAccessTemporaryCleanup(addr, loc, tempTL);
+  return std::unique_ptr<TemporaryInitialization>(
+      new TemporaryInitialization(addr, cleanup));
+}
+
 /// Create an Initialization for an uninitialized buffer.
 std::unique_ptr<TemporaryInitialization>
 SILGenFunction::useBufferAsTemporary(SILValue addr,
@@ -1372,6 +1382,59 @@ SILGenFunction::enterDormantTemporaryCleanup(SILValue addr,
   return Cleanups.getCleanupsDepth();
 }
 
+namespace {
+
+struct FormalAccessReleaseValueCleanup : Cleanup {
+  FormalEvaluationContext::stable_iterator Depth;
+
+  FormalAccessReleaseValueCleanup() : Depth() {}
+
+  void setState(SILGenFunction &gen, CleanupState newState) override {
+    if (newState == CleanupState::Dead) {
+      getEvaluation(gen).setFinished();
+    }
+
+    state = newState;
+  }
+
+  void emit(SILGenFunction &gen, CleanupLocation l) override {
+    getEvaluation(gen).finish(gen);
+  }
+
+  void dump(SILGenFunction &gen) const override {
+#ifndef NDEBUG
+    llvm::errs() << "FormalAccessReleaseValueCleanup "
+                 << "State:" << getState() << "\n"
+                 << "Value:" << getValue(gen) << "\n";
+#endif
+  }
+
+  OwnedFormalAccess &getEvaluation(SILGenFunction &gen) const {
+    auto &evaluation = *gen.FormalEvalContext.find(Depth);
+    assert(evaluation.getKind() == FormalAccess::Owned);
+    return static_cast<OwnedFormalAccess &>(evaluation);
+  }
+
+  SILValue getValue(SILGenFunction &gen) const {
+    return getEvaluation(gen).getValue();
+  }
+};
+
+} // end anonymous namespace
+
+CleanupHandle SILGenFunction::enterDormantFormalAccessTemporaryCleanup(
+    SILValue addr, SILLocation loc, const TypeLowering &tempTL) {
+  if (tempTL.isTrivial())
+    return CleanupHandle::invalid();
+
+  auto &cleanup = Cleanups.pushCleanup<FormalAccessReleaseValueCleanup>();
+  CleanupHandle handle = Cleanups.getTopCleanup();
+  Cleanups.setCleanupState(handle, CleanupState::Dormant);
+  FormalEvalContext.push<OwnedFormalAccess>(loc, handle, addr);
+  cleanup.Depth = FormalEvalContext.stable_begin();
+  return handle;
+}
+
 void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) {
   assert(vd->getDeclContext()->isLocalContext() &&
          "can't emit a local var for a non-local var decl");

lib/SILGen/SILGenFunction.h

@@ -1533,6 +1533,14 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   std::unique_ptr<TemporaryInitialization>
   emitTemporary(SILLocation loc, const TypeLowering &tempTL);
 
+  /// Emit the allocation for a local temporary, provides an
+  /// Initialization that can be used to initialize it, and registers
+  /// cleanups in the current active formal evaluation scope.
+  ///
+  /// The initialization is guaranteed to be a single buffer.
+  std::unique_ptr<TemporaryInitialization>
+  emitFormalAccessTemporary(SILLocation loc, const TypeLowering &tempTL);
+
   /// Provides an Initialization that can be used to initialize an already-
   /// allocated temporary, and registers cleanups in the active scope.
   ///
@@ -1545,6 +1553,12 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   CleanupHandle enterDormantTemporaryCleanup(SILValue temp,
                                              const TypeLowering &tempTL);
 
+  /// Enter a currently-dormant cleanup to destroy the value in the
+  /// given address.
+  CleanupHandle
+  enterDormantFormalAccessTemporaryCleanup(SILValue temp, SILLocation loc,
+                                           const TypeLowering &tempTL);
+
   /// Destroy and deallocate an initialized local variable.
   void destroyLocalVariable(SILLocation L, VarDecl *D);
 

lib/SILGen/SILGenLValue.cpp

@@ -150,14 +150,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenLValue
                                        AccessKind accessKind);
 };
 
-static ManagedValue emitGetIntoTemporary(SILGenFunction &gen,
-                                         SILLocation loc,
-                                         ManagedValue base,
-                                         LogicalPathComponent &&component) {
-  // Create a temporary.
-  auto temporaryInit =
-    gen.emitTemporary(loc, gen.getTypeLowering(component.getTypeOfRValue()));
-
+static ManagedValue
+emitGetIntoTemporary(SILGenFunction &gen, SILLocation loc, ManagedValue base,
+                     std::unique_ptr<TemporaryInitialization> &&temporaryInit,
+                     LogicalPathComponent &&component) {
   // Emit a 'get' into the temporary.
   RValue value =
     std::move(component).get(gen, loc, base, SGFContext(temporaryInit.get()));
@@ -177,7 +173,12 @@ ManagedValue LogicalPathComponent::getMaterialized(SILGenFunction &gen,
   // If this is just for a read, emit a load into a temporary memory
   // location.
   if (kind == AccessKind::Read) {
-    return emitGetIntoTemporary(gen, loc, base, std::move(*this));
+    // Create a temporary.
+    std::unique_ptr<TemporaryInitialization> temporaryInit =
+        gen.emitFormalAccessTemporary(loc,
+                                      gen.getTypeLowering(getTypeOfRValue()));
+    return emitGetIntoTemporary(gen, loc, base, std::move(temporaryInit),
+                                std::move(*this));
   }
 
   assert(gen.InWritebackScope &&
@@ -189,6 +190,11 @@ ManagedValue LogicalPathComponent::getMaterialized(SILGenFunction &gen,
 
   ManagedValue temporary;
   {
+    // Create a temporary.
+    std::unique_ptr<TemporaryInitialization> temporaryInit =
+        gen.emitFormalAccessTemporary(loc,
+                                      gen.getTypeLowering(getTypeOfRValue()));
+
     FormalEvaluationScope Scope(gen);
 
     // Otherwise, we need to emit a get and set.  Borrow the base for
@@ -197,7 +203,8 @@ ManagedValue LogicalPathComponent::getMaterialized(SILGenFunction &gen,
         base ? base.formalEvaluationBorrow(gen, loc) : ManagedValue();
 
     // Emit a 'get' into a temporary and then pop the borrow of base.
-    temporary = emitGetIntoTemporary(gen, loc, getterBase, std::move(*this));
+    temporary = emitGetIntoTemporary(
+        gen, loc, getterBase, std::move(temporaryInit), std::move(*this));
   }
 
   // Push a writeback for the temporary.