[clang][NFC] Specify Type and ExtQuals as having 16-byte alignment

[Endilll]

While working on LLDB visualizer for QualType, I stumbled upon Type and ExtQuals defined with alignas(8). Such alignment leaves only 3 lower bits available for pointer tagging, whereas QualType requires 4 (3 qualifiers + discriminator between Type * and ExtQuals *). Turns out Type and its derived classes are allocated with TypeAlignment == 16 passed to Allocate(). So I'm removing misleading alignas(8) and fixing corresponding static asserts. Since they are already allocated with 16-byte alignment, this is a non-functional change.

[llvmbot]

@llvm/pr-subscribers-clang

Changes

While working on LLDB visualizer for QualType, I stumbled upon Type and ExtQuals defined with alignas(8). Such alignment leaves only 3 lower bits available for pointer tagging, whereas QualType requires 4 (3 qualifiers + discriminator between Type * and ExtQuals *). Turns out Type and its derived classes are allocated with TypeAlignment == 16 passed to Allocate(). So I'm removing misleading alignas(8) and fixing corresponding static asserts. Since they are already allocated with 16-byte alignment, this is a non-functional change.

---

Full diff: https://github.com/llvm/llvm-project/pull/68377.diff

1 Files Affected:

• (modified) clang/include/clang/AST/Type.h (+10-7)

diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index a78d8f60462b231..4e98858f6e9432a 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1482,7 +1482,7 @@ class ExtQualsTypeCommonBase {
 /// in three low bits on the QualType pointer; a fourth bit records whether
 /// the pointer is an ExtQuals node. The extended qualifiers (address spaces,
 /// Objective-C GC attributes) are much more rare.
-class ExtQuals : public ExtQualsTypeCommonBase, public llvm::FoldingSetNode {
+class alignas(TypeAlignment) ExtQuals : public ExtQualsTypeCommonBase, public llvm::FoldingSetNode {
   // NOTE: changing the fast qualifiers should be straightforward as
   // long as you don't make 'const' non-fast.
   // 1. Qualifiers:
@@ -1507,6 +1507,9 @@ class ExtQuals : public ExtQualsTypeCommonBase, public llvm::FoldingSetNode {
       : ExtQualsTypeCommonBase(baseType,
                                canon.isNull() ? QualType(this_(), 0) : canon),
         Quals(quals) {
+    static_assert(alignof(decltype(*this)) % TypeAlignment == 0,
+                  "Insufficient alignment!");
+
     assert(Quals.hasNonFastQualifiers()
            && "ExtQuals created with no fast qualifiers");
     assert(!Quals.hasFastQualifiers()
@@ -1594,7 +1597,7 @@ enum class AutoTypeKeyword {
 ///
 /// Types, once created, are immutable.
 ///
-class alignas(8) Type : public ExtQualsTypeCommonBase {
+class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
 public:
   enum TypeClass {
 #define TYPE(Class, Base) Class,
@@ -1982,9 +1985,9 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
   Type(TypeClass tc, QualType canon, TypeDependence Dependence)
       : ExtQualsTypeCommonBase(this,
                                canon.isNull() ? QualType(this_(), 0) : canon) {
-    static_assert(sizeof(*this) <= 8 + sizeof(ExtQualsTypeCommonBase),
+    static_assert(sizeof(*this) <= 16 + sizeof(ExtQualsTypeCommonBase),
                   "changing bitfields changed sizeof(Type)!");
-    static_assert(alignof(decltype(*this)) % sizeof(void *) == 0,
+    static_assert(alignof(decltype(*this)) % TypeAlignment == 0,
                   "Insufficient alignment!");
     TypeBits.TC = tc;
     TypeBits.Dependence = static_cast<unsigned>(Dependence);
@@ -5348,7 +5351,7 @@ class DeducedType : public Type {
 
 /// Represents a C++11 auto or C++14 decltype(auto) type, possibly constrained
 /// by a type-constraint.
-class alignas(8) AutoType : public DeducedType, public llvm::FoldingSetNode {
+class AutoType : public DeducedType, public llvm::FoldingSetNode {
   friend class ASTContext; // ASTContext creates these
 
   ConceptDecl *TypeConstraintConcept;
@@ -5456,7 +5459,7 @@ class DeducedTemplateSpecializationType : public DeducedType,
 /// TemplateArguments, followed by a QualType representing the
 /// non-canonical aliased type when the template is a type alias
 /// template.
-class alignas(8) TemplateSpecializationType
+class TemplateSpecializationType
     : public Type,
       public llvm::FoldingSetNode {
   friend class ASTContext; // ASTContext creates these
@@ -5872,7 +5875,7 @@ class DependentNameType : public TypeWithKeyword, public llvm::FoldingSetNode {
 /// Represents a template specialization type whose template cannot be
 /// resolved, e.g.
 ///   A<T>::template B<T>
-class alignas(8) DependentTemplateSpecializationType
+class DependentTemplateSpecializationType
     : public TypeWithKeyword,
       public llvm::FoldingSetNode {
   friend class ASTContext; // ASTContext creates these

[Endilll]

My plan is to phase-out TypeAlignment in subsequent patches, because we have proper language support for alignas and alignof now. It's the only one of its kind passed to Allocate() (I inspected all references to this function).

[Endilll]

CC @zygoloid as the author of ee0ce30

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Endilll]

Relevant code snippets:
https://github.com/llvm/llvm-project/blob/777a6e6f10b2b90496d248b7fa904fce834484be/clang/lib/AST/ASTContext.cpp#L5813C1-L5816C33

https://github.com/llvm/llvm-project/blob/777a6e6f10b2b90496d248b7fa904fce834484be/clang/lib/AST/ASTContext.cpp#L4997C1-L5002C70

https://github.com/llvm/llvm-project/blob/777a6e6f10b2b90496d248b7fa904fce834484be/clang/lib/AST/ASTContext.cpp#L5192C1-L5195C66

[cor3ntin]

Generally LGTM, i just have 1 small concern regarding allowing a bigger Type

[AaronBallman]

LGTM! If @zygoloid (or others) have concerns, we can revert and address them post-commit.