Merge pull request swiftlang#9789 from swiftlang/gaborh/noescape-nonptr

Xazax-hun · web-flow · commit 06a77c5cc0ff · 2025-01-02T19:56:41.000+01:00
[Clang] Permit noescape on non-pointer types
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`	`}`