[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Apr 6 02:40:53 PDT 2020

labath added a comment.

Thanks for doing this. This looks more-or-less like what I was expecting. The need for the "bool" return value from the index functions is unfortunate (I did not anticipate that), but it does seem unavoidable.

Since none of these functions (I guess) are storing the callback function, could you replace std::function with llvm::function_ref ?

Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:130-163
+  bool GetValues(ConstString unique_cstr,
+                 std::function<bool(T value)> callback) const {
+    for (const Entry &entry : llvm::make_range(std::equal_range(
+             m_map.begin(), m_map.end(), unique_cstr, Compare())))
+      if (callback(entry.value))
+        return true;
I'm not sure these functions are really needed. They have just one caller, and they'd be trivial if this class provided appropriate abstractions. Maybe just add begin/end/equal_range methods, so that one can write:
for (value: map / map.equal_range(str))

(ideally, I'd like to have iterators instead of callbacks for the index classes too, but iterators and class hierarchies don't mix very well)

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h:79
   llvm::Optional<DIERef> ToDIERef(const DebugNames::Entry &entry);
-  void Append(const DebugNames::Entry &entry, DIEArray &offsets);
+  bool Append(const DebugNames::Entry &entry,
+              std::function<bool(DIERef ref)> callback);
Maybe rename this to `ProcessEntry` or something?

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list