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

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.


================
Comment at: lldb/source/Core/Debugger.cpp:685
 
-  m_dummy_target_sp = m_target_list.GetDummyTarget(*this);
+  {
+    ArchSpec arch(Target::GetDefaultArchitecture());
----------------
teemperor wrote:
> Maybe have a small comment here that summarizes the block: `// Create dummy target`?
I'll either fix this in a more substantial patch revision or before committing.


================
Comment at: lldb/source/Core/Debugger.cpp:692
+    m_dummy_target_sp.reset(
+        new Target(*this, arch, default_platform_sp, is_dummy_target));
+  }
----------------
JDevlieghere wrote:
> Should we move all this into `Target` and have a private static `TargetSP GetDummyTarget()`?
I personally find that having the target creation inlined in the Debugger constructor is nice: it precludes some other code from setting up a separate dummy target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872