[lldb][TypeSystemClang] Pass around enum value as uint64_t

[Michael137]

The DWARFASTParserClang reads enum values as int64_ts regardless of the enumerators signedness. Then we pass it to AddEnumerationValueToEnumerationType and only then do we create an APSInt from it. However, there are other places where we read/pass around the enum value as unsigned. This patch makes sure we consistently use the same integer type for the enum value and let APSInt take care of signedness. This shouldn't have any observable effect.

[Michael137]

this is a drive-by cleanup

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

The DWARFASTParserClang reads enum values as int64_ts regardless of the enumerators signedness. Then we pass it to AddEnumerationValueToEnumerationType and only then do we create an APSInt from it. However, there are other places where we read/pass around the enum value as unsigned. This patch makes sure we consistently use the same integer type for the enum value and let APSInt take care of signedness. This shouldn't have any observable effect.

---

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

4 Files Affected:

• (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+3-5)
• (modified) lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (+1-1)
• (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+4-6)
• (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+1-1)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba693..3492fb472da8d1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2329,8 +2329,7 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
       continue;
 
     const char *name = nullptr;
-    bool got_value = false;
-    int64_t enum_value = 0;
+    std::optional<uint64_t> enum_value;
     Declaration decl;
 
     for (size_t i = 0; i < attributes.Size(); ++i) {
@@ -2339,7 +2338,6 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
       if (attributes.ExtractFormValueAtIndex(i, form_value)) {
         switch (attr) {
         case DW_AT_const_value:
-          got_value = true;
           if (is_signed)
             enum_value = form_value.Signed();
           else
@@ -2368,9 +2366,9 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
       }
     }
 
-    if (name && name[0] && got_value) {
+    if (name && name[0] && enum_value) {
       m_ast.AddEnumerationValueToEnumerationType(
-          clang_type, decl, name, enum_value, enumerator_byte_size * 8);
+          clang_type, decl, name, *enum_value, enumerator_byte_size * 8);
       ++enumerators_added;
     }
   }
diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index 990bacd89bf346..c6dd72e22fb4c1 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1155,7 +1155,7 @@ bool PDBASTParser::AddEnumValue(CompilerType enum_type,
   Variant v = enum_value.getValue();
   std::string name =
       std::string(MSVCUndecoratedNameParser::DropScope(enum_value.getName()));
-  int64_t raw_value;
+  uint64_t raw_value;
   switch (v.Type) {
   case PDB_VariantType::Int8:
     raw_value = v.Value.Int8;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index cb246fde976c2f..1da8fbe0bcd6dd 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -8541,12 +8541,10 @@ clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(
 
 clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(
     const CompilerType &enum_type, const Declaration &decl, const char *name,
-    int64_t enum_value, uint32_t enum_value_bit_size) {
-  CompilerType underlying_type = GetEnumerationIntegerType(enum_type);
-  bool is_signed = false;
-  underlying_type.IsIntegerType(is_signed);
-
-  llvm::APSInt value(enum_value_bit_size, !is_signed);
+    uint64_t enum_value, uint32_t enum_value_bit_size) {
+  assert(enum_type.IsEnumerationType());
+  llvm::APSInt value(enum_value_bit_size,
+                     !enum_type.IsEnumerationIntegerTypeSigned());
   value = enum_value;
 
   return AddEnumerationValueToEnumerationType(enum_type, decl, name, value);
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 83f954270e3093..e70ad4c2973a5c 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1040,7 +1040,7 @@ class TypeSystemClang : public TypeSystem {
   // Modifying Enumeration types
   clang::EnumConstantDecl *AddEnumerationValueToEnumerationType(
       const CompilerType &enum_type, const Declaration &decl, const char *name,
-      int64_t enum_value, uint32_t enum_value_bit_size);
+      uint64_t enum_value, uint32_t enum_value_bit_size);
   clang::EnumConstantDecl *AddEnumerationValueToEnumerationType(
       const CompilerType &enum_type, const Declaration &decl, const char *name,
       const llvm::APSInt &value);

[felipepiovezan]

after staring at this long enough, I think this is fine, since the uint->int conversion is done by apint