[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-06-07 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334224: [Parse] Use CapturedStmt for @finally on MSVC 
(authored by smeenai, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47564

Files:
  cfe/trunk/include/clang/AST/Stmt.h
  cfe/trunk/include/clang/Basic/CapturedStmt.h
  cfe/trunk/include/clang/Sema/ScopeInfo.h
  cfe/trunk/lib/Parse/ParseObjc.cpp
  cfe/trunk/test/SemaObjC/finally-msvc.m


Index: cfe/trunk/test/SemaObjC/finally-msvc.m
===
--- cfe/trunk/test/SemaObjC/finally-msvc.m
+++ cfe/trunk/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
Index: cfe/trunk/lib/Parse/ParseObjc.cpp
===
--- cfe/trunk/lib/Parse/ParseObjc.cpp
+++ cfe/trunk/lib/Parse/ParseObjc.cpp
@@ -2585,13 +2585,26 @@
   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;
Index: cfe/trunk/include/clang/AST/Stmt.h
===
--- cfe/trunk/include/clang/AST/Stmt.h
+++ cfe/trunk/include/clang/AST/Stmt.h
@@ -2133,7 +2133,7 @@
 
   /// The pointer part is the implicit the outlined function and the 
   /// int part is the captured region kind, 'CR_Default' etc.
-  llvm::PointerIntPair CapDeclAndKind;
+  llvm::PointerIntPair CapDeclAndKind;
 
   /// The record for captured variables, a RecordDecl or CXXRecordDecl.
   RecordDecl *TheRecordDecl = nullptr;
Index: cfe/trunk/include/clang/Sema/ScopeInfo.h
===
--- cfe/trunk/include/clang/Sema/ScopeInfo.h
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h
@@ -748,6 +748,8 @@
 switch (CapRegionKind) {
 case CR_Default:
   return "default captured statement";
+case CR_ObjCAtFinally:
+  return "Objective-C @finally statement";
 case CR_OpenMP:
   return "OpenMP region";
 }
Index: cfe/trunk/include/clang/Basic/CapturedStmt.h
===
--- cfe/trunk/include/clang/Basic/CapturedStmt.h
+++ cfe/trunk/include/clang/Basic/CapturedStmt.h
@@ -16,6 +16,7 @@
 /// The different kinds of captured statement.
 enum CapturedRegionKind {
   CR_Default,
+  CR_ObjCAtFinally,
   CR_OpenMP
 };
 


Index: cfe/trunk/test/SemaObjC/finally-msvc.m
===
--- cfe/trunk/test/SemaObjC/finally-msvc.m
+++ cfe/trunk/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
Index: cfe/trunk/lib/Parse/ParseObjc.cpp
===
--- cfe/trunk/lib/Parse/ParseObjc.cpp
+++ cfe/trunk/lib/Parse/ParseObjc.cpp
@@ -2585,13 +2585,26 @@
   ParseScope FinallyScope(this,
   Scope::DeclScope | Scope::CompoundStmtScope);
 
+  bool ShouldCapture =
+  getTargetInfo().getTriple().isWindowsMSVCEnvironment();
+  if (ShouldCapture)
+Actions.ActOnCapturedRegionStart(Tok.getLocation(), 

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-06-04 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay,  We can try this, then.


Repository:
  rC Clang

https://reviews.llvm.org/D47564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-06-04 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment.

In https://reviews.llvm.org/D47564#1119925, @rjmccall wrote:

> In https://reviews.llvm.org/D47564#1119874, @smeenai wrote:
>
> > In https://reviews.llvm.org/D47564#1118424, @rjmccall wrote:
> >
> > > No, it was just a general question.  Have you gotten this to a point 
> > > where it's testable?
> >
> >
> > Yup, it's been working fine in my local testing. There's one more patch 
> > that I need to put up, which actually handles doing proper codegen for 
> > @try/@catch/@finally; I'm working on cleaning that up right now.
>
>
> Okay.  And simple tests with throwing exceptions out of the `@finally` block 
> seem to work?


Yup, it works fine. It's essentially the same as calling a function inside a 
catch block which throws (since the finally is modeled as a catch-all, and the 
finally body is outlined into a function which gets called by that catch-all).


Repository:
  rC Clang

https://reviews.llvm.org/D47564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-06-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/D47564#1119874, @smeenai wrote:

> In https://reviews.llvm.org/D47564#1118424, @rjmccall wrote:
>
> > No, it was just a general question.  Have you gotten this to a point where 
> > it's testable?
>
>
> Yup, it's been working fine in my local testing. There's one more patch that 
> I need to put up, which actually handles doing proper codegen for 
> @try/@catch/@finally; I'm working on cleaning that up right now.


Okay.  And simple tests with throwing exceptions out of the `@finally` block 
seem to work?

> The other piece of the puzzle is https://reviews.llvm.org/D47233, which emits 
> the proper typeinfo required for this.

Yes, I'll try to get to that, sorry.


Repository:
  rC Clang

https://reviews.llvm.org/D47564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-06-01 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment.

In https://reviews.llvm.org/D47564#1118424, @rjmccall wrote:

> In https://reviews.llvm.org/D47564#1118423, @smeenai wrote:
>
> > In https://reviews.llvm.org/D47564#1118381, @rjmccall wrote:
> >
> > > That's an interesting idea.  I don't see any particular reason not to do 
> > > it this way if you're willing to accept that it's never going to support 
> > > the full control-flow possibilities of @finally.  You will need to add 
> > > JumpDiagnostics logic to prevent branches out of the block, and I don't 
> > > know how this will interact with attempts to throw an exception out.
> >
> >
> > There's already some logic in CapturedStmt to prevent branches out of the 
> > block:
> >
> > - Attempting to return will produce "cannot return from Objective-C 
> > @finally statement"
> > - Attempting to goto out of the block will result in "use of undeclared 
> > label", which is a bad diagnostic (and should be improved), but it does 
> > error
>
>
> Alright, that makes sense.
>
> > It should be possible to add support for returns, at least; the idea we'd 
> > discussed with @rnk was setting a flag in the captured function to indicate 
> > a return having been executed, and then reading that flag outside the 
> > captured function and acting on it appropriately. gotos would be more 
> > complicated, but I think we could make them work if we really wanted to.
> > 
> > Throwing an exception out should just work, I think; the outlined function 
> > will just participate normally in exception handling. Did you have a 
> > specific case you were thinking of?
>
> No, it was just a general question.  Have you gotten this to a point where 
> it's testable?


Yup, it's been working fine in my local testing. There's one more patch that I 
need to put up, which actually handles doing proper codegen for 
@try/@catch/@finally; I'm working on cleaning that up right now. The other 
piece of the puzzle is https://reviews.llvm.org/D47233, which emits the proper 
typeinfo required for this.


Repository:
  rC Clang

https://reviews.llvm.org/D47564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-05-31 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/D47564#1118423, @smeenai wrote:

> In https://reviews.llvm.org/D47564#1118381, @rjmccall wrote:
>
> > That's an interesting idea.  I don't see any particular reason not to do it 
> > this way if you're willing to accept that it's never going to support the 
> > full control-flow possibilities of @finally.  You will need to add 
> > JumpDiagnostics logic to prevent branches out of the block, and I don't 
> > know how this will interact with attempts to throw an exception out.
>
>
> There's already some logic in CapturedStmt to prevent branches out of the 
> block:
>
> - Attempting to return will produce "cannot return from Objective-C @finally 
> statement"
> - Attempting to goto out of the block will result in "use of undeclared 
> label", which is a bad diagnostic (and should be improved), but it does error


Alright, that makes sense.

> It should be possible to add support for returns, at least; the idea we'd 
> discussed with @rnk was setting a flag in the captured function to indicate a 
> return having been executed, and then reading that flag outside the captured 
> function and acting on it appropriately. gotos would be more complicated, but 
> I think we could make them work if we really wanted to.
> 
> Throwing an exception out should just work, I think; the outlined function 
> will just participate normally in exception handling. Did you have a specific 
> case you were thinking of?

No, it was just a general question.  Have you gotten this to a point where it's 
testable?


Repository:
  rC Clang

https://reviews.llvm.org/D47564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-05-31 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment.

In https://reviews.llvm.org/D47564#1118381, @rjmccall wrote:

> That's an interesting idea.  I don't see any particular reason not to do it 
> this way if you're willing to accept that it's never going to support the 
> full control-flow possibilities of @finally.  You will need to add 
> JumpDiagnostics logic to prevent branches out of the block, and I don't know 
> how this will interact with attempts to throw an exception out.


There's already some logic in CapturedStmt to prevent branches out of the block:

- Attempting to return will produce "cannot return from Objective-C @finally 
statement"
- Attempting to goto out of the block will result in "use of undeclared label", 
which is a bad diagnostic (and should be improved), but it does error

Are there any other branches you had in mind? I could add tests for them, 
perhaps, though it's also covered by `test/Sema/captured-statements.c`.

It should be possible to add support for returns, at least; the idea we'd 
discussed with @rnk was setting a flag in the captured function to indicate a 
return having been executed, and then reading that flag outside the captured 
function and acting on it appropriately. gotos would be more complicated, but I 
think we could make them work if we really wanted to.

Throwing an exception out should just work, I think; the outlined function will 
just participate normally in exception handling. Did you have a specific case 
you were thinking of?


Repository:
  rC Clang

https://reviews.llvm.org/D47564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-05-31 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

That's an interesting idea.  I don't see any particular reason not to do it 
this way if you're willing to accept that it's never going to support the full 
control-flow possibilities of @finally.  You will need to add JumpDiagnostics 
logic to prevent branches out of the block, and I don't know how this will 
interact with attempts to throw an exception out.


Repository:
  rC Clang

https://reviews.llvm.org/D47564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-05-31 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 149351.
smeenai edited the summary of this revision.
smeenai added a comment.

@rnk comment


Repository:
  rC Clang

https://reviews.llvm.org/D47564

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/CapturedStmt.h
  include/clang/Sema/ScopeInfo.h
  lib/Parse/ParseObjc.cpp
  test/SemaObjC/finally-msvc.m


Index: test/SemaObjC/finally-msvc.m
===
--- /dev/null
+++ 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
Index: lib/Parse/ParseObjc.cpp
===
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -2585,13 +2585,26 @@
   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;
Index: include/clang/Sema/ScopeInfo.h
===
--- include/clang/Sema/ScopeInfo.h
+++ include/clang/Sema/ScopeInfo.h
@@ -748,6 +748,8 @@
 switch (CapRegionKind) {
 case CR_Default:
   return "default captured statement";
+case CR_ObjCAtFinally:
+  return "Objective-C @finally statement";
 case CR_OpenMP:
   return "OpenMP region";
 }
Index: include/clang/Basic/CapturedStmt.h
===
--- include/clang/Basic/CapturedStmt.h
+++ include/clang/Basic/CapturedStmt.h
@@ -16,6 +16,7 @@
 /// The different kinds of captured statement.
 enum CapturedRegionKind {
   CR_Default,
+  CR_ObjCAtFinally,
   CR_OpenMP
 };
 
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -2133,7 +2133,7 @@
 
   /// The pointer part is the implicit the outlined function and the 
   /// int part is the captured region kind, 'CR_Default' etc.
-  llvm::PointerIntPair CapDeclAndKind;
+  llvm::PointerIntPair CapDeclAndKind;
 
   /// The record for captured variables, a RecordDecl or CXXRecordDecl.
   RecordDecl *TheRecordDecl = nullptr;


Index: test/SemaObjC/finally-msvc.m
===
--- /dev/null
+++ 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
Index: lib/Parse/ParseObjc.cpp
===
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -2585,13 +2585,26 @@
   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 = 

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-05-31 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

Using CapturedStmt to do frontend outlining was the direction I suggested. I 
want to hear what @rsmith and @rjmccall think, though.




Comment at: lib/Parse/ParseObjc.cpp:2588
 
+  bool ShouldCapture = Actions.getASTContext()
+   .getTargetInfo()

Parser has a `getTargetInfo()` method, so this can be shorter.


Repository:
  rC Clang

https://reviews.llvm.org/D47564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-05-30 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai updated this revision to Diff 149220.
smeenai added a comment.

Proper diff


Repository:
  rC Clang

https://reviews.llvm.org/D47564

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/CapturedStmt.h
  include/clang/Sema/ScopeInfo.h
  lib/Parse/ParseObjc.cpp
  test/SemaObjC/finally-msvc.m


Index: test/SemaObjC/finally-msvc.m
===
--- /dev/null
+++ 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
Index: lib/Parse/ParseObjc.cpp
===
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -2585,13 +2585,28 @@
   ParseScope FinallyScope(this,
   Scope::DeclScope | Scope::CompoundStmtScope);
 
+  bool ShouldCapture = Actions.getASTContext()
+   .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;
Index: include/clang/Sema/ScopeInfo.h
===
--- include/clang/Sema/ScopeInfo.h
+++ include/clang/Sema/ScopeInfo.h
@@ -748,6 +748,8 @@
 switch (CapRegionKind) {
 case CR_Default:
   return "default captured statement";
+case CR_ObjCAtFinally:
+  return "Objective-C @finally statement";
 case CR_OpenMP:
   return "OpenMP region";
 }
Index: include/clang/Basic/CapturedStmt.h
===
--- include/clang/Basic/CapturedStmt.h
+++ include/clang/Basic/CapturedStmt.h
@@ -16,6 +16,7 @@
 /// The different kinds of captured statement.
 enum CapturedRegionKind {
   CR_Default,
+  CR_ObjCAtFinally,
   CR_OpenMP
 };
 
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -2133,7 +2133,7 @@
 
   /// The pointer part is the implicit the outlined function and the 
   /// int part is the captured region kind, 'CR_Default' etc.
-  llvm::PointerIntPair CapDeclAndKind;
+  llvm::PointerIntPair CapDeclAndKind;
 
   /// The record for captured variables, a RecordDecl or CXXRecordDecl.
   RecordDecl *TheRecordDecl = nullptr;


Index: test/SemaObjC/finally-msvc.m
===
--- /dev/null
+++ 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
Index: lib/Parse/ParseObjc.cpp
===
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -2585,13 +2585,28 @@
   ParseScope FinallyScope(this,
   Scope::DeclScope | Scope::CompoundStmtScope);
 
+  bool ShouldCapture = Actions.getASTContext()
+   .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
   

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

2018-05-30 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision.
smeenai added reviewers: rjmccall, rnk, rsmith.
Herald added a subscriber: cfe-commits.

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 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.


Repository:
  rC Clang

https://reviews.llvm.org/D47564

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/CapturedStmt.h
  include/clang/Sema/ScopeInfo.h
  lib/CodeGen/CGCXXABI.h
  lib/CodeGen/CGObjCMac.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  lib/Parse/ParseObjc.cpp
  test/CodeGenObjC/dllstorage.m
  test/CodeGenObjC/exceptions-msvc.m
  test/SemaObjC/finally-msvc.m

Index: test/SemaObjC/finally-msvc.m
===
--- /dev/null
+++ 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
Index: test/CodeGenObjC/exceptions-msvc.m
===
--- /dev/null
+++ test/CodeGenObjC/exceptions-msvc.m
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -triple i686--windows-msvc -fobjc-runtime=ios-6.0 -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -triple i686--windows-msvc -fobjc-runtime=ios-6.0 -fobjc-arc -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -triple x86_64--windows-msvc -fobjc-runtime=ios-6.0 -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X64 %s
+// RUN: %clang_cc1 -triple x86_64--windows-msvc -fobjc-runtime=ios-6.0 -fobjc-arc -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X64 %s
+
+#if __has_feature(objc_arc)
+#define WEAK __weak
+#else
+#define WEAK
+#endif
+
+// CHECK-DAG: $OBJC_EHTYPE_id = comdat any
+// X86-DAG: @OBJC_EHTYPE_id = linkonce_odr global {{%[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [18 x i8] c".PAUobjc_object@@\00" }, comdat
+// X64-DAG: @OBJC_EHTYPE_id = linkonce_odr global {{%[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [19 x i8] c".PEAUobjc_object@@\00" }, comdat
+
+@class I;
+
+// CHECK-DAG: $"OBJC_EHTYPE_$_I" = comdat any
+// X86-DAG: @"OBJC_EHTYPE_$_I" = linkonce_odr global {{%[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [8 x i8] c".PAUI@@\00" }, comdat
+// X64-DAG: @"OBJC_EHTYPE_$_I" = linkonce_odr global {{%[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [9 x i8] c".PEAUI@@\00" }, comdat
+
+@class J;
+
+// CHECK-DAG: $"OBJC_EHTYPE_$_J" = comdat any
+// X86-DAG: @"OBJC_EHTYPE_$_J" = linkonce_odr global {{%[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [8 x i8] c".PAUJ@@\00" }, comdat
+// X64-DAG: @"OBJC_EHTYPE_$_J" = linkonce_odr global {{%[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [9 x i8] c".PEAUJ@@\00" }, comdat
+
+// The EHType shouldn't be exported