[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