[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.


================
Comment at: lldb/include/lldb/Core/Debugger.h:435-436
+      ReportInterruption(InterruptionReport(cur_func, 
+                                            llvm::formatv(formatv, 
+                                            std::forward<Args>(args)...)));
+    }
----------------
mib wrote:
> If you use `LLVM_PRETTY_FUNCTION` you won't need this.
I'm not sure I understand how you want to use LLVM_PRETTY_FUNCTION.  From what I can tell, LLVM_PRETTY_FUNCTION prints something that looks like the function invocation as written.  But ReportInterruption and the associated macros provide a formatv API.


================
Comment at: lldb/source/Target/TargetList.cpp:518
              "target already exists it the list");
+  UnregisterInProcessTarget(target_sp.get());
   m_target_list.push_back(std::move(target_sp));
----------------
bulbazord wrote:
> I'm somewhat puzzled by this. Why are we unregistering the target in `AddTargetInternal` when we're registering it in `CreateTargetInternal`? Maybe I don't understand the intent behind `{Register,Unregister}InProcessTarget`.
The point behind this is someone says "make me a target" and then in the process of making the target, the Target code does something slow (e.g. look for external debug files) that we want to interrupt.  So we need a way for the debugger to find the "in the process of being made" Targets.  

 "AddTargetInternal" is the API where the target gets added to the Debugger's TargetList, so after that method has run we will find the target in the regular TargetList.  But between CreateTargetInternal and AddTargetInternal, that target isn't stored anywhere that the Debugger knows how to query.  I added this list to hold the targets that are in the process of getting made, before they get added to the Debugger's TargetList.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154542