Skip to content
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

[clang][NFC] Refactor enums that hold size of Type and DeclContext bit-fields #70296

Merged
merged 2 commits into from Oct 26, 2023

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Oct 26, 2023

This patch refactor said enums to hold total size of a bit-field, and not just non-inherited bits. This brings Type and DeclContext in line with Comment and Stmt. It also makes it unnecessary to list all transitive bases of a bit-field as unnamed bit-fields, which makes it more friendly towards debuggers.

@Endilll Endilll added clang:frontend Language frontend issues, e.g. anything involving "Sema" code-quality labels Oct 26, 2023
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Oct 26, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 26, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Vlad Serebrennikov (Endilll)

Changes

This patch refactor said enums to hold total size of a bit-field, and not just non-inherited bits. This brings Type and DeclContext in line with Comment and Stmt. It also makes it unnecessary to list all transitive bases of a bit-field as unnamed bit-fields, which makes it more friendly towards debuggers.


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

3 Files Affected:

  • (modified) clang/include/clang/AST/DeclBase.h (+21-28)
  • (modified) clang/include/clang/AST/Type.h (+3-3)
  • (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+9-9)
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index d383e46e22e16f4..e9eb43900cf5371 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -1515,16 +1515,14 @@ class DeclContext {
     uint64_t IsThisDeclarationADemotedDefinition : 1;
   };
 
