[cfe-dev] [RFC] Captured Statements

[Dmitri Gribenko]

On Wed, Jan 30, 2013 at 10:23 PM, Pan, Wei <wei.pan at intel.com> wrote:
> Hi Doug,
>
> Thanks for the feedback! All suggestions make good sense to us. We will address them in our future patches.
>
> Before sending out patches for commit, we are attaching an *incomplete* implementation of captured statements. By applying this patch, clang should compile the following function:
>
> void foo(int &x) {
>   #pragma captured
>   {
>     int y = 100;
>     x += y;
>
>     #pragma captured
>     {
>       y++;
>       x -= y;
>     }
>   }
> }
>
> There are a number of missing features like template support that we will be working on.
>
> We would welcome any suggestions or feedback.

Hi Wei,

This is not a full review.

+  CXCursor_LastStmt                      = CXCursor_ExampleCapturedStmt,

Please bump CINDEX_VERSION_MINOR.

+// \brief the base class for capturing a statement into a separate function

Please use three slashes, a capital letter at the beginning and a full
stop at the end.

+void CapturedStmt::setCaptures(ASTContext &Context,
+                               const Capture *begin,
+                               const Capture *end) {
...
+  void *buffer = Context.Allocate(allocationSize, /*alignment*/sizeof(void*));

(1) Use llvm::alignOf<>.
(2) Tail-allocate instead of doing an additional allocation.  See
CXXTryStmt::Create as an example.

+++ b/test/CodeGen/captured-statements.c
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-check-prefix=CHECK-GLOBALS
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-1
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-2

Please use a temporary file.

+++ b/test/CodeGenCXX/captured-statements.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - | FileCheck %s
-check-prefix=CHECK-1

This test uses CHECK-2 and CHECK-3, too.

+  if (CurCGCapturedStmtInfo)
+    delete CurCGCapturedStmtInfo;

It is safe to delete a null pointer.

+  Token *Toks = (Token*) PP.getPreprocessorAllocator().Allocate(
+    sizeof(Token) * 1, llvm::alignOf<Token>());
+  new (Toks) Token();

Allocate<Token>() ?

+  if (Tok.isNot(tok::l_brace)) {
+    PP.Diag(Tok, diag::err_expected_lbrace) << "after #pragma captured";

Please don't put prose into %x.  Define a separate diagnostic if needed.

+SmallVector<CapturedStmt::Capture, 4>
+Sema::buildCapturedStmtCaptureList(SmallVector<CapturingScopeInfo::Capture, 4>
+                                     &Candidates) {

Candidates should be an ArrayRef, and return value should be an
out-parameter of type SmallVectorImpl<>.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/