Skip to content

_Alignas on a struct declaration gives a low-quality diagnostic #58637

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.

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

Closed
AaronBallman opened this issue Oct 26, 2022 · 5 comments · Fixed by #65638
Closed

_Alignas on a struct declaration gives a low-quality diagnostic #58637

AaronBallman opened this issue Oct 26, 2022 · 5 comments · Fixed by #65638
Assignees
Labels
c11 clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer enhancement Improving things as opposed to bug fixing, e.g. new or missing feature good first issue https://github.com/llvm/llvm-project/contribute

Comments

@AaronBallman
Copy link
Collaborator

AaronBallman commented Oct 26, 2022

_Alignas(int) struct T {
  int i;
};

gives the diagnostic: warning: attribute '_Alignas' is ignored, place it after "struct" to apply attribute to type declaration which seems like a really helpful message at first glance. However, C does not allow you to put an _Alignas specifier on the struct type itself. So following that advice:

struct _Alignas(int) T {
  int i;
};

gives: error: declaration of anonymous struct must be a definition and warning: declaration does not declare anything.

The original diagnostic should say the attribute is ignored without the helpful message about placement (which is correct for C++).

@AaronBallman AaronBallman added enhancement Improving things as opposed to bug fixing, e.g. new or missing feature good first issue https://github.com/llvm/llvm-project/contribute c11 clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels Oct 26, 2022
@llvmbot
Copy link
Member

llvmbot commented Oct 26, 2022

@llvm/issue-subscribers-c11

@llvmbot
Copy link
Member

llvmbot commented Oct 26, 2022

@llvm/issue-subscribers-good-first-issue

@theo-lw
Copy link
Contributor

theo-lw commented Jan 7, 2023

Here's an attempt at a fix:

https://reviews.llvm.org/D141177

@Unique-Usman
Copy link
Contributor

@AaronBallman , I will like work on this issue, @theo-lw are you still working on this?

@jerinphilip
Copy link
Contributor

Hi, I'm trying to take a shot at this building on top of D141177 and its existing review. My active branch is here. I'm looking for some clarification w.r.t the solution before bringing in any review. Changes in bd41371 has D141177 outdated. I was expecting to use the replacements - something like isCXX11Attribute() && getLangOpts().C11 to identify the exception case and add code fixing the diagnostic.

Looks like I'm running into a few problems:

sample.c
#include <stdalign.h>

_Alignas(int) struct TLeft {
  int i;
};

#ifdef ENABLE_PLACE_AFTER
struct _Alignas(int) TRight { int i; };
#endif

int main() {
  struct TLeft t1 = {3};
#ifdef ENABLE_PLACE_AFTER
  TRight t1 = {4};
#endif
  return 0;
}
./bin/clang ../user-scripts/sample.c
--- <EmitAttributeDiagnostic> 
ParsedAttr.isCXX11Attribute() = 0
ParsedAttr.isInvalid() = 0
ParsedAttr.isTypeAttr() = 0
ParsedAttr.getForm().isAlignas() = 0
getLangOpts().C11 = 1
getLangOpts().C99 = 1
getLangOpts().C23 = 0
Firing normal /home/jerin/code/llvm-project/clang/lib/Sema/SemaDecl.cpp: 5366
../user-scripts/sample.c:3:1: warning: attribute '_Alignas' is ignored, place it after "struct" to apply attribute to type declaration [-Wignored-attributes]
    3 | _Alignas(int) struct TLeft {
      | ^
--- </EmitAttributeDiagnostic> 
./bin/clang++ ../user-scripts/sample.c -DENABLE_PLACE_AFTER=1
clang++: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Wdeprecated]
--- <EmitAttributeDiagnostic> 
ParsedAttr.isCXX11Attribute() = 0
ParsedAttr.isInvalid() = 0
ParsedAttr.isTypeAttr() = 0
ParsedAttr.getForm().isAlignas() = 0
getLangOpts().C11 = 0
getLangOpts().C99 = 0
getLangOpts().C23 = 0
Firing normal /home/jerin/code/llvm-project/clang/lib/Sema/SemaDecl.cpp: 5366
../user-scripts/sample.c:3:1: warning: attribute '_Alignas' is ignored, place it after "struct" to apply attribute to type declaration [-Wignored-attributes]
    3 | _Alignas(int) struct TLeft {
      | ^
--- </EmitAttributeDiagnostic> 
../user-scripts/sample.c:8:1: error: declaration of anonymous struct must be a definition
    8 | struct _Alignas(int) TRight { int i; };
      | ^
../user-scripts/sample.c:8:1: warning: declaration does not declare anything [-Wmissing-declarations]
../user-scripts/sample.c:14:3: error: unknown type name 'TRight'
   14 |   TRight t1 = {4};
      |   ^
2 warnings and 2 errors generated.

Not sure what's going on above, but isCXX11Attribute() and isAlignAs() are not returning true as I expect to make the solution work. Is this expected? Excuse my debugging like a caveman with printf, but I hope this suffice to explain.

AaronBallman pushed a commit that referenced this issue Nov 7, 2023
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.
qihangkong pushed a commit to rvgpu/llvm that referenced this issue Apr 18, 2024
Fixes llvm/llvm-project#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c11 clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer enhancement Improving things as opposed to bug fixing, e.g. new or missing feature good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants