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

Reply via email to