SILGen: Emit mark_function_escape when capturing variable boxes.

There's no longer a direct use of the variable's address when we invoke the closure, but we have a handy mark_function_escape instruction to mark the use without requiring merging analysis of the box and its contents. This also gives us a slightly more accurate error message when a variable is prematurely captured, noting the variable was captured by a closure instead of just generically used.

Author: jckarter

include/swift/AST/DiagnosticsSIL.def

@@ -97,7 +97,7 @@ ERROR(variable_used_before_initialized,sil_analysis,none,
 ERROR(variable_inout_before_initialized,sil_analysis,none,
       "%select{variable|constant}1 '%0' passed by reference before being"
       " initialized", (StringRef, bool))
-ERROR(variable_escape_before_initialized,sil_analysis,none,
+ERROR(variable_closure_use_uninit,sil_analysis,none,
       "%select{variable|constant}1 '%0' captured by a closure before being"
       " initialized", (StringRef, bool))
 
@@ -145,7 +145,7 @@ ERROR(return_from_init_without_initing_stored_properties,sil_analysis,none,
       "return from initializer without initializing all"
       " stored properties", ())
 
-ERROR(global_variable_function_use_uninit,sil_analysis,none,
+ERROR(variable_function_use_uninit,sil_analysis,none,
       "%select{variable|constant}1 '%0' used by function definition before"
       " being initialized",
       (StringRef, bool))

lib/SILGen/SILGenFunction.cpp

@@ -223,6 +223,10 @@ void SILGenFunction::emitCaptures(SILLocation loc,
                                   AnyFunctionRef TheClosure,
                                   SmallVectorImpl<ManagedValue> &capturedArgs) {
   auto captureInfo = SGM.Types.getLoweredLocalCaptures(TheClosure);
+  // For boxed captures, we need to mark the contained variables as having
+  // escaped for DI diagnostics.
+  SmallVector<SILValue, 2> escapesToMark;
+  
   for (auto capture : captureInfo.getCaptures()) {
     auto *vd = capture.getDecl();
 
@@ -285,6 +289,7 @@ void SILGenFunction::emitCaptures(SILLocation loc,
       if (vl.box) {
         B.createStrongRetain(loc, vl.box);
         capturedArgs.push_back(emitManagedRValueWithCleanup(vl.box));
+        escapesToMark.push_back(vl.value);
       } else {
         // Address only 'let' values are passed by box.  This isn't great, in
         // that a variable captured by multiple closures will be boxed for each
@@ -302,6 +307,12 @@ void SILGenFunction::emitCaptures(SILLocation loc,
     }
     }
   }
+  
+  // Mark box addresses as captured for DI purposes. The values must have
+  // been fully initialized before we close over them.
+  if (!escapesToMark.empty()) {
+    B.createMarkFunctionEscape(loc, escapesToMark);
+  }
 }
 
 ManagedValue

lib/SILPasses/EarlySIL/DefiniteInitialization.cpp

@@ -1089,10 +1089,14 @@ void LifetimeChecker::handleEscapeUse(const DIMemoryUse &Use) {
   }
 
   Diag<StringRef, bool> DiagMessage;
-  if (isa<MarkFunctionEscapeInst>(Inst))
-    DiagMessage = diag::global_variable_function_use_uninit;
-  else
-    DiagMessage = diag::variable_escape_before_initialized;
+  if (isa<MarkFunctionEscapeInst>(Inst)) {
+    if (Inst->getLoc().isASTNode<AbstractClosureExpr>())
+      DiagMessage = diag::variable_closure_use_uninit;
+    else
+      DiagMessage = diag::variable_function_use_uninit;
+  } else {
+    DiagMessage = diag::variable_closure_use_uninit;
+  }
 
   diagnoseInitError(Use, DiagMessage);
 }

lib/SILPasses/IPO/CapturePromotion.cpp

@@ -650,7 +650,9 @@ isNonmutatingCapture(SILArgument *BoxArg) {
             return false;
         continue;
       }
-      if (!isa<LoadInst>(O->getUser()) && !isa<DebugValueAddrInst>(O->getUser()))
+      if (!isa<LoadInst>(O->getUser())
+          && !isa<DebugValueAddrInst>(O->getUser())
+          && !isa<MarkFunctionEscapeInst>(O->getUser()))
         return false;
     }
   }
@@ -665,6 +667,10 @@ isNonmutatingCapture(SILArgument *BoxArg) {
 static bool
 isNonescapingUse(Operand *O, SmallVectorImpl<SILInstruction*> &Mutations) {
   auto *U = O->getUser();
+  // Marking the boxed value as escaping is OK. It's just a DI annotation.
+  if (isa<MarkFunctionEscapeInst>(U))
+    return true;
+  
   // A store or assign is ok if the alloc_box is the destination.
   if (isa<StoreInst>(U) || isa<AssignInst>(U)) {
     if (O->getOperandNumber() != 1)

test/SILGen/properties.swift

@@ -675,9 +675,10 @@ func propertyWithDidSetTakingOldValue() {
 // CHECK-NEXT:  debug_value %5 : $Int
 // CHECK-NEXT:  assign %0 to %3 : $*Int
 // CHECK-NEXT:  strong_retain %1 : $@box Int
+// CHECK-NEXT:  mark_function_escape %3
 // CHECK-NEXT:  // function_ref
-// CHECK-NEXT:  %9 = function_ref @_TFF10properties32propertyWithDidSetTakingOldValueFT_T_WL_1pSi : $@convention(thin) (Int, @owned @box Int, @inout Int) -> ()
-// CHECK-NEXT:  %10 = apply %9(%5, %1, %3) : $@convention(thin) (Int, @owned @box Int, @inout Int) -> ()
+// CHECK-NEXT:  %9 = function_ref @_TFF10properties32propertyWithDidSetTakingOldValueFT_T_WL_1pSi : $@convention(thin) (Int, @owned @box Int) -> ()
+// CHECK-NEXT:  %10 = apply %9(%5, %1) : $@convention(thin) (Int, @owned @box Int) -> ()
 // CHECK-NEXT:  strong_release %1 : $@box Int
 // CHECK-NEXT:  %12 = tuple ()
 // CHECK-NEXT:  return %12 : $()

test/SILPasses/definite_init_diagnostics.swift

@@ -3,8 +3,6 @@
 // FIXME: rdar://problem/19648117 Needs splitting objc parts out
 // XFAIL: linux
 
-// XFAIL: *
-
 import Swift
 import gizmo
 
@@ -59,7 +57,7 @@ func test2() {
 
   // expected-warning @+1 {{variable 'b1' was never mutated}} {{3-6=let}}
   var b1 : Int        // expected-note {{variable defined here}}
-  takes_closure {     // expected-error {{variable 'b1' used before being initialized}}
+  takes_closure {     // expected-error {{variable 'b1' captured by a closure before being initialized}}
     markUsed(b1)
   }