Reapply "[Parse] Use CapturedStmt for @finally on MSVC"

This reapplies r334224 and adds explicit triples to some tests to fix
them on Windows (where otherwise they would have run with the default
windows-msvc triple, which I'm changing the behavior for).

Original commit message:
The body of a `@finally` needs to be executed on both exceptional and
non-exceptional paths. On landingpad platforms, this is straightforward:
the `@finally` body is emitted as a normal (non-exceptional) cleanup,
and then a catch-all is emitted which branches to that cleanup (the
cleanup has code to conditionally re-throw based on a flag which is set
by the catch-all).

Unfortunately, we can't use the same approach for MSVC exceptions, where
the catch-all will be emitted as a catchpad. We can't just branch to the
cleanup from within the catchpad, since we can only exit it via a
catchret, at which point the exception is destroyed and we can't
rethrow. We could potentially emit the finally body inside the catchpad
and have the normal cleanup path somehow branch into it, but that would
require some new IR construct that could branch into a catchpad.

Instead, after discussing it with Reid Kleckner, we decided that
frontend outlining was the best approach, similar to how SEH `__finally`
works today. We decided to use CapturedStmt (which was also suggested by
Reid) rather than CaptureFinder (which is what `__finally` uses) since
the latter doesn't handle a lot of cases we care about, e.g. self
accesses, property accesses, block captures, etc. Extending
CaptureFinder to handle those additional cases proved unwieldy, whereas
CapturedStmt already took care of all of those.  In theory `__finally`
could also be moved over to CapturedStmt, which would remove some
existing limitations (e.g. the inability to capture this), although
CaptureFinder would still be needed for SEH filters.

The one case supported by `@finally` but not CapturedStmt (or
CaptureFinder for that matter) is arbitrary control flow out of the
`@finally`, e.g. having a return statement inside a `@finally`. We can
add that support as a follow-up, but in practice we've found it to be
used very rarely anyway.

Differential Revision: https://reviews.llvm.org/D47564

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@334251 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h
index 03c808d394e508d3bb872c2beb5d83c90dd2514a..3b678dcc754f87c0bc858155e2a214c7fca09af8 100644 (file)
--- a/include/clang/AST/Stmt.h
+++ b/include/clang/AST/Stmt.h
@@ -2133,7 +2133,7 @@ private:
 
   /// The pointer part is the implicit the outlined function and the 
   /// int part is the captured region kind, 'CR_Default' etc.
-  llvm::PointerIntPair<CapturedDecl *, 1, CapturedRegionKind> CapDeclAndKind;
+  llvm::PointerIntPair<CapturedDecl *, 2, CapturedRegionKind> CapDeclAndKind;
 
   /// The record for captured variables, a RecordDecl or CXXRecordDecl.
   RecordDecl *TheRecordDecl = nullptr;

diff --git a/include/clang/Basic/CapturedStmt.h b/include/clang/Basic/CapturedStmt.h
index 6c8d6edab0b5023489cde95cc40fa9b3ae306022..324e1b1d3d09329da4a5e3f1f5fa08f48817be20 100644 (file)
--- a/include/clang/Basic/CapturedStmt.h
+++ b/include/clang/Basic/CapturedStmt.h
@@ -16,6 +16,7 @@ namespace clang {
 /// The different kinds of captured statement.
 enum CapturedRegionKind {
   CR_Default,
+  CR_ObjCAtFinally,
   CR_OpenMP
 };
 

diff --git a/include/clang/Sema/ScopeInfo.h b/include/clang/Sema/ScopeInfo.h
index b0f6bac994675d89df9254d2d8bae4e561265bdc..fa1b1037a2f71f7668af483a049e53a401e873d6 100644 (file)
--- a/include/clang/Sema/ScopeInfo.h
+++ b/include/clang/Sema/ScopeInfo.h
@@ -748,6 +748,8 @@ public:
     switch (CapRegionKind) {
     case CR_Default:
       return "default captured statement";
+    case CR_ObjCAtFinally:
+      return "Objective-C @finally statement";
     case CR_OpenMP:
       return "OpenMP region";
     }

diff --git a/lib/Parse/ParseObjc.cpp b/lib/Parse/ParseObjc.cpp
index 6ca0ad855f54064d55293ed9f948692384348b1f..ff33d5fb96bc215ffb2cdf400287db8e7c3ccb91 100644 (file)
--- a/lib/Parse/ParseObjc.cpp
+++ b/lib/Parse/ParseObjc.cpp
@@ -2585,13 +2585,26 @@ StmtResult Parser::ParseObjCTryStmt(SourceLocation atLoc) {
       ParseScope FinallyScope(this,
                               Scope::DeclScope | Scope::CompoundStmtScope);
 
+      bool ShouldCapture =
+          getTargetInfo().getTriple().isWindowsMSVCEnvironment();
+      if (ShouldCapture)
+        Actions.ActOnCapturedRegionStart(Tok.getLocation(), getCurScope(),
+                                         CR_ObjCAtFinally, 1);
+
       StmtResult FinallyBody(true);
       if (Tok.is(tok::l_brace))
         FinallyBody = ParseCompoundStatementBody();
       else
         Diag(Tok, diag::err_expected) << tok::l_brace;
-      if (FinallyBody.isInvalid())
+
+      if (FinallyBody.isInvalid()) {
         FinallyBody = Actions.ActOnNullStmt(Tok.getLocation());
+        if (ShouldCapture)
+          Actions.ActOnCapturedRegionError();
+      } else if (ShouldCapture) {
+        FinallyBody = Actions.ActOnCapturedRegionEnd(FinallyBody.get());
+      }
+
       FinallyStmt = Actions.ActOnObjCAtFinallyStmt(AtCatchFinallyLoc,
                                                    FinallyBody.get());
       catch_or_finally_seen = true;

diff --git a/test/Coverage/ast-printing.m b/test/Coverage/ast-printing.m
index 81c3a6b0ba53b130418483fbc65b5f8229ab759b..d9909b28591562cd253eced1eaed280ea2b8302f 100644 (file)
--- a/test/Coverage/ast-printing.m
+++ b/test/Coverage/ast-printing.m
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -fobjc-exceptions %s
-// RUN: %clang_cc1 -ast-print -fobjc-exceptions %s
-// RUN: %clang_cc1 -ast-dump -fobjc-exceptions %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -fsyntax-only -fobjc-exceptions %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -ast-print -fobjc-exceptions %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -ast-dump -fobjc-exceptions %s
 
 #include "objc-language-features.inc"

diff --git a/test/PCH/objc_stmts.m b/test/PCH/objc_stmts.m
index 8deb14a8154511263823616f2842650dd4495342..ee7b780731a9708885c55fcefd2c62ef3ab61b54 100644 (file)
--- a/test/PCH/objc_stmts.m
+++ b/test/PCH/objc_stmts.m
@@ -1,11 +1,11 @@
 // Test this without pch.
-// RUN: %clang_cc1 -include %S/objc_stmts.h -emit-llvm -fobjc-exceptions -o - %s
-// RUN: %clang_cc1 -include %S/objc_stmts.h -ast-print -fobjc-exceptions -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -include %S/objc_stmts.h -emit-llvm -fobjc-exceptions -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -include %S/objc_stmts.h -ast-print -fobjc-exceptions -o - %s | FileCheck %s
 
 // Test with pch.
-// RUN: %clang_cc1 -x objective-c -emit-pch -fobjc-exceptions -o %t %S/objc_stmts.h
-// RUN: %clang_cc1 -include-pch %t -emit-llvm -fobjc-exceptions -o - %s 
-// RUN: %clang_cc1 -include-pch %t -ast-print -fobjc-exceptions -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -x objective-c -emit-pch -fobjc-exceptions -o %t %S/objc_stmts.h
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -include-pch %t -emit-llvm -fobjc-exceptions -o - %s 
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -include-pch %t -ast-print -fobjc-exceptions -o - %s | FileCheck %s
 
 // CHECK: @catch(A *a)
 // CHECK: @catch(B *b)

diff --git a/test/Parser/objc-try-catch-1.m b/test/Parser/objc-try-catch-1.m
index a3220ebc64774caea97da587393657b03476408a..3a60148c8be9091ec172c550238c98148b9134a4 100644 (file)
--- a/test/Parser/objc-try-catch-1.m
+++ b/test/Parser/objc-try-catch-1.m
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -fobjc-exceptions %s
-// RUN: %clang_cc1 -fsyntax-only -verify -fobjc-exceptions -x objective-c++ %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -fsyntax-only -verify -fobjc-exceptions %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -fsyntax-only -verify -fobjc-exceptions -x objective-c++ %s
 void * proc();
 
 @interface NSConstantString

diff --git a/test/Rewriter/rewrite-modern-try-finally.m b/test/Rewriter/rewrite-modern-try-finally.m
index 500133b8614266f28610bed22d7ce1424a90c3ad..41737e95f0cd51a855648df979601d39a931ace2 100644 (file)
--- a/test/Rewriter/rewrite-modern-try-finally.m
+++ b/test/Rewriter/rewrite-modern-try-finally.m
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -x objective-c -Wno-return-type -fblocks -fms-extensions -rewrite-objc %s -o %t-rw.cpp
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -fexceptions  -Wno-address-of-temporary -D"SEL=void*" -D"__declspec(X)=" %t-rw.cpp
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -x objective-c -Wno-return-type -fblocks -fms-extensions -rewrite-objc %s -o %t-rw.cpp
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -fsyntax-only -fcxx-exceptions -fexceptions  -Wno-address-of-temporary -D"SEL=void*" -D"__declspec(X)=" %t-rw.cpp
 
 typedef struct objc_class *Class;
 typedef struct objc_object {

diff --git a/test/SemaObjC/finally-msvc.m b/test/SemaObjC/finally-msvc.m
new file mode 100644 (file)
index 0000000..5db08a7
--- /dev/null
+++ b/test/SemaObjC/finally-msvc.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple i686--windows-msvc -fexceptions -fobjc-exceptions -ast-dump %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64--windows-msvc -fexceptions -fobjc-exceptions -ast-dump %s 2>&1 | FileCheck %s
+
+void f() {
+  @try {
+  } @finally {
+  }
+}
+
+// CHECK:      ObjCAtFinallyStmt
+// CHECK-NEXT:   CapturedStmt
+// CHECK-NEXT:     CapturedDecl
+// CHECK-NEXT:       CompoundStmt
+// CHECK-NEXT:       ImplicitParamDecl

diff --git a/test/SemaObjC/scope-check.m b/test/SemaObjC/scope-check.m
index b52e7a06ffc2bb92f7d08ee3dbd0d30a800ff1a9..b23edd1240f9b4492e3f2b6c39e185caf8d70f76 100644 (file)
--- a/test/SemaObjC/scope-check.m
+++ b/test/SemaObjC/scope-check.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -fobjc-exceptions -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 -fsyntax-only -verify -fobjc-exceptions -Wno-objc-root-class %s
 
 @class A, B, C;