Skip to content

🍒[Clang] Permit noescape on non-pointer types #10735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2025
Merged

🍒[Clang] Permit noescape on non-pointer types #10735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions clang/include/clang/AST/Attr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,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
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3401,6 +3401,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<
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Basic/Features.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,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)
Expand Down
7 changes: 0 additions & 7 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5249,13 +5249,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,
Expand Down
26 changes: 26 additions & 0 deletions clang/lib/AST/AttrImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,30 @@ StringLiteral *FormatMatchesAttr::getFormatString() const {
return cast<StringLiteral>(getExpectedFormat());
}

bool clang::isValidPointerAttrType(QualType T, bool RefOkay) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should some of the changes like this also go to upstream clang as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, I think so! Gabor already started the upstreaming work in llvm#117344.

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"
3 changes: 2 additions & 1 deletion clang/lib/CodeGen/CGCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2971,7 +2971,8 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
break;
}

if (FI.getExtParameterInfo(ArgNo).isNoEscape())
if (FI.getExtParameterInfo(ArgNo).isNoEscape() &&
isValidPointerAttrType(ParamType, /*RefOkay=*/true))
Attrs.addCapturesAttr(llvm::CaptureInfo::none());

if (Attrs.hasAttributes()) {
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaChecking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3215,7 +3215,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;
}
Expand Down
37 changes: 6 additions & 31 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1231,37 +1231,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;
Expand Down Expand Up @@ -1312,7 +1286,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>()) {
Expand Down Expand Up @@ -1371,10 +1345,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;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the 0 comes from in the diagnostic. The previous diagnostic looks like this:

def warn_attribute_pointers_only : Warning<
  "%0 attribute only applies to%select{| constant}1 pointer arguments">,
  InGroup<IgnoredAttributes>;

So we likely want to do something like << T instead of << 0.

return;
}
Expand Down
8 changes: 7 additions & 1 deletion clang/test/AST/ast-dump-attr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions clang/test/CodeGenCXX/noescape.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {{.*}}, {{.*}}, {{.*}} noundef captures(none) {{%.*}})
Expand All @@ -23,6 +24,9 @@ void S::m0(int *, int * __attribute__((noescape))) {}
// CHECK: define{{.*}} void @_ZN1S3vm1EPiS0_(ptr {{.*}}, {{.*}} noundef captures(none) {{%.*}})
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 {{.*}}, {{.*}}, {{.*}} noundef captures(none) {{.*}})
// CHECK: call {{.*}} ptr @_ZN1SaSEPi(ptr {{.*}}, {{.*}} noundef captures(none) {{.*}})
Expand Down
10 changes: 10 additions & 0 deletions clang/test/CodeGenObjC/noescape.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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:]"({{.*}}, {{.*}}, {{.*}} captures(none) {{.*}})

// CHECK-LABEL: define{{.*}} void @test4(
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/attr-noescape.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)}}
}
10 changes: 8 additions & 2 deletions clang/test/SemaCXX/noescape-attr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,11 @@
template<typename T>
void test1(T __attribute__((noescape)) arr, int size);

// expected-warning@+1 {{'noescape' attribute only applies to pointer arguments}}
void test2(int __attribute__((noescape)) arr, int size);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is also the case in the original patch, but this diagnostic looks off to me. 0 is invalid doesn't really mean anything. We should fix this.

CC @Xazax-hun

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we could improve the phrasing. Let's wait until @Xazax-hun can work on it – this PR is just making the rebranch CI happy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I just re-read your comment again, and I now agree we should fix the 0 being printed instead of the actual type, let me put up a PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch up: #10747

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
6 changes: 3 additions & 3 deletions clang/test/SemaObjCXX/noescape.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down