Skip to content

Commit

Permalink
[clang] Improve _Alignas on a struct declaration diagnostic (#65638)
Browse files Browse the repository at this point in the history
Fixes #58637. 

Adds `isAlignas()` method on `AttributeCommonInfo` which accounts for
C++ `alignas` as well as C11 `_Alignas`.

The method is used to improve diagnostic in C when `_Alignas` is used in
C at the wrong location.  This corrects the previously suggested move
of `_Alignas` past the declaration specifier, now warns attribute
`_Alignas` is ignored.

Based on https://reviews.llvm.org/D141177.
  • Loading branch information
jerinphilip committed Nov 7, 2023
1 parent a0303d8 commit 2e79133
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 14 deletions.
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ Attribute Changes in Clang
When viewing ``S::FruitKind`` in a debugger, it will behave as if the member
was declared as type ``E`` rather than ``unsigned``.

- Clang now warns you that the ``_Alignas`` attribute on declaration specifiers
is ignored, changed from the former incorrect suggestion to move it past
declaration specifiers. (`#58637 <https://github.com/llvm/llvm-project/issues/58637>`_)


Improvements to Clang's diagnostics
-----------------------------------
- Clang constexpr evaluator now prints template arguments when displaying
Expand Down
9 changes: 9 additions & 0 deletions clang/include/clang/Basic/AttributeCommonInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,15 @@ class AttributeCommonInfo {

bool isC23Attribute() const { return SyntaxUsed == AS_C23; }

bool isAlignas() const {
// FIXME: In the current state, the IsAlignas member variable is only true
// with the C++ `alignas` keyword but not `_Alignas`. The following
// expression works around the otherwise lost information so it will return
// true for `alignas` or `_Alignas` while still returning false for things
// like `__attribute__((aligned))`.
return (getParsedKind() == AT_Aligned && isKeywordAttribute());
}

/// The attribute is spelled [[]] in either C or C++ mode, including standard
/// attributes spelled with a keyword, like alignas.
bool isStandardAttributeSyntax() const {
Expand Down
23 changes: 14 additions & 9 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5346,16 +5346,21 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
TypeSpecType == DeclSpec::TST_interface ||
TypeSpecType == DeclSpec::TST_union ||
TypeSpecType == DeclSpec::TST_enum) {
for (const ParsedAttr &AL : DS.getAttributes())
Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
? diag::err_declspec_keyword_has_no_effect
: diag::warn_declspec_attribute_ignored)
<< AL << GetDiagnosticTypeSpecifierID(DS);
for (const ParsedAttr &AL : DeclAttrs)
Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
? diag::err_declspec_keyword_has_no_effect
: diag::warn_declspec_attribute_ignored)

auto EmitAttributeDiagnostic = [this, &DS](const ParsedAttr &AL) {
unsigned DiagnosticId = diag::warn_declspec_attribute_ignored;
if (AL.isAlignas() && !getLangOpts().CPlusPlus)
DiagnosticId = diag::warn_attribute_ignored;
else if (AL.isRegularKeywordAttribute())
DiagnosticId = diag::err_declspec_keyword_has_no_effect;
else
DiagnosticId = diag::warn_declspec_attribute_ignored;
Diag(AL.getLoc(), DiagnosticId)
<< AL << GetDiagnosticTypeSpecifierID(DS);
};

llvm::for_each(DS.getAttributes(), EmitAttributeDiagnostic);
llvm::for_each(DeclAttrs, EmitAttributeDiagnostic);
}
}

Expand Down
6 changes: 1 addition & 5 deletions clang/test/C/drs/dr4xx.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,7 @@ void dr444(void) {
/* FIXME: This should be accepted as per this DR. */
int j = (_Alignas(int) int){12}; /* expected-error {{expected expression}} */

/* FIXME: The diagnostic in this case is really bad; moving the specifier to
* where the diagnostic recommends causes a different, more inscrutable error
* about anonymous structures.
*/
_Alignas(int) struct T { /* expected-warning {{attribute '_Alignas' is ignored, place it after "struct" to apply attribute to type declaration}} */
_Alignas(int) struct T { /* expected-warning {{'_Alignas' attribute ignored}} */
int i;
};

Expand Down
1 change: 1 addition & 0 deletions clang/test/Parser/c1x-alignas.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ char c4 _Alignas(32); // expected-error {{expected ';' after top level declarato

char _Alignas(_Alignof(int)) c5;

_Alignas(int) struct c6; // expected-warning {{'_Alignas' attribute ignored}}
// CHECK-EXT: '_Alignas' is a C11 extension
// CHECK-EXT: '_Alignof' is a C11 extension
11 changes: 11 additions & 0 deletions clang/test/Parser/c2x-alignas.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// RUN: %clang_cc1 -std=c23 -fsyntax-only -verify %s

_Alignas(int) struct c1; // expected-warning {{'_Alignas' attribute ignored}}

// FIXME: `alignas` enters into C++ parsing code and never reaches the
// declaration specifier attribute diagnostic infrastructure.
//
// Fixing this will require the C23 notions of `alignas` being a keyword and
// `_Alignas` being an alternate spelling integrated into the parsing
// infrastructure.
alignas(int) struct c1; // expected-error {{misplaced attributes; expected attributes here}}
2 changes: 2 additions & 0 deletions clang/test/Parser/cxx0x-attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,3 +451,5 @@ namespace P2361 {
// expected-warning {{use of the 'deprecated' attribute is a C++14 extension}}
[[nodiscard("\123")]] int b(); // expected-error{{invalid escape sequence '\123' in an unevaluated string literal}}
}

alignas(int) struct AlignAsAttribute {}; // expected-error {{misplaced attributes; expected attributes here}}

0 comments on commit 2e79133

Please sign in to comment.