-  /// Number of non-inherited bits in TagDeclBitfields.
-  enum { NumTagDeclBits = 10 };
+  /// Number of inherited and non-inherited bits in TagDeclBitfields.
+  enum { NumTagDeclBits = NumDeclContextBits + 10 };
 
   /// Stores the bits used by EnumDecl.
   /// If modified NumEnumDeclBit and the accessor
   /// methods in EnumDecl should be updated appropriately.
   class EnumDeclBitfields {
     friend class EnumDecl;
-    /// For the bits in DeclContextBitfields.
-    uint64_t : NumDeclContextBits;
     /// For the bits in TagDeclBitfields.
     uint64_t : NumTagDeclBits;
 
@@ -1554,16 +1552,14 @@ class DeclContext {
     uint64_t HasODRHash : 1;
   };
 
-  /// Number of non-inherited bits in EnumDeclBitfields.
-  enum { NumEnumDeclBits = 20 };
+  /// Number of inherited and non-inherited bits in EnumDeclBitfields.
+  enum { NumEnumDeclBits = NumTagDeclBits + 20 };
 
   /// Stores the bits used by RecordDecl.
   /// If modified NumRecordDeclBits and the accessor
   /// methods in RecordDecl should be updated appropriately.
   class RecordDeclBitfields {
     friend class RecordDecl;
-    /// For the bits in DeclContextBitfields.
-    uint64_t : NumDeclContextBits;
     /// For the bits in TagDeclBitfields.
     uint64_t : NumTagDeclBits;
 
@@ -1615,8 +1611,8 @@ class DeclContext {
     uint64_t ODRHash : 26;
   };
 
-  /// Number of non-inherited bits in RecordDeclBitfields.
-  enum { NumRecordDeclBits = 41 };
+  /// Number of inherited and non-inherited bits in RecordDeclBitfields.
+  enum { NumRecordDeclBits = NumTagDeclBits + 41 };
 
   /// Stores the bits used by OMPDeclareReductionDecl.
   /// If modified NumOMPDeclareReductionDeclBits and the accessor
@@ -1631,8 +1627,9 @@ class DeclContext {
     uint64_t InitializerKind : 2;
   };
 
-  /// Number of non-inherited bits in OMPDeclareReductionDeclBitfields.
-  enum { NumOMPDeclareReductionDeclBits = 2 };
+  /// Number of inherited and non-inherited bits in
+  /// OMPDeclareReductionDeclBitfields.
+  enum { NumOMPDeclareReductionDeclBits = NumDeclContextBits + 2 };
 
   /// Stores the bits used by FunctionDecl.
   /// If modified NumFunctionDeclBits and the accessor
@@ -1711,16 +1708,14 @@ class DeclContext {
     uint64_t FriendConstraintRefersToEnclosingTemplate : 1;
   };
 
-  /// Number of non-inherited bits in FunctionDeclBitfields.
-  enum { NumFunctionDeclBits = 31 };
+  /// Number of inherited and non-inherited bits in FunctionDeclBitfields.
+  enum { NumFunctionDeclBits = NumDeclContextBits + 31 };
 
   /// Stores the bits used by CXXConstructorDecl. If modified
   /// NumCXXConstructorDeclBits and the accessor
   /// methods in CXXConstructorDecl should be updated appropriately.
   class CXXConstructorDeclBitfields {
     friend class CXXConstructorDecl;
-    /// For the bits in DeclContextBitfields.
-    uint64_t : NumDeclContextBits;
     /// For the bits in FunctionDeclBitfields.
     uint64_t : NumFunctionDeclBits;
 
@@ -1739,10 +1734,8 @@ class DeclContext {
     uint64_t IsSimpleExplicit : 1;
   };
 
-  /// Number of non-inherited bits in CXXConstructorDeclBitfields.
-  enum {
-    NumCXXConstructorDeclBits = 64 - NumDeclContextBits - NumFunctionDeclBits
-  };
+  /// Number of inherited and non-inherited bits in CXXConstructorDeclBitfields.
+  enum { NumCXXConstructorDeclBits = 64 };
 
   /// Stores the bits used by ObjCMethodDecl.
   /// If modified NumObjCMethodDeclBits and the accessor
@@ -1803,8 +1796,8 @@ class DeclContext {
     uint64_t HasSkippedBody : 1;
   };
 
-  /// Number of non-inherited bits in ObjCMethodDeclBitfields.
-  enum { NumObjCMethodDeclBits = 24 };
+  /// Number of inherited and non-inherited bits in ObjCMethodDeclBitfields.
+  enum { NumObjCMethodDeclBits = NumDeclContextBits + 24 };
 
   /// Stores the bits used by ObjCContainerDecl.
   /// If modified NumObjCContainerDeclBits and the accessor
@@ -1819,10 +1812,10 @@ class DeclContext {
     SourceLocation AtStart;
   };
 
-  /// Number of non-inherited bits in ObjCContainerDeclBitfields.
+  /// Number of inherited and non-inherited bits in ObjCContainerDeclBitfields.
   /// Note that here we rely on the fact that SourceLocation is 32 bits
   /// wide. We check this with the static_assert in the ctor of DeclContext.
-  enum { NumObjCContainerDeclBits = 64 - NumDeclContextBits };
+  enum { NumObjCContainerDeclBits = 64 };
 
   /// Stores the bits used by LinkageSpecDecl.
   /// If modified NumLinkageSpecDeclBits and the accessor
@@ -1843,8 +1836,8 @@ class DeclContext {
     uint64_t HasBraces : 1;
   };
 
-  /// Number of non-inherited bits in LinkageSpecDeclBitfields.
-  enum { NumLinkageSpecDeclBits = 4 };
+  /// Number of inherited and non-inherited bits in LinkageSpecDeclBitfields.
+  enum { NumLinkageSpecDeclBits = NumDeclContextBits + 4 };
 
   /// Stores the bits used by BlockDecl.
   /// If modified NumBlockDeclBits and the accessor
@@ -1869,8 +1862,8 @@ class DeclContext {
     uint64_t CanAvoidCopyToHeap : 1;
   };
 
-  /// Number of non-inherited bits in BlockDeclBitfields.
-  enum { NumBlockDeclBits = 5 };
+  /// Number of inherited and non-inherited bits in BlockDeclBitfields.
+  enum { NumBlockDeclBits = NumDeclContextBits + 5 };
 
   /// Pointer to the data structure used to lookup declarations
   /// within this context (or a DependentStoredDeclsMap if this is a
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index e3dbe3b8a45cc3e..f52d72bdb2d4155 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1663,11 +1663,12 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
     /// Actually an ArrayType::ArraySizeModifier.
     unsigned SizeModifier : 3;
   };
+  enum { NumArrayTypeBits = NumTypeBits + 3 + 3 };
 
   class ConstantArrayTypeBitfields {
     friend class ConstantArrayType;
 
-    unsigned : NumTypeBits + 3 + 3;
+    unsigned : NumArrayTypeBits;
 
     /// Whether we have a stored size expression.
     unsigned HasStoredSizeExpr : 1;
@@ -1780,7 +1781,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
     unsigned Keyword : 8;
   };
 
-  enum { NumTypeWithKeywordBits = 8 };
+  enum { NumTypeWithKeywordBits = NumTypeBits + 8 };
 
   class ElaboratedTypeBitfields {
     friend class ElaboratedType;
@@ -1913,7 +1914,6 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
   class DependentTemplateSpecializationTypeBitfields {
     friend class DependentTemplateSpecializationType;
 
-    unsigned : NumTypeBits;
     unsigned : NumTypeWithKeywordBits;
 
     /// The number of template arguments named in this class template
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 8a2ea7c7624ceb8..b3364113abf15ba 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -431,7 +431,7 @@ void ASTDeclWriter::VisitTypeAliasDecl(TypeAliasDecl *D) {
 }
 
 void ASTDeclWriter::VisitTagDecl(TagDecl *D) {
-  static_assert(DeclContext::NumTagDeclBits == 10,
+  static_assert(DeclContext::NumTagDeclBits == 23,
                 "You need to update the serializer after you change the "
                 "TagDeclBits");
 
@@ -459,7 +459,7 @@ void ASTDeclWriter::VisitTagDecl(TagDecl *D) {
 }
 
 void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) {
-  static_assert(DeclContext::NumEnumDeclBits == 20,
+  static_assert(DeclContext::NumEnumDeclBits == 43,
                 "You need to update the serializer after you change the "
                 "EnumDeclBits");
 
@@ -506,7 +506,7 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) {
 }
 
 void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) {
-  static_assert(DeclContext::NumRecordDeclBits == 41,
+  static_assert(DeclContext::NumRecordDeclBits == 64,
                 "You need to update the serializer after you change the "
                 "RecordDeclBits");
 
@@ -578,7 +578,7 @@ void ASTDeclWriter::VisitDeclaratorDecl(DeclaratorDecl *D) {
 }
 
 void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
-  static_assert(DeclContext::NumFunctionDeclBits == 31,
+  static_assert(DeclContext::NumFunctionDeclBits == 44,
                 "You need to update the serializer after you change the "
                 "FunctionDeclBits");
 
@@ -726,7 +726,7 @@ void ASTDeclWriter::VisitCXXDeductionGuideDecl(CXXDeductionGuideDecl *D) {
 }
 
 void ASTDeclWriter::VisitObjCMethodDecl(ObjCMethodDecl *D) {
-  static_assert(DeclContext::NumObjCMethodDeclBits == 24,
+  static_assert(DeclContext::NumObjCMethodDeclBits == 37,
                 "You need to update the serializer after you change the "
                 "ObjCMethodDeclBits");
 
@@ -788,7 +788,7 @@ void ASTDeclWriter::VisitObjCTypeParamDecl(ObjCTypeParamDecl *D) {
 }
 
 void ASTDeclWriter::VisitObjCContainerDecl(ObjCContainerDecl *D) {
-  static_assert(DeclContext::NumObjCContainerDeclBits == 51,
+  static_assert(DeclContext::NumObjCContainerDeclBits == 64,
                 "You need to update the serializer after you change the "
                 "ObjCContainerDeclBits");
 
@@ -1268,7 +1268,7 @@ void ASTDeclWriter::VisitCapturedDecl(CapturedDecl *CD) {
 }
 
 void ASTDeclWriter::VisitLinkageSpecDecl(LinkageSpecDecl *D) {
-  static_assert(DeclContext::NumLinkageSpecDeclBits == 4,
+  static_assert(DeclContext::NumLinkageSpecDeclBits == 17,
                 "You need to update the serializer after you change the"
                 "LinkageSpecDeclBits");
 
@@ -1479,7 +1479,7 @@ void ASTDeclWriter::VisitCXXMethodDecl(CXXMethodDecl *D) {
 }
 
 void ASTDeclWriter::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
-  static_assert(DeclContext::NumCXXConstructorDeclBits == 20,
+  static_assert(DeclContext::NumCXXConstructorDeclBits == 64,
                 "You need to update the serializer after you change the "
                 "CXXConstructorDeclBits");
 
@@ -1960,7 +1960,7 @@ void ASTDeclWriter::VisitOMPRequiresDecl(OMPRequiresDecl *D) {
 }
 
 void ASTDeclWriter::VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D) {
-  static_assert(DeclContext::NumOMPDeclareReductionDeclBits == 2,
+  static_assert(DeclContext::NumOMPDeclareReductionDeclBits == 15,
                 "You need to update the serializer after you change the "
                 "NumOMPDeclareReductionDeclBits");
 

Copy link
Member

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

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

LGTM. Consistency is good.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM though I have some NFC suggestions you can take or leave.

clang/include/clang/AST/DeclBase.h Outdated Show resolved Hide resolved
clang/include/clang/AST/DeclBase.h Show resolved Hide resolved
clang/include/clang/AST/Type.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM!

@Endilll Endilll merged commit a6d0e87 into llvm:main Oct 26, 2023
2 of 3 checks passed
@Endilll Endilll deleted the refactor-bitfield-enums branch October 26, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category code-quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants