ahatanak added inline comments.
================ Comment at: clang/lib/Sema/SemaExpr.cpp:5917 if (auto *BE = dyn_cast<BlockExpr>(Arg->IgnoreParenNoopCasts(Context))) BE->getBlockDecl()->setDoesNotEscape(); ---------------- rjmccall wrote: > ahatanak wrote: > > rjmccall wrote: > > > We need to be checking that the parameter type is a block pointer type. > > > A parameter of a type like `id` or `void*` does not have the enhanced > > > semantics of `noescape` for blocks. > > > > > > The inevitable weird C++ test case is: > > > > > > ``` > > > struct NoescapeCtor { > > > NoescapeCtor(__attribute__((noescape)) void (^)()); > > > }; > > > struct EscapeCtor { > > > EscapeCtor(void (^)()); > > > }; > > > > > > void helper1(NoescapeCtor a); > > > void test1() { helper1(^{}); } // <- should be noescape > > > > > > void helper2(NoescapeCtor &&a); > > > void test2() { helper2(^{}); } // <- should be noescape > > > > > > void helper3(__attribute__((noescape)) EscapeCtor &&a); > > > void test3() { helper3(^{}); } // <- should not be noescape > > > ``` > > > > > > You should probably also test that calls to function templates behave > > > according to the instantiated type of the parameter. I expect that that > > > should just fall out from this implementation, which I think only > > > triggers on non-dependent calls. > > I understand why the blocks should or shouldn't be `noescape` in the C++ > > example, but I'm not sure I understand the comment about `id` and `void*`. > > > > Do you mean the `DoesNotEscape` bit shouldn't be set in the following > > example? > > > > ``` > > void helper(__attribute__((noescape)) id); > > > > void test() { > > S s; > > helper(^{ (void)s; }); > > } > > ``` > The [noescape > documentation](https://clang.llvm.org/docs/AttributeReference.html#id357) > says: > > > Additionally, when the parameter is a block pointer, the same restriction > > applies to copies of the block. > > That's the restriction that makes the `noescape` block optimization sound, > and it doesn't apply when the parameter does not have block pointer type. So > yes, `DoesNotEscape` shouldn't be set in that example. Ah, I see. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101097/new/ https://reviews.llvm.org/D101097 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits