[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.


================
Comment at: lldb/source/Target/TargetList.cpp:293
   // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
+  if (!m_dummy_target_sp) {
     ArchSpec arch(Target::GetDefaultArchitecture());
----------------
vsk wrote:
> jingham wrote:
> > JDevlieghere wrote:
> > > I wonder if we gain anything by making this lazy. Maybe it's time to address that FIXME and always have a single DummyTarget held onto by the Debugger? 
> > I see no reason not to just make a dummy target when you make a debugger.  That should be a very cheap operation, and the object itself should not be large.
> > 
> > I think for regularities sake it's better to have the TargetList have all the targets, including the Dummy target, rather than have the DummyTarget held specially by the debugger.  But it would be easy for the Debugger to make a DummyTarget and inject it into the TargetList as part of its startup.  We could even make the straight constructor of TargetList do this job.  We copy the debugger TargetList to iterate over it and for similar purposes in a bunch of places, but that goes through the copy constructor so it wouldn't interfere with this.
> You're right, that's even better.
So far, the policy has been to not include the dummy target in the list of targets (i.e, you can't find it via TargetList::GetTargetAtIndex or TargetList::GetIndexOfTarget). If, down the line, we need to support that we can add the functionality though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872