[Lldb-commits] [lldb] ae570d5 - [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (#125203)

[via lldb-commits]

Author: Michael Buch
Date: 2025-01-31T13:09:46Z
New Revision: ae570d5d77e806784064ee819211868e745d0fbe

URL: https://github.com/llvm/llvm-project/commit/ae570d5d77e806784064ee819211868e745d0fbe
DIFF: https://github.com/llvm/llvm-project/commit/ae570d5d77e806784064ee819211868e745d0fbe.diff

LOG: [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (#125203)

I tried using `CompleteEnumType` to replace some duplicated code in
`DWARFASTParserClang::ParseEnum` but tests started failing.

`CompleteEnumType` parses/attaches the child enumerators using the
signedness it got from `CompilerType::IsIntegerType`. However, this
would only report the correct signedness for builtin integer types
(never for `clang::EnumType`s). We have a different API for that in
`CompilerType::IsIntegerOrEnumerationType` which could've been used
there instead. This patch calls `IsEnumerationIntegerTypeSigned` to
determine signedness because we always pass an enum type into
`CompleteEnumType` anyway.

Based on git history this has been the case for a long time, but
possibly never caused issues because `ParseEnum` was completing the
definition manually instead of through `CompleteEnumType`.

I couldn't find a good way to test `CompleteEnumType` on its own because
it expects an enum type to be passed to it, which only gets created in
`ParseEnum` (at which point we already call `CompleteEnumType`). The
only other place we call `CompleteEnumType` at is in
[`CompleteTypeFromDWARF`](https://github.com/llvm/llvm-project/blob/466217eb0334d467ec8e9b96c52ee988aaadc389/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L2260-L2262).
Though I think we don't actually ever end up calling into that codepath
because we eagerly complete enum definitions. Maybe we can remove that
call to `CompleteEnumType` in a follow-up.

Added: 
    

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba693..dca193fc11b552 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1019,16 +1019,7 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc,
     // Declaration DIE is inserted into the type map in ParseTypeFromDWARF
   }
 
-
-  if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-    if (def_die.HasChildren()) {
-      bool is_signed = false;
-      enumerator_clang_type.IsIntegerType(is_signed);
-      ParseChildEnumerators(clang_type, is_signed,
-                            type_sp->GetByteSize(nullptr).value_or(0), def_die);
-    }
-    TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-  } else {
+  if (!CompleteEnumType(def_die, type_sp.get(), clang_type)) {
     dwarf->GetObjectFile()->GetModule()->ReportError(
         "DWARF DIE at {0:x16} named \"{1}\" was not able to start its "
         "definition.\nPlease file a bug and attach the file at the "
@@ -2221,13 +2212,14 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
 bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
                                            lldb_private::Type *type,
                                            const CompilerType &clang_type) {
+  assert(clang_type.IsEnumerationType());
+
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-    if (die.HasChildren()) {
-      bool is_signed = false;
-      clang_type.IsIntegerType(is_signed);
-      ParseChildEnumerators(clang_type, is_signed,
+    if (die.HasChildren())
+      ParseChildEnumerators(clang_type,
+                            clang_type.IsEnumerationIntegerTypeSigned(),
                             type->GetByteSize(nullptr).value_or(0), die);
-    }
+
     TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
   }
   return (bool)clang_type;