Reject uses of noescape parameters that would cause them to escape. Namely, we only

allow the parameter to be caller, or captured by another no-escape closure.


Swift SVN r24121

Author: lattner

include/swift/AST/DiagnosticsSema.def

@@ -1432,6 +1432,15 @@ NOTE(add_self_to_type,tce_sema,none,
 
 ERROR(type_of_metatype,tce_sema,none,
       "'.dynamicType' is not allowed after a type name", ())
+ERROR(invalid_noescape_use,tce_sema,none,
+      "@noescape argument %0 may only be called", (Identifier))
+
+ERROR(closure_noescape_use,tce_sema,none,
+      "closure use of @noescape argument %0 may allow it to escape",
+      (Identifier))
+ERROR(decl_closure_noescape_use,tce_sema,none,
+      "declaration closing over @noescape argument %0 may allow it to escape",
+      (Identifier))
 
 //------------------------------------------------------------------------------
 // Type Check Statements

lib/Sema/MiscDiagnostics.cpp

@@ -105,6 +105,7 @@ static void diagUnreachableCode(TypeChecker &TC, const Stmt *S) {
 ///   - Module values may only occur as part of qualification.
 ///   - Metatype names cannot generally be used as values: they need a "T.self"
 ///     qualification unless used in narrow case (e.g. T() for construction).
+///   - NoEscape parameters are only allowed to be called, not copied around.
 ///
 static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E) {
   class DiagnoseWalker : public ASTWalker {
@@ -122,20 +123,31 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E) {
       // Diagnose metatype values that don't appear as part of a property,
       // method, or constructor reference.
 
-      // See through implicit conversions.  We want to treat things like:
-      /// (apply_expr (metatype_cvt (typeexpr metatype)) (4)) as an apply.
+      // See through implicit conversions of the expression.  We want to be able
+      // to associate the parent of this expression with the ultimate callee.
       auto Base = E;
       while (auto Conv = dyn_cast<ImplicitConversionExpr>(Base))
         Base = Conv->getSubExpr();
-      if (auto *DRE = dyn_cast<DeclRefExpr>(Base))
+
+      if (auto *DRE = dyn_cast<DeclRefExpr>(Base)) {
+        // Verify metatype uses.
         if (isa<TypeDecl>(DRE->getDecl()))
           checkUseOfMetaTypeName(Base);
+
+        // Verify noescape parameter uses.
+        if (auto *PD = dyn_cast<ParamDecl>(DRE->getDecl()))
+          if (PD->getAttrs().hasAttribute<NoEscapeAttr>())
+            checkNoEscapeParameterUse(DRE);
+      }
       if (auto *MRE = dyn_cast<MemberRefExpr>(Base))
         if (isa<TypeDecl>(MRE->getMember().getDecl()))
           checkUseOfMetaTypeName(Base);
       if (isa<TypeExpr>(Base))
         checkUseOfMetaTypeName(Base);
 
+      if (auto *CE = dyn_cast<ClosureExpr>(E))
+        checkNoEscapeClosureCaptures(CE);
+
       return { true, E };
     }
 
@@ -152,6 +164,37 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E) {
       TC.diagnose(E->getStartLoc(), diag::value_of_module_type);
     }
 
+    /// The DRE argument is a reference to a noescape parameter.  Verify that
+    /// its uses are ok.
+    void checkNoEscapeParameterUse(DeclRefExpr *DRE) {
+      // The only valid use of the noescape parameter is an immediate call.
+      // TODO: this could be generalized in various ways, for example to allow
+      // passing the decl off as an argument to a noescape call.  Start simple.
+      if (auto *ParentExpr = Parent.getAsExpr())
+        if (isa<CallExpr>(ParentExpr)) // param()
+          return;
+      TC.diagnose(DRE->getStartLoc(), diag::invalid_noescape_use,
+                  DRE->getDecl()->getName());
+    }
+
+    /// Check the specified closure to make sure it doesn't capture a noescape
+    /// value, or that it is itself noescape if so.
+    void checkNoEscapeClosureCaptures(ClosureExpr *CE) {
+      if (!CE->getType() || CE->getType()->is<ErrorType>())
+        return; // Ignore errorneous code.
+      auto Ty = CE->getType()->castTo<FunctionType>();
+
+      // If this closure is used in a noescape context, it can do anything.
+      if (Ty->isNoEscape()) return;
+
+      // Otherwise, check the capture list to make sure it isn't escaping
+      // something.
+      for (auto CapVD : CE->getCaptureInfo().getCaptures())
+        if (CapVD->getAttrs().hasAttribute<NoEscapeAttr>())
+          TC.diagnose(CE->getStartLoc(), diag::closure_noescape_use,
+                      CapVD->getName());
+    }
+
     void checkUseOfMetaTypeName(Expr *E) {
       // If we've already checked this at a higher level, we're done.
       if (!AlreadyDiagnosedMetatypes.insert(E).second)

lib/Sema/TypeCheckExpr.cpp

@@ -614,15 +614,14 @@ namespace {
   class FindCapturedVars : public ASTWalker {
     TypeChecker &TC;
     llvm::SetVector<ValueDecl*> &captures;
-    DeclContext *CurExprAsDC;
+    DeclContext *CurDC;
     SourceLoc CaptureLoc;
     llvm::SmallPtrSet<ValueDecl *, 2> Diagnosed;
 
   public:
-    FindCapturedVars(TypeChecker &tc,
-                     llvm::SetVector<ValueDecl*> &captures,
+    FindCapturedVars(TypeChecker &tc, llvm::SetVector<ValueDecl*> &captures,
                      AnyFunctionRef AFR)
-        : TC(tc), captures(captures), CurExprAsDC(AFR.getAsDeclContext()) {
+        : TC(tc), captures(captures), CurDC(AFR.getAsDeclContext()) {
       if (auto AFD = AFR.getAbstractFunctionDecl())
         CaptureLoc = AFD->getLoc();
       else {
@@ -642,28 +641,34 @@ namespace {
       S->walk(*this);
     }
 
-    std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
-      if (auto *DRE = dyn_cast<DeclRefExpr>(E))
-        return walkToDeclRefExpr(DRE);
+    /// Add the specified capture to the closure's capture list, diagnosing it
+    /// if invalid.
+    void addCapture(ValueDecl *VD, SourceLoc Loc) {
+      captures.insert(VD);
 
-      // Don't recur into child closures. They should already have a capture
-      // list computed; we just propagate it, filtering out stuff that they
-      // capture from us.
-      if (auto *SubCE = dyn_cast<AbstractClosureExpr>(E)) {
-        for (auto D : SubCE->getCaptureInfo().getCaptures())
-          if (D->getDeclContext() != CurExprAsDC)
-            captures.insert(D);
-        return { false, E };
-      }
-      return { true, E };
+      // If VD is a noescape decl, then the closure we're computing this for
+      // must also be noescape.
+      if (!VD->getAttrs().hasAttribute<NoEscapeAttr>())
+        return;
+
+      // Closure expressions are processed as part of expression checking.
+      if (isa<AbstractClosureExpr>(CurDC))
+        return;
+
+      // Don't repeatedly diagnose the same thing.
+      if (!Diagnosed.insert(VD).second)
+        return;
+
+      // Otherwise, diagnose this as an invalid capture.
+      TC.diagnose(Loc, diag::decl_closure_noescape_use, VD->getName());
     }
 
     std::pair<bool, Expr *> walkToDeclRefExpr(DeclRefExpr *DRE) {
       auto *D = DRE->getDecl();
 
       // Decl references that are within the Capture are local references, ones
       // from parent context are captures.
-      if (!CurExprAsDC->isChildContextOf(D->getDeclContext()))
+      if (!CurDC->isChildContextOf(D->getDeclContext()))
         return { false, DRE };
 
       // Only capture var decls at global scope.  Other things can be captured
@@ -688,7 +693,7 @@ namespace {
       if (auto FD = dyn_cast<FuncDecl>(D)) {
         // TODO: Local functions cannot be recursive, because SILGen
         // cannot handle it yet.
-        if (CurExprAsDC == FD) {
+        if (CurDC == FD) {
           TC.diagnose(DRE->getLoc(), 
                       diag::unsupported_recursive_local_function);
           return { false, DRE };
@@ -704,10 +709,25 @@ namespace {
           TC.LocalFunctionCaptures.push_back({FD, DRE->getLoc()});
       }
 
-      captures.insert(D);
+      addCapture(D, DRE->getStartLoc());
       return { false, DRE };
     }
 
+    std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
+      if (auto *DRE = dyn_cast<DeclRefExpr>(E))
+        return walkToDeclRefExpr(DRE);
+
+      // Don't recurse into child closures. They should already have a capture
+      // list computed; we just propagate it, filtering out stuff that they
+      // capture from us.
+      if (auto *SubCE = dyn_cast<AbstractClosureExpr>(E)) {
+        for (auto D : SubCE->getCaptureInfo().getCaptures())
+          if (D->getDeclContext() != CurDC)
+            addCapture(D, E->getStartLoc());
+        return { false, E };
+      }
+      return { true, E };
+    }
   };
 }
 

test/attr/attr_noescape.swift

@@ -2,17 +2,40 @@
 
 @__noescape var fn : () -> Int = { 4 }  // expected-error {{'__noescape' may only be used on 'parameter' declarations}}
 
-func takesClosure(@__noescape fn : () -> Int) {
-  takesClosure { 4 }  // ok
+func doesEscape(fn : () -> Int) {}
+
+func takesNoEscapeClosure(@__noescape fn : () -> Int) {
+  takesNoEscapeClosure { 4 }  // ok
+
+  fn()  // ok
+
+  var x = fn  // expected-error {{@noescape argument 'fn' may only be called}}
+
+  // This is ok, because the closure itself is noescape.
+  takesNoEscapeClosure { fn() }
+
+  // This is not ok, because it escapes the 'fn' closure.
+  doesEscape { fn() }   // expected-error {{closure use of @noescape argument 'fn' may allow it to escape}}
+
+  // This is not ok, because it escapes the 'fn' closure.
+  func nested_function() {
+    fn()   // expected-error {{declaration closing over @noescape argument 'fn' may allow it to escape}}
+  }
+
 }
 
 class SomeClass {
   final var x = 42
 
   func test() {
+    // This should require "self."
+    doesEscape { x }  // expected-error {{reference to property 'x' in closure requires explicit 'self.' to make capture semantics explicit}}
+
     // Since 'takesClosure' doesn't escape its closure, it doesn't require
     // "self." qualification of member references.
-    takesClosure { x }
+    takesNoEscapeClosure { x }
+
+
   }