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

via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 5 07:07:56 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Ilia Kuklin (kuilpd)

<details>
<summary>Changes</summary>

The information about an enum's best promotion type is discarded after compilation and is not present in debug info. This patch repeats the same analysis of each enum value as in the front-end to determine the best promotion type during DWARF info parsing.

Fixes #<!-- -->86989

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


6 Files Affected:

- (modified) clang/include/clang/AST/Decl.h (+1-1) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+95-1) 
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+5-24) 
- (added) lldb/test/API/lang/cpp/enum_promotion/Makefile (+3) 
- (added) lldb/test/API/lang/cpp/enum_promotion/TestCPPEnumPromotion.py (+37) 
- (added) lldb/test/API/lang/cpp/enum_promotion/main.cpp (+22) 


``````````diff
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..520fd40eda4825 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,103 @@ 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::EnumDecl *enum_decl =
+      ClangUtil::GetQualType(clang_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 : enum_decl->getIntegerType();
+  } 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);
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
+}

``````````

</details>


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


More information about the cfe-commits mailing list