[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread JF Bastien via Phabricator via cfe-commits
jfb closed this revision. jfb added a comment. r332801 Repository: rC Clang https://reviews.llvm.org/D47096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In https://reviews.llvm.org/D47096#1105492, @rjmccall wrote: > Test case should go in test/CodeGenCXX. Also, there already is a > `blocks.cpp` there. I moved it, but didn't merge with the existing block.cpp because it just checks for not crashing. I'd rather make sure

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 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. LGTM. Repository: rC Clang https://reviews.llvm.org/D47096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 147648. jfb added a comment. - move test Repository: rC Clang https://reviews.llvm.org/D47096 Files: lib/CodeGen/CGDecl.cpp test/CodeGenCXX/block-capture.cpp Index: test/CodeGenCXX/block-capture.cpp

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Test case should go in test/CodeGenCXX. Also, there already is a `blocks.cpp` there. Repository: rC Clang https://reviews.llvm.org/D47096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In https://reviews.llvm.org/D47096#1105455, @rjmccall wrote: > `children()` is actually defined at the `Stmt` level, and if you look at how > it's implemented on e.g. `IfStmt`, you can see that it visits all of the > child `Stmt`s, including the if-condition. So it should

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 147647. jfb added a comment. - Follow John's suggestion. Repository: rC Clang https://reviews.llvm.org/D47096 Files: lib/CodeGen/CGDecl.cpp test/CodeGen/block-capture.cpp Index: test/CodeGen/block-capture.cpp

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D47096#1105374, @jfb wrote: > In https://reviews.llvm.org/D47096#1105368, @rjmccall wrote: > > > RecursiveASTVisitor instantiations are huge. Can you just make the > > function take a Stmt and then do the first few checks if it happens to

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In https://reviews.llvm.org/D47096#1105368, @rjmccall wrote: > RecursiveASTVisitor instantiations are huge. Can you just make the function > take a Stmt and then do the first few checks if it happens to be an Expr? I'm not super-familiar with the code, so I might be

[PATCH] D47096: CodeGen: block capture shouldn't ICE

2018-05-18 Thread JF Bastien via Phabricator via cfe-commits
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