[Lldb-commits] [lldb] [lldb][DataFormatter] Remove support for old std::map layout (PR #97549)

[Michael Buch via lldb-commits]

https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/97549

>From e80ca1531751eb6750eb65fcec75c760e0282792 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 3 Jul 2024 12:06:49 +0200
Subject: [PATCH 1/2] [lldb][DataFormatter] Remove support for old std::map
 layout

We currently supported the layout from pre-2016 (before the layout
change in 14caaddd3f08e798dcd9ac0ddfc).
We have another upcoming layout change in `__tree` and `map` (as part of
require rewriting parts of this formatter. Removing the support will
make those changes more straightforward to review/maintain.

Being backward compatible would be great but we have no tests that
actually verify that the old layout still works (and our oldest
matrix bot tests clang-15). If anyone feels strongly about keeping
this layout, we could possibly factor out that logic and keep it around.
---
 .../Plugins/Language/CPlusPlus/LibCxxMap.cpp  | 68 ++++++++-----------
 1 file changed, 29 insertions(+), 39 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..7eb6a3637acd2 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -264,39 +264,33 @@ void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
   if (!node)
     return;
   CompilerType node_type(node->GetCompilerType());
-  uint64_t bit_offset;
-  if (node_type.GetIndexOfFieldWithName("__value_", nullptr, &bit_offset) !=
-      UINT32_MAX) {
-    // Old layout (pre 089a7cc5dea)
-    m_skip_size = bit_offset / 8u;
-  } else {
-    auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null<TypeSystemClang>();
-    if (!ast_ctx)
-      return;
-    CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-        llvm::StringRef(),
-        {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
-         {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
-         {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
-         {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
-         {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
-    std::string child_name;
-    uint32_t child_byte_size;
-    int32_t child_byte_offset = 0;
-    uint32_t child_bitfield_bit_size;
-    uint32_t child_bitfield_bit_offset;
-    bool child_is_base_class;
-    bool child_is_deref_of_parent;
-    uint64_t language_flags;
-    auto child_type =
-        llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
-            nullptr, 4, true, true, true, child_name, child_byte_size,
-            child_byte_offset, child_bitfield_bit_size,
-            child_bitfield_bit_offset, child_is_base_class,
-            child_is_deref_of_parent, nullptr, language_flags));
-    if (child_type && child_type->IsValid())
-      m_skip_size = (uint32_t)child_byte_offset;
-  }
+  auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null<TypeSystemClang>();
+  if (!ast_ctx)
+    return;
+
+  CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+      llvm::StringRef(),
+      {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+       {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+       {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+       {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
+       {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
+  std::string child_name;
+  uint32_t child_byte_size;
+  int32_t child_byte_offset = 0;
+  uint32_t child_bitfield_bit_size;
+  uint32_t child_bitfield_bit_offset;
+  bool child_is_base_class;
+  bool child_is_deref_of_parent;
+  uint64_t language_flags;
+  auto child_type =
+      llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
+          nullptr, 4, true, true, true, child_name, child_byte_size,
+          child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
+          child_is_base_class, child_is_deref_of_parent, nullptr,
+          language_flags));
+  if (child_type && child_type->IsValid())
+    m_skip_size = (uint32_t)child_byte_offset;
 }
 
 lldb::ValueObjectSP
@@ -343,12 +337,8 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
       return lldb::ValueObjectSP();
     }
     GetValueOffset(iterated_sp);
-    auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-    if (child_sp)
-      iterated_sp = child_sp;
-    else
-      iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
-          m_skip_size, m_element_type, true);
+    iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
+                                                         m_element_type, true);
     if (!iterated_sp) {
       m_tree = nullptr;
       return lldb::ValueObjectSP();

>From 3f760be3995be0b3d96eab8b95a9e8ff710232a0 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 3 Jul 2024 15:45:40 +0200
Subject: [PATCH 2/2] fixup! remove more leftovers of the old layout support

---
 lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 7eb6a3637acd2..ac15bacdb2df4 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -231,11 +231,6 @@ bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
   deref = m_root_node->Dereference(error);
   if (!deref || error.Fail())
     return false;
-  deref = deref->GetChildMemberWithName("__value_");
-  if (deref) {
-    m_element_type = deref->GetCompilerType();
-    return true;
-  }
   deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
   if (!deref)
     return false;