[Sema] Don't set BlockDecl's DoesNotEscape bit If the block is being passed to a function taking a reference parameter

[ahatanak]

Summary

In the following example, the block passed to the function is marked as noescape, which is incorrect as the noescape attribute applies to the reference:

typedef void (^BlockTy)();
 __block S6 b5;
void noescapeFuncRefParam1(__attribute__((noescape)) BlockTy &&);
noescapeFuncRefParam1(^{ (void)b5; });

This partially fixes PR50043.

rdar://77030453

[rjmccall]

clang/lib/Sema/SemaExpr.cpp

5917

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.

[ahatanak]

clang/lib/Sema/SemaExpr.cpp

5917

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; });
}

[rjmccall]

clang/lib/Sema/SemaExpr.cpp

5917

The noescape documentation 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.

[ahatanak]

clang/lib/Sema/SemaExpr.cpp

5917

Ah, I see.

[ahatanak]

Check that the parameter has a block pointer type. Add more test cases.

[ahatanak]

clang/test/SemaObjCXX/noescape.mm

20

I didn't realize clang rejects noescape on parameters of template functions. I think this should be fixed in a followup patch.

[rjmccall]

LGTM

clang/lib/Sema/SemaExpr.cpp

if (ArgIx < Args.size()) {
           FDecl && FDecl->hasAttr<CFAuditedTransferAttr>() &&
           (!Param || !Param->hasAttr<CFConsumedAttr>()))
         Arg = stripARCUnbridgedCast(Arg);
       else if (getLangOpts().ObjCAutoRefCount &&
                FDecl && FDecl->hasAttr<CFAuditedTransferAttr>() &&
                (!Param || !Param->hasAttr<CFConsumedAttr>()))
         CFAudited = true;
 
-      if (Proto->getExtParameterInfo(i).isNoEscape())
+      if (Proto->getExtParameterInfo(i).isNoEscape() &&
+          ProtoArgType->isBlockPointerType())
         if (auto *BE = dyn_cast<BlockExpr>(Arg->IgnoreParenNoopCasts(Context)))
           BE->getBlockDecl()->setDoesNotEscape();
 
       InitializedEntity Entity =
           Param ? InitializedEntity::InitializeParameter(Context, Param,
                                                          ProtoArgType)
                 : InitializedEntity::InitializeParameter(
                       Context, ProtoArgType, Proto->isParamConsumed(i));
 
       // Remember that parameter belongs to a CF audited API.

clang/lib/Sema/SemaExprObjC.cpp

for (unsigned i = 0; i < NumNamedArgs; i++) {
     if (Args[i]->isTypeDependent())
       continue;
 
     Expr *argExpr = Args[i];
 
     ParmVarDecl *param = Method->parameters()[i];
     assert(argExpr && "CheckMessageArgumentTypes(): missing expression");
 
-    if (param->hasAttr<NoEscapeAttr>())
+    if (param->hasAttr<NoEscapeAttr>() &&
+        param->getType()->isBlockPointerType())
       if (auto *BE = dyn_cast<BlockExpr>(
               argExpr->IgnoreParenNoopCasts(Context)))
         BE->getBlockDecl()->setDoesNotEscape();
 
     // Strip the unbridged-cast placeholder expression off unless it's
     // a consumed argument.
     if (argExpr->hasPlaceholderType(BuiltinType::ARCUnbridgedCast) &&
         !param->hasAttr<CFConsumedAttr>())

clang/test/SemaObjCXX/noescape.mm

 void escapingFunc0(BlockTy);
 void noescapeFunc0(id, __attribute__((noescape)) BlockTy);
 void noescapeFunc1(id, [[clang::noescape]] BlockTy);
 void noescapeFunc2(__attribute__((noescape)) int *); // expected-note {{previous declaration is here}}
 void noescapeFunc3(__attribute__((noescape)) id);
 void noescapeFunc4(__attribute__((noescape)) int &);
 void noescapeFunc2(int *); // expected-error {{conflicting types for 'noescapeFunc2'}}
 
+template <class T>
+void noescapeFunc5(__attribute__((noescape)) T); // expected-warning {{'noescape' attribute only applies to pointer arguments}}
+template <class T>
+void noescapeFunc6(__attribute__((noescape)) const T &);
+template <class T>
+void noescapeFunc7(__attribute__((noescape)) T &&);
+
 void invalidFunc0(int __attribute__((noescape))); // expected-warning {{'noescape' attribute only applies to pointer arguments}}
 void invalidFunc1(int __attribute__((noescape(0)))); // expected-error {{'noescape' attribute takes no arguments}}
 void invalidFunc2(int0 *__attribute__((noescape))); // expected-error {{use of undeclared identifier 'int0'; did you mean 'int'?}}
 void invalidFunc3(__attribute__((noescape)) int (S::*Ty)); // expected-warning {{'noescape' attribute only applies to pointer arguments}}
 void invalidFunc4(__attribute__((noescape)) void (S::*Ty)()); // expected-warning {{'noescape' attribute only applies to pointer arguments}}
 int __attribute__((noescape)) g; // expected-warning {{'noescape' attribute only applies to parameters}}
 
 struct S1 {
   virtual void m0(int *__attribute__((noescape))); // expected-note {{parameter of overridden method is annotated with __attribute__((noescape))}}
 };
 
 struct S2 : S1 {
   void m0(int *__attribute__((noescape))) override;
 };
 
 struct S3 : S1 {
   void m0(int *) override; // expected-warning {{parameter of overriding method should be annotated with __attribute__((noescape))}}
 };
 
 __attribute__((objc_root_class))
 @interface C0
 -(void) m0:(int*)__attribute__((noescape)) p; // expected-note {{parameter of overridden method is annotated with __attribute__((noescape))}}
+- (void)noescapeLValRefParam:(const BlockTy &)__attribute__((noescape))p;
+- (void)noescapeRValRefParam:(BlockTy &&)__attribute__((noescape))p;
 @end
 
 @implementation C0
 -(void) m0:(int*)__attribute__((noescape)) p {}
+- (void)noescapeLValRefParam:(const BlockTy &)__attribute__((noescape))p {
+}
+- (void)noescapeRValRefParam:(BlockTy &&)__attribute__((noescape))p {
+}
 @end
 
 @interface C1 : C0
 -(void) m0:(int*)__attribute__((noescape)) p;
 @end
 
 @implementation C1 : C0
 -(void) m0:(int*)__attribute__((noescape)) p {}

 +(void) m1:(int*)__attribute__((noescape)) p {
 }
 -(void) m1:(int*) p {
 }
 @end
 
 struct S6 {
   S6();
-  S6(const S6 &) = delete; // expected-note 3 {{'S6' has been explicitly marked deleted here}}
+  S6(const S6 &) = delete; // expected-note 12 {{'S6' has been explicitly marked deleted here}}
   int f;
 };
 
-void test1() {
+void test1(C0 *c0) {
   id a;
   // __block variables that are not captured by escaping blocks don't
   // necessitate having accessible copy constructors.
   __block S6 b0;
   __block S6 b1; // expected-error {{call to deleted constructor of 'S6'}}
   __block S6 b2; // expected-error {{call to deleted constructor of 'S6'}}
   __block S6 b3; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b4; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b5; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b6; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b7; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b8; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b9; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b10; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b11; // expected-error {{call to deleted constructor of 'S6'}}
+  __block S6 b12;
+  __block S6 b13;
+  __block S6 b14; // expected-error {{call to deleted constructor of 'S6'}}
 
   noescapeFunc0(a, ^{ (void)b0; });
   escapingFunc0(^{ (void)b1; });
   {
     noescapeFunc0(a, ^{ (void)b0; (void)b1; });
   }
   noescapeFunc0(a, ^{ escapingFunc0(^{ (void)b2; }); });
   escapingFunc0(^{ noescapeFunc0(a, ^{ (void)b3; }); });
+
+  [c0 noescapeLValRefParam:^{
+    (void)b4;
+  }];
+
+  [c0 noescapeRValRefParam:^{
+    (void)b5;
+  }];
+
+  void noescape_id(__attribute__((noescape)) id);
+  noescape_id(^{
+    (void)b6;
+  });
+
+  void noescapeLValRefParam(__attribute__((noescape)) const BlockTy &);
+  noescapeLValRefParam(^{
+    (void)b7;
+  });
+
+  void noescapeRValRefParam(__attribute__((noescape)) BlockTy &&);
+  noescapeRValRefParam(^{
+    (void)b8;
+  });
+
+  // FIXME: clang shouldn't reject this.
+  noescapeFunc5(^{
+    (void)b9;
+  });
+
+  noescapeFunc6(^{
+    (void)b10;
+  });
+
+  noescapeFunc7(^{
+    (void)b11;
+  });
+
+  struct NoescapeCtor {
+    NoescapeCtor(__attribute__((noescape)) void (^)());
+  };
+  struct EscapeCtor {
+    EscapeCtor(void (^)());
+  };
+
+  void helper1(NoescapeCtor a);
+  helper1(^{
+    (void)b12;
+  });
+
+  void helper2(NoescapeCtor && a);
+  helper2(^{
+    (void)b13;
+  });
+
+  void helper3(__attribute__((noescape)) EscapeCtor && a);
+  helper3(^{
+    (void)b14;
+  });
 }