[clang] [Clang] Permit noescape on non-pointer types (PR #117344)
Gábor Horváth via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 3 06:12:13 PST 2024
https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/117344
>From d48e28d43473b317a74bd8fe3b2f62e8f99ed3c8 Mon Sep 17 00:00:00 2001
From: Gabor Horvath <gaborh at apple.com>
Date: Fri, 22 Nov 2024 16:19:35 +0000
Subject: [PATCH] [Clang] Permit noescape on non-pointer types
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. 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.
---
clang/docs/ReleaseNotes.rst | 2 +
clang/include/clang/AST/Attr.h | 7 ++++
clang/include/clang/Basic/AttrDocs.td | 13 ++++---
.../clang/Basic/DiagnosticSemaKinds.td | 3 ++
clang/include/clang/Basic/Features.def | 1 +
clang/include/clang/Sema/Sema.h | 7 ----
clang/lib/AST/AttrImpl.cpp | 26 +++++++++++++
clang/lib/CodeGen/CGCall.cpp | 3 +-
clang/lib/Sema/SemaChecking.cpp | 2 +-
clang/lib/Sema/SemaDeclAttr.cpp | 37 +++----------------
clang/test/AST/ast-dump-attr.cpp | 8 +++-
clang/test/CodeGenCXX/noescape.cpp | 4 ++
clang/test/CodeGenObjC/noescape.m | 10 +++++
clang/test/SemaCXX/noescape-attr.cpp | 10 ++++-
clang/test/SemaObjCXX/noescape.mm | 6 +--
15 files changed, 87 insertions(+), 52 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e73640033f9774..9697395b1d9e4c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -512,6 +512,8 @@ Attribute Changes in Clang
- The ``target_version`` attribute is now only supported for AArch64 and RISC-V architectures.
+- Now ``__attribute__((noescape))`` can be applied to non-pointer types like ``std::span``.
+
Improvements to Clang's diagnostics
-----------------------------------
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 725498e132fc28..753d814ee83bb4 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -406,6 +406,13 @@ inline ParameterABI ParameterABIAttr::getABI() const {
llvm_unreachable("bad parameter ABI attribute kind");
}
}
+
+/// 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/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 5de39be4805600..2ecd883944d8a6 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -228,12 +228,13 @@ members, and static locals.
def NoEscapeDocs : Documentation {
let Category = DocCatVariable;
let Content = [{
-``noescape`` placed on a function parameter of a pointer type is used to inform
-the compiler that the pointer cannot escape: that is, no reference to the object
-the pointer points to that is derived from the parameter value will survive
-after the function returns. Users are responsible for making sure parameters
-annotated with ``noescape`` do not actually escape. Calling ``free()`` on such
-a parameter does not constitute an escape.
+``noescape`` placed on a function parameter is used to inform the compiler that
+the pointer (or the pointer members in case of a user defined type) cannot
+escape: that is, no reference to the object the pointer points to that is
+derived from the parameter value will survive after the function returns. Users
+are responsible for making sure parameters annotated with ``noescape`` do not
+actually escape. Calling ``free()`` on such a parameter does not constitute an
+escape.
For example:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8495884dcd058f..027173da098d02 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3346,6 +3346,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
index 9088c867d53ce4..5a5cd8f77311cd 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -87,6 +87,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
index b8684d11460eda..632bdcf66a33be 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4479,13 +4479,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. 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);
-
/// 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
index f198a9acf8481f..1e7dfa82b71704 100644
--- 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
index 28a5526fbea068..815706b72ca803 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2878,7 +2878,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
index a248a6b53b0d06..9342224b831282 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3110,7 +3110,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
index 4fd8ef6dbebf84..4d150b54b94cb9 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -1220,37 +1220,11 @@ static void handleNoSpecializations(Sema &S, Decl *D, const ParsedAttr &AL) {
NoSpecializationsAttr::Create(S.Context, Message, AL));
}
-bool Sema::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();
-}
-
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;
@@ -1291,7 +1265,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 (S.isValidPointerAttrType(T))
+ if (isValidPointerAttrType(T))
AnyPointers = true;
}
@@ -1339,10 +1313,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
index f5a74815714218..ba32f91a8daf65 100644
--- 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
index e6d7a727ebb368..3429c3254dec20 100644
--- 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 *, int __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 *, int __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
index 395f110251f3c3..caa53565f9aaab 100644
--- 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/SemaCXX/noescape-attr.cpp b/clang/test/SemaCXX/noescape-attr.cpp
index 78dc4f07ffef87..fd54e09d1118b1 100644
--- a/clang/test/SemaCXX/noescape-attr.cpp
+++ b/clang/test/SemaCXX/noescape-attr.cpp
@@ -3,5 +3,11 @@
template<typename T>
void test1(T __attribute__((noescape)) arr, int size);
-// expected-warning at +1 {{'noescape' attribute only applies to pointer arguments}}
-void test2(int __attribute__((noescape)) arr, int size);
\ No newline at end of file
+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
index 999a91b87300b7..74290daf174266 100644
--- a/clang/test/SemaObjCXX/noescape.mm
+++ b/clang/test/SemaObjCXX/noescape.mm
@@ -23,11 +23,11 @@
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 {
More information about the cfe-commits
mailing list