jfb created this revision.
jfb added a reviewer: rjmccall.
Herald added subscribers: cfe-commits, aheejin.
When a lambda capture captures a __block in the same statement, the compiler
asserts out because isCapturedBy assumes that an Expr can only be a BlockExpr,
StmtExpr, or if it's a Stmt then all the statement's children are expressions.
That's wrong, we need to visit all sub-statements even if they're not
expressions to see if they also capture.
Fix this issue by pulling out the isCapturedBy logic to use RecursiveASTVisitor.
rdar://problem/39926584
Repository:
rC Clang
https://reviews.llvm.org/D47096
Files:
lib/CodeGen/CGDecl.cpp
test/CodeGen/block-capture.cpp
Index: test/CodeGen/block-capture.cpp
===================================================================
--- /dev/null
+++ test/CodeGen/block-capture.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -fblocks -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %struct.__block_byref_baz = type { i8*, %struct.__block_byref_baz*, i32, i32, i32 }
+// CHECK: [[baz:%[0-9a-z_]*]] = alloca %struct.__block_byref_baz
+// CHECK: [[bazref:%[0-9a-z_\.]*]] = getelementptr inbounds %struct.__block_byref_baz, %struct.__block_byref_baz* [[baz]], i32 0, i32 1
+// CHECK: store %struct.__block_byref_baz* [[baz]], %struct.__block_byref_baz** [[bazref]]
+// CHECK: [[disposable:%[0-9a-z_]*]] = bitcast %struct.__block_byref_baz* [[baz]] to i8*
+// CHECK: call void @_Block_object_dispose(i8* [[disposable]]
+
+int main() {
+ __block int baz = [&]() { return 0; }();
+ return 0;
+}
Index: lib/CodeGen/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -26,6 +26,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclOpenMP.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/CodeGen/CGFunctionInfo.h"
@@ -1244,53 +1245,65 @@
return emission;
}
-/// Determines whether the given __block variable is potentially
-/// captured by the given expression.
-static bool isCapturedBy(const VarDecl &var, const Expr *e) {
- // Skip the most common kinds of expressions that make
- // hierarchy-walking expensive.
- e = e->IgnoreParenCasts();
+static bool isCapturedBy(const VarDecl &, const Expr *);
- if (const BlockExpr *be = dyn_cast<BlockExpr>(e)) {
- const BlockDecl *block = be->getBlockDecl();
- for (const auto &I : block->captures()) {
- if (I.getVariable() == &var)
- return true;
- }
+class IsCapturedBy : public RecursiveASTVisitor<IsCapturedBy> {
+ bool IsCaptured = false;
+ const VarDecl &Var;
+ bool setCaptured() {
+ IsCaptured = true;
+ // End visitation early, we know it's captured.
+ return false;
+ }
+public:
+ IsCapturedBy(const VarDecl &Var) : Var(Var) {}
+ bool isCaptured() const { return IsCaptured; }
+
+ bool TraverseBlockExpr(BlockExpr *BE) {
+ const BlockDecl *Block = BE->getBlockDecl();
+ for (const auto &I : Block->captures())
+ if (I.getVariable() == &Var)
+ return setCaptured();
// No need to walk into the subexpressions.
return false;
}
- if (const StmtExpr *SE = dyn_cast<StmtExpr>(e)) {
+ bool TraverseStmtExpr(StmtExpr *SE) {
const CompoundStmt *CS = SE->getSubStmt();
for (const auto *BI : CS->body())
if (const auto *E = dyn_cast<Expr>(BI)) {
- if (isCapturedBy(var, E))
- return true;
- }
- else if (const auto *DS = dyn_cast<DeclStmt>(BI)) {
- // special case declarations
- for (const auto *I : DS->decls()) {
- if (const auto *VD = dyn_cast<VarDecl>((I))) {
- const Expr *Init = VD->getInit();
- if (Init && isCapturedBy(var, Init))
- return true;
- }
+ if (isCapturedBy(Var, E))
+ return setCaptured();
+ } else if (const auto *DS = dyn_cast<DeclStmt>(BI)) {
+ // special case declarations
+ for (const auto *I : DS->decls()) {
+ if (const auto *VD = dyn_cast<VarDecl>((I))) {
+ const Expr *Init = VD->getInit();
+ if (Init && isCapturedBy(Var, Init))
+ return setCaptured();
}
+ }
}
else
- // FIXME. Make safe assumption assuming arbitrary statements cause capturing.
- // Later, provide code to poke into statements for capture analysis.
- return true;
- return false;
+ // FIXME. Make safe assumption assuming arbitrary statements cause
+ // capturing. Later, provide code to poke into statements for capture
+ // analysis.
+ return setCaptured();
+ return true;
}
+};
- for (const Stmt *SubStmt : e->children())
- if (isCapturedBy(var, cast<Expr>(SubStmt)))
- return true;
+/// Determines whether the given __block variable is potentially
+/// captured by the given expression.
+static bool isCapturedBy(const VarDecl &Var, const Expr *E) {
+ // Skip the most common kinds of expressions that make
+ // hierarchy-walking expensive.
+ E = E->IgnoreParenCasts();
- return false;
+ IsCapturedBy Visitor(Var);
+ Visitor.TraverseStmt(const_cast<Expr *>(E));
+ return Visitor.isCaptured();
}
/// Determine whether the given initializer is trivial in the sense
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
