[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

Ilia Kuklin via lldb-commits lldb-commits at lists.llvm.org
Tue Nov 5 07:21:36 PST 2024


https://github.com/kuilpd updated https://github.com/llvm/llvm-project/pull/115005

>From 5290832b802d98b9d293b6910c0837911ec490c4 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin <ikuklin at accesssoftek.com>
Date: Mon, 4 Nov 2024 14:33:45 +0500
Subject: [PATCH 1/4] [lldb] Analyze enum promotion type during parsing

---
 clang/include/clang/AST/Decl.h                |  2 +-
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 95 ++++++++++++++++++-
 .../TypeSystem/Clang/TypeSystemClang.cpp      | 29 +-----
 3 files changed, 100 insertions(+), 26 deletions(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 8c39ef3d5a9fa6..a78e6c92abb22b 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3891,6 +3891,7 @@ class EnumDecl : public TagDecl {
   void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED,
                                     TemplateSpecializationKind TSK);
 
+public:
   /// Sets the width in bits required to store all the
   /// non-negative enumerators of this enum.
   void setNumPositiveBits(unsigned Num) {
@@ -3902,7 +3903,6 @@ class EnumDecl : public TagDecl {
   /// negative enumerators of this enum. (see getNumNegativeBits)
   void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; }
 
-public:
   /// True if this tag declaration is a scoped enumeration. Only
   /// possible in C++11 mode.
   void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a30d898a93cc4d..a21607c2020161 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2248,6 +2248,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
     return 0;
 
   size_t enumerators_added = 0;
+  unsigned NumNegativeBits = 0;
+  unsigned NumPositiveBits = 0;
 
   for (DWARFDIE die : parent_die.children()) {
     const dw_tag_t tag = die.Tag();
@@ -2299,11 +2301,102 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
     }
 
     if (name && name[0] && got_value) {
-      m_ast.AddEnumerationValueToEnumerationType(
+      auto ECD = m_ast.AddEnumerationValueToEnumerationType(
           clang_type, decl, name, enum_value, enumerator_byte_size * 8);
       ++enumerators_added;
+
+      llvm::APSInt InitVal = ECD->getInitVal();
+      // Keep track of the size of positive and negative values.
+      if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+        // If the enumerator is zero that should still be counted as a positive
+        // bit since we need a bit to store the value zero.
+        unsigned ActiveBits = InitVal.getActiveBits();
+        NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+      } else {
+        NumNegativeBits =
+            std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());
+      }
     }
   }
+
+  /// The following code follows the same logic as in Sema::ActOnEnumBody
+  /// clang/lib/Sema/SemaDecl.cpp
+  // If we have an empty set of enumerators we still need one bit.
+  // From [dcl.enum]p8
+  // If the enumerator-list is empty, the values of the enumeration are as if
+  // the enumeration had a single enumerator with value 0
+  if (!NumPositiveBits && !NumNegativeBits)
+    NumPositiveBits = 1;
+
+  clang::QualType qual_type(ClangUtil::GetQualType(clang_type));
+  clang::EnumDecl *enum_decl = qual_type->getAs<clang::EnumType>()->getDecl();
+  enum_decl->setNumPositiveBits(NumPositiveBits);
+  enum_decl->setNumNegativeBits(NumNegativeBits);
+
+  // C++0x N3000 [conv.prom]p3:
+  //   An rvalue of an unscoped enumeration type whose underlying
+  //   type is not fixed can be converted to an rvalue of the first
+  //   of the following types that can represent all the values of
+  //   the enumeration: int, unsigned int, long int, unsigned long
+  //   int, long long int, or unsigned long long int.
+  // C99 6.4.4.3p2:
+  //   An identifier declared as an enumeration constant has type int.
+  // The C99 rule is modified by C23.
+  clang::QualType BestPromotionType;
+  unsigned BestWidth;
+
+  auto &Context = m_ast.getASTContext();
+  unsigned LongWidth = Context.getTargetInfo().getLongWidth();
+  unsigned IntWidth = Context.getTargetInfo().getIntWidth();
+  unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+  unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
+
+  bool is_cpp = Language::LanguageIsCPlusPlus(
+      SymbolFileDWARF::GetLanguage(*parent_die.GetCU()));
+
+  if (NumNegativeBits) {
+    // If there is a negative value, figure out the smallest integer type (of
+    // int/long/longlong) that fits.
+    if (NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) {
+      BestWidth = CharWidth;
+    } else if (NumNegativeBits <= ShortWidth && NumPositiveBits < ShortWidth) {
+      BestWidth = ShortWidth;
+    } else if (NumNegativeBits <= IntWidth && NumPositiveBits < IntWidth) {
+      BestWidth = IntWidth;
+    } else if (NumNegativeBits <= LongWidth && NumPositiveBits < LongWidth) {
+      BestWidth = LongWidth;
+    } else {
+      BestWidth = Context.getTargetInfo().getLongLongWidth();
+    }
+    BestPromotionType = (BestWidth <= IntWidth ? Context.IntTy : qual_type);
+  } else {
+    // If there is no negative value, figure out the smallest type that fits
+    // all of the enumerator values.
+    if (NumPositiveBits <= CharWidth) {
+      BestPromotionType = Context.IntTy;
+      BestWidth = CharWidth;
+    } else if (NumPositiveBits <= ShortWidth) {
+      BestPromotionType = Context.IntTy;
+      BestWidth = ShortWidth;
+    } else if (NumPositiveBits <= IntWidth) {
+      BestWidth = IntWidth;
+      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
+                              ? Context.UnsignedIntTy
+                              : Context.IntTy;
+    } else if (NumPositiveBits <= LongWidth) {
+      BestWidth = LongWidth;
+      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
+                              ? Context.UnsignedLongTy
+                              : Context.LongTy;
+    } else {
+      BestWidth = Context.getTargetInfo().getLongLongWidth();
+      BestPromotionType = (NumPositiveBits == BestWidth || !is_cpp)
+                              ? Context.UnsignedLongLongTy
+                              : Context.LongLongTy;
+    }
+  }
+  enum_decl->setPromotionType(BestPromotionType);
+
   return enumerators_added;
 }
 
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index f5063175d6e070..68bc81f51e6a92 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2341,7 +2341,6 @@ CompilerType TypeSystemClang::CreateEnumerationType(
   if (decl_ctx)
     decl_ctx->addDecl(enum_decl);
 
-  // TODO: check if we should be setting the promotion type too?
   enum_decl->setIntegerType(ClangUtil::GetQualType(integer_clang_type));
 
   enum_decl->setAccess(AS_public); // TODO respect what's in the debug info
@@ -8461,30 +8460,11 @@ bool TypeSystemClang::CompleteTagDeclarationDefinition(
   if (enum_decl->isCompleteDefinition())
     return true;
 
-  clang::ASTContext &ast = lldb_ast->getASTContext();
-
-  /// TODO This really needs to be fixed.
-
   QualType integer_type(enum_decl->getIntegerType());
   if (!integer_type.isNull()) {
-    unsigned NumPositiveBits = 1;
-    unsigned NumNegativeBits = 0;
-
-    clang::QualType promotion_qual_type;
-    // If the enum integer type is less than an integer in bit width,
-    // then we must promote it to an integer size.
-    if (ast.getTypeSize(enum_decl->getIntegerType()) <
-        ast.getTypeSize(ast.IntTy)) {
-      if (enum_decl->getIntegerType()->isSignedIntegerType())
-        promotion_qual_type = ast.IntTy;
-      else
-        promotion_qual_type = ast.UnsignedIntTy;
-    } else
-      promotion_qual_type = enum_decl->getIntegerType();
-
-    enum_decl->completeDefinition(enum_decl->getIntegerType(),
-                                  promotion_qual_type, NumPositiveBits,
-                                  NumNegativeBits);
+    enum_decl->completeDefinition(
+        enum_decl->getIntegerType(), enum_decl->getPromotionType(),
+        enum_decl->getNumPositiveBits(), enum_decl->getNumNegativeBits());
   }
   return true;
 }
@@ -8544,7 +8524,8 @@ clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(
   bool is_signed = false;
   underlying_type.IsIntegerType(is_signed);
 
-  llvm::APSInt value(enum_value_bit_size, is_signed);
+  // APSInt constructor's sign argument is isUnsigned
+  llvm::APSInt value(enum_value_bit_size, !is_signed);
   value = enum_value;
 
   return AddEnumerationValueToEnumerationType(enum_type, decl, name, value);

>From b2f5a17fddd324c3c3814b8ca645868999254de8 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin <ikuklin at accesssoftek.com>
Date: Tue, 5 Nov 2024 19:18:41 +0500
Subject: [PATCH 2/4] Fix enum's underlying type retrieval

---
 .../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp       | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a21607c2020161..520fd40eda4825 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2328,8 +2328,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
   if (!NumPositiveBits && !NumNegativeBits)
     NumPositiveBits = 1;
 
-  clang::QualType qual_type(ClangUtil::GetQualType(clang_type));
-  clang::EnumDecl *enum_decl = qual_type->getAs<clang::EnumType>()->getDecl();
+  clang::EnumDecl *enum_decl =
+      ClangUtil::GetQualType(clang_type)->getAs<clang::EnumType>()->getDecl();
   enum_decl->setNumPositiveBits(NumPositiveBits);
   enum_decl->setNumNegativeBits(NumNegativeBits);
 
@@ -2368,7 +2368,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
     } else {
       BestWidth = Context.getTargetInfo().getLongLongWidth();
     }
-    BestPromotionType = (BestWidth <= IntWidth ? Context.IntTy : qual_type);
+    BestPromotionType =
+        BestWidth <= IntWidth ? Context.IntTy : enum_decl->getIntegerType();
   } else {
     // If there is no negative value, figure out the smallest type that fits
     // all of the enumerator values.

>From 62c801145a2312d2c9339d30cf116fc2e709d630 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin <ikuklin at accesssoftek.com>
Date: Tue, 5 Nov 2024 19:19:14 +0500
Subject: [PATCH 3/4] Add a test

---
 .../test/API/lang/cpp/enum_promotion/Makefile |  3 ++
 .../enum_promotion/TestCPPEnumPromotion.py    | 37 +++++++++++++++++++
 .../test/API/lang/cpp/enum_promotion/main.cpp | 22 +++++++++++
 3 files changed, 62 insertions(+)
 create mode 100644 lldb/test/API/lang/cpp/enum_promotion/Makefile
 create mode 100644 lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py
 create mode 100644 lldb/test/API/lang/cpp/enum_promotion/main.cpp

diff --git a/lldb/test/API/lang/cpp/enum_promotion/Makefile b/lldb/test/API/lang/cpp/enum_promotion/Makefile
new file mode 100644
index 00000000000000..99998b20bcb050
--- /dev/null
+++ b/lldb/test/API/lang/cpp/enum_promotion/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py b/lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py
new file mode 100644
index 00000000000000..a4eed53c8d80f2
--- /dev/null
+++ b/lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py
@@ -0,0 +1,37 @@
+"""
+Test LLDB type promotion of unscoped enums.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCPPEnumPromotion(TestBase):
+
+    @skipIf(debug_info=no_match(["dwarf", "dwo"]))
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("main.cpp")
+        )
+        UChar_promoted = self.frame().FindVariable("UChar_promoted")
+        UShort_promoted = self.frame().FindVariable("UShort_promoted")
+        UInt_promoted = self.frame().FindVariable("UInt_promoted")
+        SLong_promoted = self.frame().FindVariable("SLong_promoted")
+        ULong_promoted = self.frame().FindVariable("ULong_promoted")
+        NChar_promoted = self.frame().FindVariable("NChar_promoted")
+        NShort_promoted = self.frame().FindVariable("NShort_promoted")
+        NInt_promoted = self.frame().FindVariable("NInt_promoted")
+        NLong_promoted = self.frame().FindVariable("NLong_promoted")
+
+        # Check that LLDB's promoted type is the same as the compiler's
+        self.expect_expr("+EnumUChar::UChar", result_type=UChar_promoted.type.name)
+        self.expect_expr("+EnumUShort::UShort", result_type=UShort_promoted.type.name)
+        self.expect_expr("+EnumUInt::UInt", result_type=UInt_promoted.type.name)
+        self.expect_expr("+EnumSLong::SLong", result_type=SLong_promoted.type.name)
+        self.expect_expr("+EnumULong::ULong", result_type=ULong_promoted.type.name)
+        self.expect_expr("+EnumNChar::NChar", result_type=NChar_promoted.type.name)
+        self.expect_expr("+EnumNShort::NShort", result_type=NShort_promoted.type.name)
+        self.expect_expr("+EnumNInt::NInt", result_type=NInt_promoted.type.name)
+        self.expect_expr("+EnumNLong::NLong", result_type=NLong_promoted.type.name)
diff --git a/lldb/test/API/lang/cpp/enum_promotion/main.cpp b/lldb/test/API/lang/cpp/enum_promotion/main.cpp
new file mode 100644
index 00000000000000..fafb96f6c7b075
--- /dev/null
+++ b/lldb/test/API/lang/cpp/enum_promotion/main.cpp
@@ -0,0 +1,22 @@
+enum EnumUChar { UChar = 1 };
+enum EnumUShort { UShort = 0x101 };
+enum EnumUInt { UInt = 0x10001 };
+enum EnumSLong { SLong = 0x100000001 };
+enum EnumULong { ULong = 0xFFFFFFFFFFFFFFF0 };
+enum EnumNChar { NChar = -1 };
+enum EnumNShort { NShort= -0x101 };
+enum EnumNInt { NInt = -0x10001 };
+enum EnumNLong { NLong = -0x100000001 };
+
+int main() {
+  auto UChar_promoted = +EnumUChar::UChar;
+  auto UShort_promoted = +EnumUShort::UShort;
+  auto UInt_promoted = +EnumUInt::UInt;
+  auto SLong_promoted = +EnumSLong::SLong;
+  auto ULong_promoted = +EnumULong::ULong;
+  auto NChar_promoted = +EnumNChar::NChar;
+  auto NShort_promoted = +EnumNShort::NShort;
+  auto NInt_promoted = +EnumNInt::NInt;
+  auto NLong_promoted = +EnumNLong::NLong;
+  return 0; // break here
+}

>From 19b98ad94b73adeb59025218397ac1903e25d109 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin <ikuklin at accesssoftek.com>
Date: Tue, 5 Nov 2024 20:10:58 +0500
Subject: [PATCH 4/4] Fix formatting

---
 lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py | 2 +-
 lldb/test/API/lang/cpp/enum_promotion/main.cpp                | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py b/lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py
index a4eed53c8d80f2..3cf110441a0ae7 100644
--- a/lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py
+++ b/lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py
@@ -7,8 +7,8 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestCPPEnumPromotion(TestBase):
 
+class TestCPPEnumPromotion(TestBase):
     @skipIf(debug_info=no_match(["dwarf", "dwo"]))
     def test(self):
         self.build()
diff --git a/lldb/test/API/lang/cpp/enum_promotion/main.cpp b/lldb/test/API/lang/cpp/enum_promotion/main.cpp
index fafb96f6c7b075..bcdb0adff5d401 100644
--- a/lldb/test/API/lang/cpp/enum_promotion/main.cpp
+++ b/lldb/test/API/lang/cpp/enum_promotion/main.cpp
@@ -4,7 +4,7 @@ enum EnumUInt { UInt = 0x10001 };
 enum EnumSLong { SLong = 0x100000001 };
 enum EnumULong { ULong = 0xFFFFFFFFFFFFFFF0 };
 enum EnumNChar { NChar = -1 };
-enum EnumNShort { NShort= -0x101 };
+enum EnumNShort { NShort = -0x101 };
 enum EnumNInt { NInt = -0x10001 };
 enum EnumNLong { NLong = -0x100000001 };
 



More information about the lldb-commits mailing list