[Clang] Permit noescape on non-pointer types

Gabor Horvath · Gabor Horvath · commit e465c257ac16 · 2025-01-02T11:53:26.000+01:00
In modern C++ we often use span, string_view or other view objects instead of
raw pointers. This PR opens the door to mark those noescape. This can be useful
to document the API contracts, for interop with memory safe languages like
Swift or Rust, and possibly in the future to implement take this into account
in codegen.

The PR also adds a feature test macro. The goal is to help avoiding warnings
when the code is compiler by earlier versions of clang that does not permit
this attribute on non-pointer types.

rdar://140196481
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
@@ -380,6 +380,13 @@ inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
   DB.AddTaggedVal(reinterpret_cast<uint64_t>(At), DiagnosticsEngine::ak_attr);
   return DB;
 }
+
+/// Determine if type T is a valid subject for a nonnull and similar
+/// attributes. Dependent types are considered valid so they can be checked
+/// during instantiation time. By default, we look through references (the
+/// behavior used by nonnull), but if the second parameter is true, then we
+/// treat a reference type as valid.
+bool isValidPointerAttrType(QualType T, bool RefOkay = false);
 }  // end namespace clang
 
 #endif
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3355,6 +3355,9 @@ def warn_attribute_return_pointers_refs_only : Warning<
 def warn_attribute_pointer_or_reference_only : Warning<
   "%0 attribute only applies to a pointer or reference (%1 is invalid)">,
   InGroup<IgnoredAttributes>;
+def warn_attribute_pointer_or_reference_or_record_only : Warning<
+  "%0 attribute only applies to a pointer, reference, class, struct, or union (%1 is invalid)">,
+  InGroup<IgnoredAttributes>;
 def err_attribute_no_member_pointers : Error<
   "%0 attribute cannot be used with pointers to members">;
 def err_attribute_invalid_implicit_this_argument : Error<
diff --git a/clang/include/clang/Basic/Features.def b/clang/include/clang/Basic/Features.def
@@ -86,6 +86,7 @@ FEATURE(attribute_overloadable, true)
 FEATURE(attribute_unavailable_with_message, true)
 FEATURE(attribute_unused_on_fields, true)
 FEATURE(attribute_diagnose_if_objc, true)
+FEATURE(attribute_noescape_nonpointer, true)
 FEATURE(blocks, LangOpts.Blocks)
 FEATURE(c_thread_safety_attributes, true)
 FEATURE(cxx_exceptions, LangOpts.CXXExceptions)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -4663,12 +4663,6 @@ class Sema final : public SemaBase {
                                       StringRef &Str,
                                       SourceLocation *ArgLocation = nullptr);
 
-  /// Determine if type T is a valid subject for a nonnull and similar
-  /// attributes. By default, we look through references (the behavior used by
-  /// nonnull), but if the second parameter is true, then we treat a reference
-  /// type as valid.
-  bool isValidPointerAttrType(QualType T, bool RefOkay = false);
-
   /// AddAssumeAlignedAttr - Adds an assume_aligned attribute to a particular
   /// declaration.
   void AddAssumeAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
diff --git a/clang/lib/AST/AttrImpl.cpp b/clang/lib/AST/AttrImpl.cpp
@@ -270,4 +270,30 @@ unsigned AlignedAttr::getAlignment(ASTContext &Ctx) const {
   return Ctx.getTargetDefaultAlignForAttributeAligned();
 }
 
+bool clang::isValidPointerAttrType(QualType T, bool RefOkay) {
+  if (T->isDependentType())
+    return true;
+  if (RefOkay) {
+    if (T->isReferenceType())
+      return true;
+  } else {
+    T = T.getNonReferenceType();
+  }
+
+  // The nonnull attribute, and other similar attributes, can be applied to a
+  // transparent union that contains a pointer type.
+  if (const RecordType *UT = T->getAsUnionType()) {
+    if (UT && UT->getDecl()->hasAttr<TransparentUnionAttr>()) {
+      RecordDecl *UD = UT->getDecl();
+      for (const auto *I : UD->fields()) {
+        QualType QT = I->getType();
+        if (QT->isAnyPointerType() || QT->isBlockPointerType())
+          return true;
+      }
+    }
+  }
+
+  return T->isAnyPointerType() || T->isBlockPointerType();
+}
+
 #include "clang/AST/AttrImpl.inc"
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
@@ -2846,7 +2846,8 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
       break;
     }
 
-    if (FI.getExtParameterInfo(ArgNo).isNoEscape())
+    if (FI.getExtParameterInfo(ArgNo).isNoEscape() &&
+        isValidPointerAttrType(ParamType, /*RefOkay=*/true))
       Attrs.addAttribute(llvm::Attribute::NoCapture);
 
     if (Attrs.hasAttributes()) {
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
@@ -3045,7 +3045,7 @@ static void CheckNonNullArguments(Sema &S,
       if (!NonNull->args_size()) {
         // Easy case: all pointer arguments are nonnull.
         for (const auto *Arg : Args)
-          if (S.isValidPointerAttrType(Arg->getType()))
+          if (isValidPointerAttrType(Arg->getType()))
             CheckNonNullArgument(S, Arg, CallSiteLoc);
         return;
       }
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -1220,35 +1220,11 @@ static void handlePreferredName(Sema &S, Decl *D, const ParsedAttr &AL) {
         << TT->getDecl();
 }
 
-bool Sema::isValidPointerAttrType(QualType T, bool RefOkay) {
-  if (RefOkay) {
-    if (T->isReferenceType())
-      return true;
-  } else {
-    T = T.getNonReferenceType();
-  }
-
-  // The nonnull attribute, and other similar attributes, can be applied to a
-  // transparent union that contains a pointer type.
-  if (const RecordType *UT = T->getAsUnionType()) {
-    if (UT && UT->getDecl()->hasAttr<TransparentUnionAttr>()) {
-      RecordDecl *UD = UT->getDecl();
-      for (const auto *I : UD->fields()) {
-        QualType QT = I->getType();
-        if (QT->isAnyPointerType() || QT->isBlockPointerType())
-          return true;
-      }
-    }
-  }
-
-  return T->isAnyPointerType() || T->isBlockPointerType();
-}
-
 static bool attrNonNullArgCheck(Sema &S, QualType T, const ParsedAttr &AL,
                                 SourceRange AttrParmRange,
                                 SourceRange TypeRange,
                                 bool isReturnValue = false) {
-  if (!S.isValidPointerAttrType(T)) {
+  if (!isValidPointerAttrType(T)) {
     if (isReturnValue)
       S.Diag(AL.getLoc(), diag::warn_attribute_return_pointers_only)
           << AL << AttrParmRange << TypeRange;
@@ -1296,7 +1272,7 @@ static void handleNonNullAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     for (unsigned I = 0, E = getFunctionOrMethodNumParams(D);
          I != E && !AnyPointers; ++I) {
       QualType T = getFunctionOrMethodParamType(D, I);
-      if (T->isDependentType() || S.isValidPointerAttrType(T)) {
+      if (T->isDependentType() || isValidPointerAttrType(T)) {
         AnyPointers = true;
         /*TO_UPSTREAM(BoundsSafety) ON*/
         if (auto DCPTy = T->getAs<CountAttributedType>()) {
@@ -1355,10 +1331,11 @@ static void handleNoEscapeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (D->isInvalidDecl())
     return;
 
-  // noescape only applies to pointer types.
+  // noescape only applies to pointer and record types.
   QualType T = cast<ParmVarDecl>(D)->getType();
-  if (!S.isValidPointerAttrType(T, /* RefOkay */ true)) {
-    S.Diag(AL.getLoc(), diag::warn_attribute_pointers_only)
+  if (!isValidPointerAttrType(T, /* RefOkay */ true) && !T->isRecordType()) {
+    S.Diag(AL.getLoc(),
+           diag::warn_attribute_pointer_or_reference_or_record_only)
         << AL << AL.getRange() << 0;
     return;
   }
diff --git a/clang/test/AST/ast-dump-attr.cpp b/clang/test/AST/ast-dump-attr.cpp
@@ -230,8 +230,14 @@ __attribute__((external_source_symbol(generated_declaration, defined_in="module"
 // CHECK-NEXT: ExternalSourceSymbolAttr{{.*}} "Swift" "module" GeneratedDeclaration "testUSR"
 
 namespace TestNoEscape {
+  struct S { int *p; };
   void noescapeFunc(int *p0, __attribute__((noescape)) int *p1) {}
-  // CHECK: `-FunctionDecl{{.*}} noescapeFunc 'void (int *, __attribute__((noescape)) int *)'
+  // CHECK: |-FunctionDecl{{.*}} noescapeFunc 'void (int *, __attribute__((noescape)) int *)'
+  // CHECK-NEXT: ParmVarDecl
+  // CHECK-NEXT: ParmVarDecl
+  // CHECK-NEXT: NoEscapeAttr
+  void noescapeFunc2(int *p0, __attribute__((noescape)) S p1) {}
+  // CHECK: `-FunctionDecl{{.*}} noescapeFunc2 'void (int *, __attribute__((noescape)) S)'
   // CHECK-NEXT: ParmVarDecl
   // CHECK-NEXT: ParmVarDecl
   // CHECK-NEXT: NoEscapeAttr
diff --git a/clang/test/CodeGenCXX/noescape.cpp b/clang/test/CodeGenCXX/noescape.cpp
@@ -6,6 +6,7 @@ struct S {
   S &operator=(int * __attribute__((noescape)));
   void m0(int *, int * __attribute__((noescape)));
   virtual void vm1(int *, int * __attribute__((noescape)));
+  virtual void vm2(int *, S __attribute__((noescape)));
 };
 
 // CHECK: define{{.*}} void @_ZN1SC2EPiS0_(ptr {{.*}}, {{.*}}, {{.*}} nocapture noundef {{%.*}})
@@ -23,6 +24,9 @@ void S::m0(int *, int * __attribute__((noescape))) {}
 // CHECK: define{{.*}} void @_ZN1S3vm1EPiS0_(ptr {{.*}}, {{.*}} nocapture noundef {{%.*}})
 void S::vm1(int *, int * __attribute__((noescape))) {}
 
+// CHECK-NOT: nocapture
+void S::vm2(int *, S __attribute__((noescape))) {}
+
 // CHECK-LABEL: define{{.*}} void @_Z5test0P1SPiS1_(
 // CHECK: call void @_ZN1SC1EPiS0_(ptr {{.*}}, {{.*}}, {{.*}} nocapture noundef {{.*}})
 // CHECK: call {{.*}} ptr @_ZN1SaSEPi(ptr {{.*}}, {{.*}} nocapture noundef {{.*}})
diff --git a/clang/test/CodeGenObjC/noescape.m b/clang/test/CodeGenObjC/noescape.m
@@ -8,11 +8,16 @@
   long long *ll;
 } __attribute__((transparent_union));
 
+struct S {
+  int *p;
+};
+
 void escapingFunc0(BlockTy);
 void noescapeFunc0(id, __attribute__((noescape)) BlockTy);
 void noescapeFunc1(__attribute__((noescape)) int *);
 void noescapeFunc2(__attribute__((noescape)) id);
 void noescapeFunc3(__attribute__((noescape)) union U);
+void noescapeFunc4(__attribute__((noescape)) struct S s);
 
 // Block descriptors of non-escaping blocks don't need pointers to copy/dispose
 // helper functions.
@@ -53,6 +58,11 @@ void test3(union U u) {
   noescapeFunc3(u);
 }
 
+// CHECK-NOT: nocapture
+void testNonPtr(struct S s) {
+  noescapeFunc4(s);
+}
+
 // CHECK: define internal void @"\01-[C0 m0:]"({{.*}}, {{.*}}, {{.*}} nocapture {{.*}})
 
 // CHECK-LABEL: define{{.*}} void @test4(
diff --git a/clang/test/Sema/attr-noescape.c b/clang/test/Sema/attr-noescape.c
@@ -8,5 +8,5 @@ int *global_var __attribute((noescape)); // expected-warning{{'noescape' attribu
 
 void foo(__attribute__((noescape)) int *int_ptr,
          __attribute__((noescape)) int (^block)(int),
-         __attribute((noescape)) int integer) { // expected-warning{{'noescape' attribute only applies to pointer arguments}}
+         __attribute((noescape)) int integer) { // expected-warning{{'noescape' attribute only applies to a pointer, reference, class, struct, or union (0 is invalid)}}
 }
diff --git a/clang/test/SemaCXX/noescape-attr.cpp b/clang/test/SemaCXX/noescape-attr.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template<typename T>
+void test1(T __attribute__((noescape)) arr, int size);
+
+void test2(int __attribute__((noescape)) a, int b); // expected-warning {{'noescape' attribute only applies to a pointer, reference, class, struct, or union (0 is invalid)}}
+
+struct S { int *p; };
+void test3(S __attribute__((noescape)) s);
+
+#if !__has_feature(attribute_noescape_nonpointer)
+  #error "attribute_noescape_nonpointer should be supported"
+#endif
diff --git a/clang/test/SemaObjCXX/noescape.mm b/clang/test/SemaObjCXX/noescape.mm
@@ -17,17 +17,17 @@
 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}}
+void noescapeFunc5(__attribute__((noescape)) T);
 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 invalidFunc0(int __attribute__((noescape))); // expected-warning {{'noescape' attribute only applies to a pointer, reference, class, struct, or union (0 is invalid)}}
 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}}
+void invalidFunc3(__attribute__((noescape)) int (S::*Ty)); // expected-warning {{'noescape' attribute only applies to a pointer, reference, class, struct, or union (0 is invalid)}}
+void invalidFunc4(__attribute__((noescape)) void (S::*Ty)()); // expected-warning {{'noescape' attribute only applies to a pointer, reference, class, struct, or union (0 is invalid)}}
 int __attribute__((noescape)) g; // expected-warning {{'noescape' attribute only applies to parameters}}
 
 struct S1 {
@@ -144,7 +144,7 @@ -(void) m1:(int*) p {
 
 struct S6 {
   S6();
-  S6(const S6 &) = delete; // expected-note 12 {{'S6' has been explicitly marked deleted here}}
+  S6(const S6 &) = delete; // expected-note 11 {{'S6' has been explicitly marked deleted here}}
   int f;
 };
 
@@ -161,7 +161,7 @@ void test1(C0 *c0) {
   __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 b9;
   __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;
@@ -199,7 +199,6 @@ void test1(C0 *c0) {
     (void)b8;
   });
 
-  // FIXME: clang shouldn't reject this.
   noescapeFunc5(^{
     (void)b9;
   });

Original file line number	Diff line number	Diff line change
`@@ -2846,7 +2846,8 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,`
`2846`	`2846`	`break;`
`2847`	`2847`	`}`
`2848`	`2848`
`2849`		`- if (FI.getExtParameterInfo(ArgNo).isNoEscape())`
	`2849`	`+ if (FI.getExtParameterInfo(ArgNo).isNoEscape() &&`
	`2850`	`+ isValidPointerAttrType(ParamType, /RefOkay=/true))`
`2850`	`2851`	`Attrs.addAttribute(llvm::Attribute::NoCapture);`
`2851`	`2852`
`2852`	`2853`	`if (Attrs.hasAttributes()) {`
Original file line number	Diff line number	Diff line change
`@@ -3045,7 +3045,7 @@ static void CheckNonNullArguments(Sema &S,`
`3045`	`3045`	`if (!NonNull->args_size()) {`
`3046`	`3046`	`// Easy case: all pointer arguments are nonnull.`
`3047`	`3047`	`for (const auto *Arg : Args)`
`3048`		`- if (S.isValidPointerAttrType(Arg->getType()))`
	`3048`	`+ if (isValidPointerAttrType(Arg->getType()))`
`3049`	`3049`	`CheckNonNullArgument(S, Arg, CallSiteLoc);`
`3050`	`3050`	`return;`
`3051`	`3051`	`}`