[Lldb-commits] [PATCH] D74637: Unify DIERef vs. user_id_t

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: labath, clayborg.
jankratochvil added a project: LLDB.
Herald added a subscriber: aprantl.

As discussed <https://reviews.llvm.org/D73206#1871895> in D73206 <https://reviews.llvm.org/D73206> there is both `DIERef` and `user_id_t` and sometimes (for DWZ) we need to encode Main CU into them and sometimes we cannot as it is unavailable at that point and at the same time not even needed.
I have also noticed `DIERef` and `user_id_t` in fact contain the same information which can be seen in SymbolFileDWARF::GetUID <https://github.com/llvm/llvm-project/blob/llvmorg-10.0.0-rc2/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1229>.
(Offtopic: Moreover `UserID` is also another form of `user_id_t`.)
SB* API/ABI is already using `user_id_t` and it needs to encode Main CU for DWZ. Therefore what about making `DIERef` the identifier not containing Main CU and `user_id_t` the identifier containing Main CU?
@labath also said:

> I think it would be good to have only one kind of "user id". What are the cases where you need a main-cu-less user id?

This patch does that in a small scale, is it the proper way forward?
(Personally I think the non-MainCU and plus-MainCU forms should be the same, either both `user_id_t` or both `DIERef` - and to drop the other form completely - but that can be always easily refactored any time.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74637

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -498,7 +498,7 @@
   bool m_fetched_external_modules : 1;
   lldb_private::LazyBool m_supports_DW_AT_APPLE_objc_complete_type;
 
-  typedef std::set<lldb::user_id_t> DIERefSet;
+  typedef std::set<DIERef> DIERefSet;
   typedef llvm::StringMap<DIERefSet> NameToOffsetMap;
   NameToOffsetMap m_function_scope_qualified_name_map;
   std::unique_ptr<DWARFDebugRanges> m_ranges;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2385,9 +2385,9 @@
       dwo->GetMangledNamesForFunction(scope_qualified_name, mangled_names);
   }
 
-  for (lldb::user_id_t uid :
+  for (DIERef die_ref :
        m_function_scope_qualified_name_map.lookup(scope_qualified_name)) {
-    DWARFDIE die = GetDIE(uid);
+    DWARFDIE die = GetDIE(die_ref);
     mangled_names.push_back(ConstString(die.GetMangledName()));
   }
 }
@@ -3035,7 +3035,7 @@
                                            .AsCString(""));
       if (scope_qualified_name.size()) {
         m_function_scope_qualified_name_map[scope_qualified_name].insert(
-            die.GetID());
+            *die.GetDIERef());
       }
     }
   }
Index: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
@@ -44,6 +44,16 @@
 
   dw_offset_t die_offset() const { return m_die_offset; }
 
+  bool operator<(DIERef other) const {
+    if (m_dwo_num_valid != other.m_dwo_num_valid)
+      return m_dwo_num_valid < other.m_dwo_num_valid;
+    if (m_dwo_num_valid && (m_dwo_num != other.m_dwo_num))
+      return m_dwo_num < other.m_dwo_num;
+    if (m_section != other.m_section)
+      return m_section < other.m_section;
+    return m_die_offset < other.m_die_offset;
+  }
+
 private:
   uint32_t m_dwo_num : 30;
   uint32_t m_dwo_num_valid : 1;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74637.244738.patch
Type: text/x-patch
Size: 2297 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200214/8e67d685/attachment.bin>