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

2020-11-05 Thread Vedant Kumar via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG16e5a347e70b: [TargetList] Simplify dummy target creation (authored by vsk). Changed prior to commit: https://reviews.llvm.org/D90872?vs=303252&id

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

2020-11-05 Thread 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 th

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

2020-11-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: lldb/source/Core/Debugger.cpp:692 +m_dummy_target_sp.reset( +new Target(*this, arch, default_platform_sp, is_dummy_target)); + } Should we move all this into `Target` and have a private static `TargetSP

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

2020-11-05 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lldb/source/Core/Debugger.cpp:685 - m_dummy_target_sp = m_target_list.GetDummyTarget(*this); + { +ArchSpec arch(Target::GetDefaultArchitecture

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

2020-11-05 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Oh, my bad, apparently we were leaving the dummy target out of the TargetList. That's a little odd, but I probably did it so that's not so unusual... Ignore my comments... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90

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

2020-11-05 Thread 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());

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

2020-11-05 Thread Jim Ingham via Phabricator via lldb-commits
jingham 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

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

2020-11-05 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 303252. vsk marked an inline comment as done. vsk added a comment. Make the dummy target per-debugger. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90872/new/ https://reviews.llvm.org/D90872 Files: lldb/include

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

2020-11-05 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked an inline comment as done. 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 a

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

2020-11-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere 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::GetDefaultArchite

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

2020-11-05 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 303223. vsk added a comment. Delete some dead code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90872/new/ https://reviews.llvm.org/D90872 Files: lldb/include/lldb/Target/TargetList.h lldb/source/Target/Targ

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

2020-11-05 Thread Vedant Kumar via Phabricator via lldb-commits
vsk created this revision. vsk added reviewers: teemperor, JDevlieghere, jingham. Herald added a project: LLDB. vsk requested review of this revision. Factor out dummy target creation from CreateTargetInternal. This makes it impossible for dummy target creation to accidentally fail due to too-str