This is an archive of the discontinued LLVM Phabricator instance.

[Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas
ClosedPublic

Authored by yronglin on May 14 2023, 8:24 AM.

Details

Summary
  • Fix diagnoses when the argument to alignas or _Alignas is an incomplete type.

Before:

./alignas.cpp:1:15: error: invalid application of 'alignof' to an incomplete type 'void'
class alignas(void) Foo {};
             ~^~~~~
1 error generated.

Now:

./alignas.cpp:1:15: error: invalid application of 'alignas' to an incomplete type 'void'
class alignas(void) Foo {};
             ~^~~~~
1 error generated.
  • Improve the AST fidelity of alignas and _Alignas attribute.

Before:

AlignedAttr 0x13f07f278 <col:7> alignas
    `-ConstantExpr 0x13f07f258 <col:15, col:21> 'unsigned long'
      |-value: Int 8
      `-UnaryExprOrTypeTraitExpr 0x13f07f118 <col:15, col:21> 'unsigned long' alignof 'void *'

Now:

AlignedAttr 0x14288c608 <col:7> alignas 'void *'

Diff Detail

Event Timeline

yronglin created this revision.May 14 2023, 8:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 14 2023, 8:24 AM
yronglin requested review of this revision.May 14 2023, 8:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 14 2023, 8:24 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
yronglin updated this revision to Diff 522008.May 14 2023, 8:27 AM

Update ReleaseNotes

yronglin edited the summary of this revision. (Show Details)May 14 2023, 10:00 AM

Thank you for working on this! Just a few minor things.

clang/docs/ReleaseNotes.rst
297–301
clang/lib/Parse/ParseDecl.cpp
3039
clang/lib/Sema/SemaExpr.cpp
4715

It looks like a zero-width joiner was added here around the ! that should be removed.

4721
yronglin updated this revision to Diff 522201.May 15 2023, 8:15 AM

Address comments.

yronglin marked 3 inline comments as done.EditedMay 15 2023, 8:20 AM

Thanks a lot for your review @aaron.ballman ! BTW, does the most good approach is that we just add a new ParsedAttribute with TypeSourceInfo but not an UnaryExprOrTypeTraitExpr, and only do a sema check same as alignof(type-id)?

clang/docs/ReleaseNotes.rst
297–301

+1, and add _Alignas

clang/lib/Sema/SemaExpr.cpp
4715

Thanks, removed.

Thanks a lot for your review @aaron.ballman ! BTW, does the most good approach is that we just add a new ParsedAttribute with TypeSourceInfo but not an UnaryExprOrTypeTraitExpr, and only do a sema check same as alignof(type-id)?

Because you've already got a ParsedAttr, I think you'd use this constructor: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/ParsedAttr.h#L274 and move the call to GetTypeFromParser() until later, but it should still be modelled as a UnaryExprOrTypeTraitExpr.

yronglin marked 2 inline comments as done.May 15 2023, 8:56 AM

Thanks a lot for your review @aaron.ballman ! BTW, does the most good approach is that we just add a new ParsedAttribute with TypeSourceInfo but not an UnaryExprOrTypeTraitExpr, and only do a sema check same as alignof(type-id)?

Because you've already got a ParsedAttr, I think you'd use this constructor: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/ParsedAttr.h#L274 and move the call to GetTypeFromParser() until later, but it should still be modelled as a UnaryExprOrTypeTraitExpr.

Many thanks for the tips, I think the next step is to build UnaryExprOrTypeTraitExpr in this place (https://github.com/llvm/llvm-project/blob/9715af434579022b5ef31781be40b722d7e63bee/clang/lib/Sema/SemaDeclAttr.cpp#L4335-L4358) and then calculate the alignment? If this approach is correct, I'm glad to investigate it.

Thanks a lot for your review @aaron.ballman ! BTW, does the most good approach is that we just add a new ParsedAttribute with TypeSourceInfo but not an UnaryExprOrTypeTraitExpr, and only do a sema check same as alignof(type-id)?

Because you've already got a ParsedAttr, I think you'd use this constructor: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/ParsedAttr.h#L274 and move the call to GetTypeFromParser() until later, but it should still be modelled as a UnaryExprOrTypeTraitExpr.

Many thanks for the tips, I think the next step is to build UnaryExprOrTypeTraitExpr in this place (https://github.com/llvm/llvm-project/blob/9715af434579022b5ef31781be40b722d7e63bee/clang/lib/Sema/SemaDeclAttr.cpp#L4335-L4358) and then calculate the alignment? If this approach is correct, I'm glad to investigate it.

Hmm, not quite. Roping in @erichkeane in case he's got other ideas, but I think this will be more involved. What I think needs to happen is:

  • We need to add another argument to Aligned in Attr.td so that the attribute takes either an expression argument or a type argument.
  • We need to update SemaDeclAttr.cpp and other places accordingly
  • We need to add an overload of Sema::AddAlignedAttr() that takes a ParsedType and call this overload when given alignas(type)
  • We need to add code to TreeTransform.h to instantiate a dependent type so we can support template <typename Ty> alignas(Ty) int foo = 12;

Your approach of creating a UnaryExprOrTypeTraitExpr would still end up having us model as alignas(alignof(type)) because the argument to the alignas attribute would be the type trait.

Thanks a lot for your review @aaron.ballman ! BTW, does the most good approach is that we just add a new ParsedAttribute with TypeSourceInfo but not an UnaryExprOrTypeTraitExpr, and only do a sema check same as alignof(type-id)?

Because you've already got a ParsedAttr, I think you'd use this constructor: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/ParsedAttr.h#L274 and move the call to GetTypeFromParser() until later, but it should still be modelled as a UnaryExprOrTypeTraitExpr.

Many thanks for the tips, I think the next step is to build UnaryExprOrTypeTraitExpr in this place (https://github.com/llvm/llvm-project/blob/9715af434579022b5ef31781be40b722d7e63bee/clang/lib/Sema/SemaDeclAttr.cpp#L4335-L4358) and then calculate the alignment? If this approach is correct, I'm glad to investigate it.

Hmm, not quite. Roping in @erichkeane in case he's got other ideas, but I think this will be more involved. What I think needs to happen is:

  • We need to add another argument to Aligned in Attr.td so that the attribute takes either an expression argument or a type argument.
  • We need to update SemaDeclAttr.cpp and other places accordingly
  • We need to add an overload of Sema::AddAlignedAttr() that takes a ParsedType and call this overload when given alignas(type)
  • We need to add code to TreeTransform.h to instantiate a dependent type so we can support template <typename Ty> alignas(Ty) int foo = 12;

Your approach of creating a UnaryExprOrTypeTraitExpr would still end up having us model as alignas(alignof(type)) because the argument to the alignas attribute would be the type trait.

Sorry for the late reply and thanks for your tips! @aaron.ballman I followed your advice to update this patch, and below AST generated by clang:

class alignas(void *) Foo {};
_Alignas(void*) char align32array[128];

template <typename T> class alignas(T) Bar {};
template <unsigned N> class alignas(N) FooBar {};

int main() {
  Bar<int> bar;
  FooBar<4> fooBar;
  return 0;
}
➜  test ../rel/bin/clang++ -cc1 -triple x86_64-pc-linux -ast-dump ./alignas.cpp -std=c++11
 alignas _Alignas alignas alignasTranslationUnitDecl 0x14b043208 <<invalid sloc>> <invalid sloc>
|-TypedefDecl 0x14b043a70 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'
| `-BuiltinType 0x14b0437d0 '__int128'
|-TypedefDecl 0x14b043ae0 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'
| `-BuiltinType 0x14b0437f0 'unsigned __int128'
|-TypedefDecl 0x14b043e58 <<invalid sloc>> <invalid sloc> implicit __NSConstantString '__NSConstantString_tag'
| `-RecordType 0x14b043bd0 '__NSConstantString_tag'
|   `-CXXRecord 0x14b043b38 '__NSConstantString_tag'
|-TypedefDecl 0x14b043ef0 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'
| `-PointerType 0x14b043eb0 'char *'
|   `-BuiltinType 0x14b0432b0 'char'
|-TypedefDecl 0x14b087858 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list '__va_list_tag[1]'
| `-ConstantArrayType 0x14b087800 '__va_list_tag[1]' 1 
|   `-RecordType 0x14b043fe0 '__va_list_tag'
|     `-CXXRecord 0x14b043f48 '__va_list_tag'
|-CXXRecordDecl 0x14b0878e8 <./alignas.cpp:1:1, col:28> col:23 class Foo definition
| |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveAssignment exists simple trivial needs_implicit
| | `-Destructor simple irrelevant trivial needs_implicit
| |-AlignedAttr 0x14b087a08 <col:7> alignas 'void *'
| `-CXXRecordDecl 0x14b087a68 <col:1, col:23> col:23 implicit class Foo
|-VarDecl 0x14b087be8 <line:2:1, col:38> col:22 align32array 'char[128]'
| `-AlignedAttr 0x14b087c50 <col:1> _Alignas 'void *'
|-ClassTemplateDecl 0x14b087e80 <line:4:1, col:45> col:40 Bar
| |-TemplateTypeParmDecl 0x14b087cf8 <col:11, col:20> col:20 referenced typename depth 0 index 0 T
| |-CXXRecordDecl 0x14b087df0 <col:23, col:45> col:40 class Bar definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-AlignedAttr 0x14b0880d0 <col:29> alignas 'T'
| | `-CXXRecordDecl 0x14b088130 <col:23, col:40> col:40 implicit class Bar
| `-ClassTemplateSpecializationDecl 0x14b09e200 <col:1, col:45> col:40 class Bar definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param implicit_has_const_param
|   | |-MoveConstructor exists simple trivial
|   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument type 'int'
|   | `-BuiltinType 0x14b043310 'int'
|   |-AlignedAttr 0x14b09e458 <col:29> alignas 'int':'int'
|   |-CXXRecordDecl 0x14b09e520 <col:23, col:40> col:40 implicit class Bar
|   |-CXXConstructorDecl 0x14b09e5e0 <col:40> col:40 implicit used constexpr Bar 'void () noexcept' inline default trivial
|   | `-CompoundStmt 0x14b09eb48 <col:40>
|   |-CXXConstructorDecl 0x14b09e778 <col:40> col:40 implicit constexpr Bar 'void (const Bar<int> &)' inline default trivial noexcept-unevaluated 0x14b09e778
|   | `-ParmVarDecl 0x14b09e898 <col:40> col:40 'const Bar<int> &'
|   `-CXXConstructorDecl 0x14b09e978 <col:40> col:40 implicit constexpr Bar 'void (Bar<int> &&)' inline default trivial noexcept-unevaluated 0x14b09e978
|     `-ParmVarDecl 0x14b09ea98 <col:40> col:40 'Bar<int> &&'
|-ClassTemplateDecl 0x14b088310 <line:5:1, col:48> col:40 FooBar
| |-NonTypeTemplateParmDecl 0x14b0881f0 <col:11, col:20> col:20 referenced 'unsigned int' depth 0 index 0 N
| |-CXXRecordDecl 0x14b088280 <col:23, col:48> col:40 class FooBar definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-AlignedAttr 0x14b088580 <col:29> alignas
| | | `-DeclRefExpr 0x14b088260 <col:37> 'unsigned int' NonTypeTemplateParm 0x14b0881f0 'N' 'unsigned int'
| | `-CXXRecordDecl 0x14b0885e0 <col:23, col:40> col:40 implicit class FooBar
| `-ClassTemplateSpecializationDecl 0x14b09ed10 <col:1, col:48> col:40 class FooBar definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param implicit_has_const_param
|   | |-MoveConstructor exists simple trivial
|   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument integral 4
|   |-AlignedAttr 0x14b09ef90 <col:29> alignas
|   | `-ConstantExpr 0x14b09ef70 <col:37> 'unsigned int'
|   |   |-value: Int 4
|   |   `-SubstNonTypeTemplateParmExpr 0x14b09ef48 <col:37> 'unsigned int'
|   |     |-NonTypeTemplateParmDecl 0x14b0881f0 <col:11, col:20> col:20 referenced 'unsigned int' depth 0 index 0 N
|   |     `-IntegerLiteral 0x14b09ef28 <col:37> 'unsigned int' 4
|   |-CXXRecordDecl 0x14b09f058 <col:23, col:40> col:40 implicit class FooBar
|   |-CXXConstructorDecl 0x14980fc00 <col:40> col:40 implicit used constexpr FooBar 'void () noexcept' inline default trivial
|   | `-CompoundStmt 0x149810108 <col:40>
|   |-CXXConstructorDecl 0x14980fd68 <col:40> col:40 implicit constexpr FooBar 'void (const FooBar<4> &)' inline default trivial noexcept-unevaluated 0x14980fd68
|   | `-ParmVarDecl 0x14980fe88 <col:40> col:40 'const FooBar<4> &'
|   `-CXXConstructorDecl 0x14980ff68 <col:40> col:40 implicit constexpr FooBar 'void (FooBar<4> &&)' inline default trivial noexcept-unevaluated 0x14980ff68
|     `-ParmVarDecl 0x149810088 <col:40> col:40 'FooBar<4> &&'
`-FunctionDecl 0x14b0886e0 <line:7:1, line:11:1> line:7:5 main 'int ()'
  `-CompoundStmt 0x1498101e8 <col:12, line:11:1>
    |-DeclStmt 0x14b09eca0 <line:8:3, col:15>
    | `-VarDecl 0x14b09e3b0 <col:3, col:12> col:12 bar 'Bar<int>':'Bar<int>' callinit
    |   `-CXXConstructExpr 0x14b09ec78 <col:12> 'Bar<int>':'Bar<int>' 'void () noexcept'
    |-DeclStmt 0x1498101a0 <line:9:3, col:19>
    | `-VarDecl 0x14b09eec0 <col:3, col:13> col:13 fooBar 'FooBar<4>':'FooBar<4>' callinit
    |   `-CXXConstructExpr 0x149810178 <col:13> 'FooBar<4>':'FooBar<4>' 'void () noexcept'
    `-ReturnStmt 0x1498101d8 <line:10:3, col:10>
      `-IntegerLiteral 0x1498101b8 <col:10> 'int' 0

This AST dose not have a UnaryExprOrTypeTraitExpr in alignas, What do you think?

Sorry for the late reply and thanks for your tips! @aaron.ballman I followed your advice to update this patch, and below AST generated by clang:

class alignas(void *) Foo {};
_Alignas(void*) char align32array[128];

template <typename T> class alignas(T) Bar {};
template <unsigned N> class alignas(N) FooBar {};

int main() {
  Bar<int> bar;
  FooBar<4> fooBar;
  return 0;
}
➜  test ../rel/bin/clang++ -cc1 -triple x86_64-pc-linux -ast-dump ./alignas.cpp -std=c++11
 alignas _Alignas alignas alignasTranslationUnitDecl 0x14b043208 <<invalid sloc>> <invalid sloc>
|-TypedefDecl 0x14b043a70 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'
| `-BuiltinType 0x14b0437d0 '__int128'
|-TypedefDecl 0x14b043ae0 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'
| `-BuiltinType 0x14b0437f0 'unsigned __int128'
|-TypedefDecl 0x14b043e58 <<invalid sloc>> <invalid sloc> implicit __NSConstantString '__NSConstantString_tag'
| `-RecordType 0x14b043bd0 '__NSConstantString_tag'
|   `-CXXRecord 0x14b043b38 '__NSConstantString_tag'
|-TypedefDecl 0x14b043ef0 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'
| `-PointerType 0x14b043eb0 'char *'
|   `-BuiltinType 0x14b0432b0 'char'
|-TypedefDecl 0x14b087858 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list '__va_list_tag[1]'
| `-ConstantArrayType 0x14b087800 '__va_list_tag[1]' 1 
|   `-RecordType 0x14b043fe0 '__va_list_tag'
|     `-CXXRecord 0x14b043f48 '__va_list_tag'
|-CXXRecordDecl 0x14b0878e8 <./alignas.cpp:1:1, col:28> col:23 class Foo definition
| |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveAssignment exists simple trivial needs_implicit
| | `-Destructor simple irrelevant trivial needs_implicit
| |-AlignedAttr 0x14b087a08 <col:7> alignas 'void *'
| `-CXXRecordDecl 0x14b087a68 <col:1, col:23> col:23 implicit class Foo
|-VarDecl 0x14b087be8 <line:2:1, col:38> col:22 align32array 'char[128]'
| `-AlignedAttr 0x14b087c50 <col:1> _Alignas 'void *'
|-ClassTemplateDecl 0x14b087e80 <line:4:1, col:45> col:40 Bar
| |-TemplateTypeParmDecl 0x14b087cf8 <col:11, col:20> col:20 referenced typename depth 0 index 0 T
| |-CXXRecordDecl 0x14b087df0 <col:23, col:45> col:40 class Bar definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-AlignedAttr 0x14b0880d0 <col:29> alignas 'T'
| | `-CXXRecordDecl 0x14b088130 <col:23, col:40> col:40 implicit class Bar
| `-ClassTemplateSpecializationDecl 0x14b09e200 <col:1, col:45> col:40 class Bar definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param implicit_has_const_param
|   | |-MoveConstructor exists simple trivial
|   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument type 'int'
|   | `-BuiltinType 0x14b043310 'int'
|   |-AlignedAttr 0x14b09e458 <col:29> alignas 'int':'int'
|   |-CXXRecordDecl 0x14b09e520 <col:23, col:40> col:40 implicit class Bar
|   |-CXXConstructorDecl 0x14b09e5e0 <col:40> col:40 implicit used constexpr Bar 'void () noexcept' inline default trivial
|   | `-CompoundStmt 0x14b09eb48 <col:40>
|   |-CXXConstructorDecl 0x14b09e778 <col:40> col:40 implicit constexpr Bar 'void (const Bar<int> &)' inline default trivial noexcept-unevaluated 0x14b09e778
|   | `-ParmVarDecl 0x14b09e898 <col:40> col:40 'const Bar<int> &'
|   `-CXXConstructorDecl 0x14b09e978 <col:40> col:40 implicit constexpr Bar 'void (Bar<int> &&)' inline default trivial noexcept-unevaluated 0x14b09e978
|     `-ParmVarDecl 0x14b09ea98 <col:40> col:40 'Bar<int> &&'
|-ClassTemplateDecl 0x14b088310 <line:5:1, col:48> col:40 FooBar
| |-NonTypeTemplateParmDecl 0x14b0881f0 <col:11, col:20> col:20 referenced 'unsigned int' depth 0 index 0 N
| |-CXXRecordDecl 0x14b088280 <col:23, col:48> col:40 class FooBar definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-AlignedAttr 0x14b088580 <col:29> alignas
| | | `-DeclRefExpr 0x14b088260 <col:37> 'unsigned int' NonTypeTemplateParm 0x14b0881f0 'N' 'unsigned int'
| | `-CXXRecordDecl 0x14b0885e0 <col:23, col:40> col:40 implicit class FooBar
| `-ClassTemplateSpecializationDecl 0x14b09ed10 <col:1, col:48> col:40 class FooBar definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param implicit_has_const_param
|   | |-MoveConstructor exists simple trivial
|   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument integral 4
|   |-AlignedAttr 0x14b09ef90 <col:29> alignas
|   | `-ConstantExpr 0x14b09ef70 <col:37> 'unsigned int'
|   |   |-value: Int 4
|   |   `-SubstNonTypeTemplateParmExpr 0x14b09ef48 <col:37> 'unsigned int'
|   |     |-NonTypeTemplateParmDecl 0x14b0881f0 <col:11, col:20> col:20 referenced 'unsigned int' depth 0 index 0 N
|   |     `-IntegerLiteral 0x14b09ef28 <col:37> 'unsigned int' 4
|   |-CXXRecordDecl 0x14b09f058 <col:23, col:40> col:40 implicit class FooBar
|   |-CXXConstructorDecl 0x14980fc00 <col:40> col:40 implicit used constexpr FooBar 'void () noexcept' inline default trivial
|   | `-CompoundStmt 0x149810108 <col:40>
|   |-CXXConstructorDecl 0x14980fd68 <col:40> col:40 implicit constexpr FooBar 'void (const FooBar<4> &)' inline default trivial noexcept-unevaluated 0x14980fd68
|   | `-ParmVarDecl 0x14980fe88 <col:40> col:40 'const FooBar<4> &'
|   `-CXXConstructorDecl 0x14980ff68 <col:40> col:40 implicit constexpr FooBar 'void (FooBar<4> &&)' inline default trivial noexcept-unevaluated 0x14980ff68
|     `-ParmVarDecl 0x149810088 <col:40> col:40 'FooBar<4> &&'
`-FunctionDecl 0x14b0886e0 <line:7:1, line:11:1> line:7:5 main 'int ()'
  `-CompoundStmt 0x1498101e8 <col:12, line:11:1>
    |-DeclStmt 0x14b09eca0 <line:8:3, col:15>
    | `-VarDecl 0x14b09e3b0 <col:3, col:12> col:12 bar 'Bar<int>':'Bar<int>' callinit
    |   `-CXXConstructExpr 0x14b09ec78 <col:12> 'Bar<int>':'Bar<int>' 'void () noexcept'
    |-DeclStmt 0x1498101a0 <line:9:3, col:19>
    | `-VarDecl 0x14b09eec0 <col:3, col:13> col:13 fooBar 'FooBar<4>':'FooBar<4>' callinit
    |   `-CXXConstructExpr 0x149810178 <col:13> 'FooBar<4>':'FooBar<4>' 'void () noexcept'
    `-ReturnStmt 0x1498101d8 <line:10:3, col:10>
      `-IntegerLiteral 0x1498101b8 <col:10> 'int' 0

This AST dose not have a UnaryExprOrTypeTraitExpr in alignas, What do you think?

That is exactly what I would hope the AST would look like -- it's great! The next step would be to add CodeGen tests to validate that the correct alignment is emitted to LLVM IR with the type-base form.

Sorry for the late reply and thanks for your tips! @aaron.ballman I followed your advice to update this patch, and below AST generated by clang:

class alignas(void *) Foo {};
_Alignas(void*) char align32array[128];

template <typename T> class alignas(T) Bar {};
template <unsigned N> class alignas(N) FooBar {};

int main() {
  Bar<int> bar;
  FooBar<4> fooBar;
  return 0;
}
➜  test ../rel/bin/clang++ -cc1 -triple x86_64-pc-linux -ast-dump ./alignas.cpp -std=c++11
 alignas _Alignas alignas alignasTranslationUnitDecl 0x14b043208 <<invalid sloc>> <invalid sloc>
|-TypedefDecl 0x14b043a70 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'
| `-BuiltinType 0x14b0437d0 '__int128'
|-TypedefDecl 0x14b043ae0 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'
| `-BuiltinType 0x14b0437f0 'unsigned __int128'
|-TypedefDecl 0x14b043e58 <<invalid sloc>> <invalid sloc> implicit __NSConstantString '__NSConstantString_tag'
| `-RecordType 0x14b043bd0 '__NSConstantString_tag'
|   `-CXXRecord 0x14b043b38 '__NSConstantString_tag'
|-TypedefDecl 0x14b043ef0 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'
| `-PointerType 0x14b043eb0 'char *'
|   `-BuiltinType 0x14b0432b0 'char'
|-TypedefDecl 0x14b087858 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list '__va_list_tag[1]'
| `-ConstantArrayType 0x14b087800 '__va_list_tag[1]' 1 
|   `-RecordType 0x14b043fe0 '__va_list_tag'
|     `-CXXRecord 0x14b043f48 '__va_list_tag'
|-CXXRecordDecl 0x14b0878e8 <./alignas.cpp:1:1, col:28> col:23 class Foo definition
| |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveAssignment exists simple trivial needs_implicit
| | `-Destructor simple irrelevant trivial needs_implicit
| |-AlignedAttr 0x14b087a08 <col:7> alignas 'void *'
| `-CXXRecordDecl 0x14b087a68 <col:1, col:23> col:23 implicit class Foo
|-VarDecl 0x14b087be8 <line:2:1, col:38> col:22 align32array 'char[128]'
| `-AlignedAttr 0x14b087c50 <col:1> _Alignas 'void *'
|-ClassTemplateDecl 0x14b087e80 <line:4:1, col:45> col:40 Bar
| |-TemplateTypeParmDecl 0x14b087cf8 <col:11, col:20> col:20 referenced typename depth 0 index 0 T
| |-CXXRecordDecl 0x14b087df0 <col:23, col:45> col:40 class Bar definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-AlignedAttr 0x14b0880d0 <col:29> alignas 'T'
| | `-CXXRecordDecl 0x14b088130 <col:23, col:40> col:40 implicit class Bar
| `-ClassTemplateSpecializationDecl 0x14b09e200 <col:1, col:45> col:40 class Bar definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param implicit_has_const_param
|   | |-MoveConstructor exists simple trivial
|   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument type 'int'
|   | `-BuiltinType 0x14b043310 'int'
|   |-AlignedAttr 0x14b09e458 <col:29> alignas 'int':'int'
|   |-CXXRecordDecl 0x14b09e520 <col:23, col:40> col:40 implicit class Bar
|   |-CXXConstructorDecl 0x14b09e5e0 <col:40> col:40 implicit used constexpr Bar 'void () noexcept' inline default trivial
|   | `-CompoundStmt 0x14b09eb48 <col:40>
|   |-CXXConstructorDecl 0x14b09e778 <col:40> col:40 implicit constexpr Bar 'void (const Bar<int> &)' inline default trivial noexcept-unevaluated 0x14b09e778
|   | `-ParmVarDecl 0x14b09e898 <col:40> col:40 'const Bar<int> &'
|   `-CXXConstructorDecl 0x14b09e978 <col:40> col:40 implicit constexpr Bar 'void (Bar<int> &&)' inline default trivial noexcept-unevaluated 0x14b09e978
|     `-ParmVarDecl 0x14b09ea98 <col:40> col:40 'Bar<int> &&'
|-ClassTemplateDecl 0x14b088310 <line:5:1, col:48> col:40 FooBar
| |-NonTypeTemplateParmDecl 0x14b0881f0 <col:11, col:20> col:20 referenced 'unsigned int' depth 0 index 0 N
| |-CXXRecordDecl 0x14b088280 <col:23, col:48> col:40 class FooBar definition
| | |-DefinitionData empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-AlignedAttr 0x14b088580 <col:29> alignas
| | | `-DeclRefExpr 0x14b088260 <col:37> 'unsigned int' NonTypeTemplateParm 0x14b0881f0 'N' 'unsigned int'
| | `-CXXRecordDecl 0x14b0885e0 <col:23, col:40> col:40 implicit class FooBar
| `-ClassTemplateSpecializationDecl 0x14b09ed10 <col:1, col:48> col:40 class FooBar definition
|   |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
|   | |-DefaultConstructor exists trivial constexpr defaulted_is_constexpr
|   | |-CopyConstructor simple trivial has_const_param implicit_has_const_param
|   | |-MoveConstructor exists simple trivial
|   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
|   | |-MoveAssignment exists simple trivial needs_implicit
|   | `-Destructor simple irrelevant trivial needs_implicit
|   |-TemplateArgument integral 4
|   |-AlignedAttr 0x14b09ef90 <col:29> alignas
|   | `-ConstantExpr 0x14b09ef70 <col:37> 'unsigned int'
|   |   |-value: Int 4
|   |   `-SubstNonTypeTemplateParmExpr 0x14b09ef48 <col:37> 'unsigned int'
|   |     |-NonTypeTemplateParmDecl 0x14b0881f0 <col:11, col:20> col:20 referenced 'unsigned int' depth 0 index 0 N
|   |     `-IntegerLiteral 0x14b09ef28 <col:37> 'unsigned int' 4
|   |-CXXRecordDecl 0x14b09f058 <col:23, col:40> col:40 implicit class FooBar
|   |-CXXConstructorDecl 0x14980fc00 <col:40> col:40 implicit used constexpr FooBar 'void () noexcept' inline default trivial
|   | `-CompoundStmt 0x149810108 <col:40>
|   |-CXXConstructorDecl 0x14980fd68 <col:40> col:40 implicit constexpr FooBar 'void (const FooBar<4> &)' inline default trivial noexcept-unevaluated 0x14980fd68
|   | `-ParmVarDecl 0x14980fe88 <col:40> col:40 'const FooBar<4> &'
|   `-CXXConstructorDecl 0x14980ff68 <col:40> col:40 implicit constexpr FooBar 'void (FooBar<4> &&)' inline default trivial noexcept-unevaluated 0x14980ff68
|     `-ParmVarDecl 0x149810088 <col:40> col:40 'FooBar<4> &&'
`-FunctionDecl 0x14b0886e0 <line:7:1, line:11:1> line:7:5 main 'int ()'
  `-CompoundStmt 0x1498101e8 <col:12, line:11:1>
    |-DeclStmt 0x14b09eca0 <line:8:3, col:15>
    | `-VarDecl 0x14b09e3b0 <col:3, col:12> col:12 bar 'Bar<int>':'Bar<int>' callinit
    |   `-CXXConstructExpr 0x14b09ec78 <col:12> 'Bar<int>':'Bar<int>' 'void () noexcept'
    |-DeclStmt 0x1498101a0 <line:9:3, col:19>
    | `-VarDecl 0x14b09eec0 <col:3, col:13> col:13 fooBar 'FooBar<4>':'FooBar<4>' callinit
    |   `-CXXConstructExpr 0x149810178 <col:13> 'FooBar<4>':'FooBar<4>' 'void () noexcept'
    `-ReturnStmt 0x1498101d8 <line:10:3, col:10>
      `-IntegerLiteral 0x1498101b8 <col:10> 'int' 0

This AST dose not have a UnaryExprOrTypeTraitExpr in alignas, What do you think?

That is exactly what I would hope the AST would look like -- it's great! The next step would be to add CodeGen tests to validate that the correct alignment is emitted to LLVM IR with the type-base form.

Thanks, I'll do that!

yronglin updated this revision to Diff 524106.May 21 2023, 8:23 AM

Improve the AST fidelity of `alignas and _Alignas` attribute.

yronglin updated this revision to Diff 524107.May 21 2023, 8:26 AM

Remove extra include.

yronglin updated this revision to Diff 524108.May 21 2023, 8:50 AM

Test asm dump.

yronglin retitled this revision from [Clang] Fix the diagnoses when the argument to alignas is an incomplete type to [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas.May 21 2023, 9:04 AM
yronglin edited the summary of this revision. (Show Details)
yronglin updated this revision to Diff 524158.May 21 2023, 10:06 PM

Fix crash.

yronglin edited the summary of this revision. (Show Details)May 22 2023, 7:03 AM
yronglin edited the summary of this revision. (Show Details)

In general, I think this is looking pretty good, thank you! I'll leave it to @erichkeane to do the final sign-off as attributes code owner since this is making a fair number of changes in that area.

clang/lib/Sema/SemaDeclAttr.cpp
4415–4419
4422–4424

In general, I think this is looking pretty good, thank you! I'll leave it to @erichkeane to do the final sign-off as attributes code owner since this is making a fair number of changes in that area.

Thanks for your review @aaron.ballman ! BTW, I have two questions:

  1. Should we print the cached alignment value when dump AST?
  2. The AlignedAttr::getAlignment has a bit redundant with GetAlignOfType(in ExprConstant.cpp ), should we merge these?

In general, I think this is looking pretty good, thank you! I'll leave it to @erichkeane to do the final sign-off as attributes code owner since this is making a fair number of changes in that area.

Thanks for your review @aaron.ballman ! BTW, I have two questions:

  1. Should we print the cached alignment value when dump AST?

I think it would be nice to dump that as well, but not critical.

  1. The AlignedAttr::getAlignment has a bit redundant with GetAlignOfType(in ExprConstant.cpp ), should we merge these?

It would be good to unify them, but that can be done in a follow-up as it's a bit orthogonal.

yronglin updated this revision to Diff 525224.May 24 2023, 9:29 AM

Address comments.

yronglin marked 2 inline comments as done.May 24 2023, 9:31 AM

Thanks, maybe I can do these trivially change in separate patch.

Just 1 change I'd like, but otherwise this LGTM.

clang/lib/AST/AttrImpl.cpp
244

I don't think it makes sense to make this a lambda here rather than just implementing it inline if the value isn't cached.

yronglin updated this revision to Diff 525614.May 25 2023, 7:49 AM

Address comment.

yronglin marked an inline comment as done.May 25 2023, 7:50 AM

Thanks for your comments, @erichkeane !

erichkeane accepted this revision.May 25 2023, 7:53 AM

1 nit I don't care either way about, 1 suggestion.

clang/lib/AST/AttrImpl.cpp
247

Typically we don't do curleys on a single-statement... but I am on the fence due to how complicated it is below.

259

You can replace this 'if' with: T = T->getNonReferenceType();

This revision is now accepted and ready to land.May 25 2023, 7:53 AM
yronglin updated this revision to Diff 525636.May 25 2023, 8:36 AM

Address comments

yronglin marked 2 inline comments as done.May 25 2023, 8:39 AM
yronglin added inline comments.
clang/lib/AST/AttrImpl.cpp
247

I've updated this function, What do you think?

259

+1, both update GetAlignOfType in ExprConstant.cpp

Still LGTM, thanks!

clang/lib/AST/AttrImpl.cpp
247

Much nicer, thanks!

yronglin marked 2 inline comments as done.EditedMay 25 2023, 4:37 PM

The libcxx CI failure seems not caused bye this patch, there have a patch to fix this issue. https://reviews.llvm.org/D151508

This revision was landed with ongoing or failed builds.May 25 2023, 4:42 PM
This revision was automatically updated to reflect the committed changes.