Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 3 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -4453,9 +4453,10 @@ class Sema final : public SemaBase {
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.
/// 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
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1216,6 +1216,8 @@ static void handlePreferredName(Sema &S, Decl *D, const ParsedAttr &AL) {
}

bool Sema::isValidPointerAttrType(QualType T, bool RefOkay) {
if (T->isDependentType())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably valuable to audit the other uses of this function well, and remove checks for dependence there as redundant. And probably ensure the documentation of this function mentions that it skips on dependence.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, please check the updated documentation of the function

return true;
if (RefOkay) {
if (T->isReferenceType())
return true;
Expand Down Expand Up @@ -1284,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 (S.isValidPointerAttrType(T))
AnyPointers = true;
}

Expand Down Expand Up @@ -1409,8 +1411,7 @@ void Sema::AddAllocAlignAttr(Decl *D, const AttributeCommonInfo &CI,
AllocAlignAttr TmpAttr(Context, CI, ParamIdx());
SourceLocation AttrLoc = CI.getLoc();

if (!ResultType->isDependentType() &&
!isValidPointerAttrType(ResultType, /* RefOkay */ true)) {
if (!isValidPointerAttrType(ResultType, /* RefOkay */ true)) {
Diag(AttrLoc, diag::warn_attribute_return_pointers_refs_only)
<< &TmpAttr << CI.getRange() << getFunctionOrMethodResultSourceRange(D);
return;
Expand Down
3 changes: 3 additions & 0 deletions clang/test/SemaCXX/alloc-align-attr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ void* dependent_param_func(T param) __attribute__((alloc_align(1)));// expected-
template <int T>
void* illegal_align_param(int p) __attribute__((alloc_align(T))); // expected-error {{'alloc_align' attribute requires parameter 1 to be an integer constant}}

template <typename T>
T dependent_return_type(int p) __attribute__((alloc_align(1)));

void dependent_impl(int align) {
dependent_ret<int> a; // expected-note {{in instantiation of template class 'dependent_ret<int>' requested here}}
a.Foo(1);
Expand Down
6 changes: 6 additions & 0 deletions clang/test/SemaCXX/builtin-assume-aligned-tmpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ T *atest3() __attribute__((assume_aligned(31, o))); // expected-error {{requeste
template <typename T, int o>
T *atest4() __attribute__((assume_aligned(32, o)));

template<typename T>
T atest5(int) __attribute__((assume_aligned(2)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change causes a number of attributes to change their behavior (see all uses of isValidPointerAttrType), so we should test each of those.

Additionally, we should separately check (via a different declaration) that the diagnostic fires when instantiated.


// expected-warning@+1 {{'assume_aligned' attribute only applies to return values that are pointers or references}}
int atest6(int) __attribute__((assume_aligned(2)));

void test22() {
atest3<int, 5>();
atest4<int, 5>();
Expand Down
7 changes: 7 additions & 0 deletions clang/test/SemaCXX/noescape-attr.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s

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);
7 changes: 3 additions & 4 deletions clang/test/SemaObjCXX/noescape.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
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>
Expand Down Expand Up @@ -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;
};

Expand All @@ -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;
Expand Down Expand Up @@ -199,7 +199,6 @@ void test1(C0 *c0) {
(void)b8;
});

// FIXME: clang shouldn't reject this.
noescapeFunc5(^{
(void)b9;
});
Expand Down
Loading