[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:315
 
+      llvm::StringRef name = m_name.GetOptionValue().GetCurrentValueAsRef();
+      if (!name.empty())
----------------
nit:


================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:520
     if (args.GetArgumentCount() == 1) {
       const char *target_idx_arg = args.GetArgumentAtIndex(0);
+      uint32_t target_idx = LLDB_INVALID_INDEX32;
----------------
Maybe we can rename `target_idx_arg` to something like `target_identifier`? Since it's not necessarily an index anymore.


================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:546-551
+            if (!name.empty()) {
+              if (name == target_idx_arg) {
+                target_idx = i;
+                break;
+              }
+            }
----------------
nit:

Also, do we need the empty check? Or is it possible that `target_idx_arg` could be an empty string?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151859