[PATCH] Sema for Captured Statements

Doug Gregor dgregor at apple.com
Thu Mar 7 17:40:57 PST 2013


  This is looking pretty good. My main complaint is the FunctionDecl we're synthesizing to act as the DeclContext. I think it's doing more harm than good, because we'll have to be very careful to avoid tripping over this FunctionDecl elsewhere in the compiler (e.g., why does code completion show me this __captured_stmt_helperV2 function? or typo correction?). All we need the FunctionDecl for is to act as a DeclContext, because CodeGen can easily build the LLVM function, and calls to it, because the signature is so regular anyway. I suggest adding a CapturedStmtDecl (similar to BlockDecl) to act as the DeclContext.


================
Comment at: lib/Sema/SemaExprCXX.cpp:740
@@ +739,3 @@
+    if (CapturedRegionScopeInfo *RSI
+        = dyn_cast<CapturedRegionScopeInfo>(FunctionScopes[idx])) {
+      RecordDecl *RD = RSI->TheRecordDecl;
----------------
Please indent the '=' an extra two spaces.

================
Comment at: lib/Sema/SemaExprCXX.cpp:739
@@ +738,3 @@
+
+    if (CapturedRegionScopeInfo *RSI
+        = dyn_cast<CapturedRegionScopeInfo>(FunctionScopes[idx])) {
----------------
else if... ?

================
Comment at: lib/Sema/SemaStmt.cpp:2312
@@ -2307,1 +2311,3 @@
+  }
+
   // For blocks/lambdas with implicit return types, we check each return
----------------
There's an if-else chain below (~line 2352)  that checks block and lambda scopes. How about just extending those checks? HasImplicitReturnType will be false anyway.

================
Comment at: lib/Sema/SemaStmt.cpp:2868
@@ +2867,3 @@
+
+  IdentifierInfo *Id = &PP.getIdentifierTable().get("capture");
+  RecordDecl *RD = 0;
----------------
Please make the generated struct anonymous. We want no chance of it showing up anywhere.

================
Comment at: lib/Sema/SemaStmt.cpp:2940
@@ +2939,3 @@
+static IdentifierInfo *GetMangledHelperName(Sema &S) {
+  static unsigned count = 0;
+  StringRef name("__captured_stmt_helperV");
----------------
This isn't thread-safe. See my general comment below.


http://llvm-reviews.chandlerc.com/D433



More information about the cfe-commits mailing list