---

yaml
---
r: 344935
b: refs/heads/master
c: 1f29048
h: refs/heads/master
i:
  344933: ed08f59
  344931: 362ebd4
  344927: 4f6b4cf

Author: swift-ci

[refs]

@@ -1,5 +1,5 @@
 ---
-refs/heads/master: 74026498a0dcd0c34032c97f7a789358f7059879
+refs/heads/master: 1f29048142c7a26e86223ba3877bd79f41a32c13
 refs/heads/master-next: 203b3026584ecad859eb328b2e12490099409cd5
 refs/tags/osx-passed: b6b74147ef8a386f532cf9357a1bde006e552c54
 refs/tags/swift-2.2-SNAPSHOT-2015-12-01-a: 6bb18e013c2284f2b45f5f84f2df2887dc0f7dea

trunk/lib/SILOptimizer/Mandatory/AccessEnforcementSelection.cpp

@@ -36,6 +36,7 @@
 #include "swift/SIL/SILFunction.h"
 #include "swift/SIL/SILUndef.h"
 #include "swift/SILOptimizer/Analysis/ClosureScope.h"
+#include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 
 using namespace swift;
@@ -68,7 +69,6 @@ struct AddressCapture {
 
   AddressCapture(Operand &oper)
       : site(oper.getUser()), calleeArgIdx(site.getCalleeArgIndex(oper)) {
-    assert(isa<PartialApplyInst>(site));
     if (site.getOrigCalleeConv().getSILArgumentConvention(calleeArgIdx)
         != SILArgumentConvention::Indirect_InoutAliasable) {
       site = ApplySite();
@@ -272,8 +272,16 @@ void SelectEnforcement::analyzeProjection(ProjectBoxInst *projection) {
 
       continue;
     }
-    if (isa<PartialApplyInst>(user))
-      Captures.emplace_back(AddressCapture(*use));
+    // Handle both partial applies and directly applied non-escaping closures.
+    if (ApplySite::isa(user)) {
+      AddressCapture capture(*use);
+      if (capture.isValid())
+        Captures.emplace_back(capture);
+      else
+        // Only full apply sites can have non-inout_aliasable address arguments,
+        // but those aren't actually captures.
+        assert(FullApplySite::isa(user));
+    }
   }
 }
 
@@ -463,6 +471,13 @@ void SelectEnforcement::updateCapture(AddressCapture capture) {
     if (hasPotentiallyEscapedAt(user))
       dynamicCaptures.recordCapture(capture);
   };
+  SingleValueInstruction *PAIUser = dyn_cast<PartialApplyInst>(capture.site);
+  if (!PAIUser) {
+    // This is a full apply site. Immediately record the capture and return.
+    captureIfEscaped(capture.site.getInstruction());
+    return;
+  }
+  // For partial applies, check all use points of the closure.
   llvm::SmallSetVector<SingleValueInstruction *, 8> worklist;
   auto visitUse = [&](Operand *oper) {
     auto *user = oper->getUser();
@@ -513,7 +528,6 @@ void SelectEnforcement::updateCapture(AddressCapture capture) {
       captureIfEscaped(user);
     }
   };
-  SingleValueInstruction *PAIUser = dyn_cast<PartialApplyInst>(capture.site);
   while (true) {
     for (auto *oper : PAIUser->getUses())
       visitUse(oper);
@@ -553,11 +567,10 @@ class AccessEnforcementSelection : public SILModuleTransform {
   // they reference.
   DynamicCaptures dynamicCaptures;
 
-  // Per-function book-keeping. A box is processed the first time one of it's
-  // accesses is handled. Don't process it again for subsequent accesses.
-  llvm::DenseSet<AllocBoxInst *> handledBoxes;
-
 #ifndef NDEBUG
+  // Per-function book-keeping to verify that a box is processed before all of
+  // its accesses and captures are seen.
+  llvm::DenseSet<AllocBoxInst *> handledBoxes;
   llvm::DenseSet<SILFunction *> visited;
 #endif
 
@@ -568,7 +581,7 @@ class AccessEnforcementSelection : public SILModuleTransform {
   void processFunction(SILFunction *F);
   SourceAccess getAccessKindForBox(ProjectBoxInst *projection);
   SourceAccess getSourceAccess(SILValue address);
-  void handlePartialApply(PartialApplyInst *PAI);
+  void handleApply(ApplySite apply);
   void handleAccess(BeginAccessInst *access);
 };
 
@@ -580,6 +593,9 @@ void AccessEnforcementSelection::run() {
 }
 
 void AccessEnforcementSelection::processFunction(SILFunction *F) {
+  if (F->isExternalDeclaration())
+    return;
+
   LLVM_DEBUG(llvm::dbgs() << "Access Enforcement Selection in " << F->getName()
                           << "\n");
 
@@ -599,29 +615,43 @@ void AccessEnforcementSelection::processFunction(SILFunction *F) {
   }
   visited.insert(F);
 #endif
+
   // Deserialized functions, which have been mandatory inlined, no longer meet
   // the structural requirements on access markers required by this pass.
   if (F->wasDeserializedCanonical())
     return;
 
-  for (auto &bb : *F) {
-    for (auto ii = bb.begin(), ie = bb.end(); ii != ie;) {
+  // Perform an RPO walk so that boxes are always processed before their access.
+  auto *PO = getAnalysis<PostOrderAnalysis>()->get(F);
+  for (SILBasicBlock *bb : PO->getReversePostOrder()) {
+    for (auto ii = bb->begin(), ie = bb->end(); ii != ie;) {
       SILInstruction *inst = &*ii;
       ++ii;
 
-      if (auto access = dyn_cast<BeginAccessInst>(inst))
+      // Analyze all boxes. Even if they aren't accessed in this function, they
+      // may still have captures that require dynamic enforcement because the
+      // box has escaped prior to the capture.
+      if (auto box = dyn_cast<AllocBoxInst>(inst)) {
+        SelectEnforcement(dynamicCaptures, box).run();
+        assert(handledBoxes.insert(box).second);
+
+      } else if (auto access = dyn_cast<BeginAccessInst>(inst))
         handleAccess(access);
 
       else if (auto access = dyn_cast<BeginUnpairedAccessInst>(inst))
         assert(access->getEnforcement() == SILAccessEnforcement::Dynamic);
 
-      else if(auto pa = dyn_cast<PartialApplyInst>(inst))
-        handlePartialApply(pa);
+      // Check for unboxed captures in both partial_applies and direct
+      // applications of non-escaping closures.
+      else if (auto apply = ApplySite::isa(inst))
+        handleApply(apply);
     }
   }
   invalidateAnalysis(F, SILAnalysis::InvalidationKind::Instructions);
+#ifndef NDEBUG
   // There's no need to track handled boxes across functions.
   handledBoxes.clear();
+#endif
 }
 
 SourceAccess
@@ -690,19 +720,17 @@ SourceAccess AccessEnforcementSelection::getSourceAccess(SILValue address) {
   return SourceAccess::getStaticAccess();
 }
 
-void AccessEnforcementSelection::handlePartialApply(PartialApplyInst *PAI) {
-  ApplySite site(PAI);
-  auto calleeTy = PAI->getOrigCalleeType();
+void AccessEnforcementSelection::handleApply(ApplySite apply) {
+  auto calleeTy = apply.getOrigCalleeType();
   SILFunctionConventions calleeConv(calleeTy, *getModule());
 
-  for (Operand &oper : site.getArgumentOperands()) {
+  for (Operand &oper : apply.getArgumentOperands()) {
     AddressCapture capture(oper);
     if (!capture.isValid())
       continue;
 
-    // This partial apply creates a non-escaping closure. Check if the closure
-    // captures any Boxed variables from this scope. If so, check if the box
-    // escapes before the access just as we do for normal accesses.
+    // This is a non-escaping closure argument. If the argument requires dynamic
+    // access, record that in dynamicCaptures.
     auto sourceAccess = getSourceAccess(oper.get());
     switch (sourceAccess.kind) {
     case SourceAccess::StaticAccess:
@@ -714,8 +742,11 @@ void AccessEnforcementSelection::handlePartialApply(PartialApplyInst *PAI) {
       break;
     }
     case SourceAccess::BoxAccess:
-      if (handledBoxes.insert(sourceAccess.allocBox).second)
-        SelectEnforcement(dynamicCaptures, sourceAccess.allocBox).run();
+      // Captures of box projections are handled during SelectEnforcement, which
+      // determines the access enforcement for all users of a box. Within
+      // SelectEnforcement, we know whether the box has escaped before the
+      // capture. Here there's just nothing to do.
+      assert(handledBoxes.count(sourceAccess.allocBox));
       break;
     }
   }
@@ -734,10 +765,7 @@ void AccessEnforcementSelection::handleAccess(BeginAccessInst *access) {
     setDynamicEnforcement(access);
     break;
   case SourceAccess::BoxAccess:
-    // If this box was handled, the access enforcement would already be set.
-    assert(!handledBoxes.count(sourceAccess.allocBox));
-    SelectEnforcement(dynamicCaptures, sourceAccess.allocBox).run();
-    break;
+    llvm_unreachable("All boxes must have already been selected.");
   }
 }
 

trunk/test/IRGen/partial_apply.sil

@@ -360,7 +360,7 @@ bb0(%0 : $Base):
  return %5 : $()
 }
 
-sil public_external @partial_empty_box : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <()>, @inout ()) -> ()
+sil public_external @partial_empty_box : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <()>, @inout_aliasable ()) -> ()
 
 // CHECK-LABEL: define{{( dllexport)?}}{{( protected)?}} swiftcc void @empty_box()
 sil @empty_box : $@convention(thin) () -> () {
@@ -370,8 +370,8 @@ entry:
   // CHECK: store %swift.opaque* undef
   %b = alloc_box $<τ_0_0> { var τ_0_0 } <()>
   %ba = project_box %b : $<τ_0_0> { var τ_0_0 } <()>, 0
-  %f = function_ref @partial_empty_box : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <()>, @inout ()) -> ()
-  %g = partial_apply %f(%b, %ba) : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <()>, @inout ()) -> ()
+  %f = function_ref @partial_empty_box : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <()>, @inout_aliasable ()) -> ()
+  %g = partial_apply %f(%b, %ba) : $@convention(thin) (@owned <τ_0_0> { var τ_0_0 } <()>, @inout_aliasable ()) -> ()
   return undef : $()
 }
 

trunk/test/Interpreter/enforce_exclusive_access.swift

@@ -26,6 +26,11 @@ func modifyAndPerform<T>(_ _: UnsafeMutablePointer<T>, closure: () ->()) {
   closure()
 }
 
+/// Begin a modify access to the first parameter and call the escaping closure inside it.
+func modifyAndPerformEscaping<T>(_ _: UnsafeMutablePointer<T>, closure: () ->()) {
+  closure()
+}
+
 var globalX = X()
 
 var ExclusiveAccessTestSuite = TestSuite("ExclusiveAccess")
@@ -344,4 +349,35 @@ ExclusiveAccessTestSuite.test("KeyPathInoutKeyPathReadClassStoredProp")
   }
 }
 
+// <rdar://problem/43076947> [Exclusivity] improve diagnostics for
+// withoutActuallyEscaping.
+ExclusiveAccessTestSuite.test("withoutActuallyEscapingConflict") {
+  var localVal = 0
+  let nestedModify = { localVal = 3 }
+  withoutActuallyEscaping(nestedModify) {
+    expectCrashLater()
+    modifyAndPerform(&localVal, closure: $0)
+  }
+}
+
+ExclusiveAccessTestSuite.test("directlyAppliedConflict") {
+  var localVal = 0
+  let nestedModify = { localVal = 3 }
+  expectCrashLater()
+  _ = {
+    modifyAndPerform(&localVal, closure: nestedModify)
+  }()
+}
+
+// <rdar://problem/43122715> [Exclusivity] failure to diagnose
+// escaping closures called within directly applied noescape closures.
+ExclusiveAccessTestSuite.test("directlyAppliedEscapingConflict") {
+  var localVal = 0
+  let nestedModify = { localVal = 3 }
+  expectCrashLater()
+  _ = {
+    modifyAndPerformEscaping(&localVal, closure: nestedModify)
+  }()
+}
+
 runAllTests()