[Lldb-commits] [PATCH] D77970: 2/2: [nfc] [lldb] DWARF callbacks: DIERef -> DWARFDIE

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 4 inline comments as done.
jankratochvil added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h:82
+  /// The object will remain valid during the whole call statement:
+  /// Function(name, DIERefCallback({callback, name}));
+  struct DIERefCallbackArgs {
----------------
labath wrote:
> jankratochvil wrote:
> > This calling with curly brackets is a bit tricky but I found it the least worst option.
> I think it would be much simpler to make this a callable object instead (though I'm not sure we need to worry about optimizing to this level of detail):
> ```
> class ResultProcessor (?) {
>   ResultProcessor(callback, name);
>   bool operator()(DIERef ref);
> };
> ```
Wow, that solved everything, thanks. No more mallocs, no more stale references, the most simple calling syntax. No compromises.
I did not know `llvm::function_ref<>` does not need any `std::function`, it is fine with a callable object.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:186
 
+  auto dierefcallback = DIERefCallback({callback, class_name.GetStringRef()});
   for (DIERef ref : incomplete_types)
----------------
labath wrote:
> dangling `DIERefCallbackArgs` here.
oops, I even did not handle the usage in a patch of mine.  But the stale references are no longer possible with the callable object now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77970/new/

https://reviews.llvm.org/D77970