[lldb] Code review follow up

Author: Mikhail Zakharov

lldb/source/Commands/CommandObjectProcess.cpp

@@ -1377,6 +1377,9 @@ class CommandObjectProcessStatus : public CommandObjectParsed {
       case 'v':
         m_verbose = true;
         break;
+      case 'd':
+        m_dump = true;
+        break;
       default:
         llvm_unreachable("Unimplemented option");
       }
@@ -1386,6 +1389,7 @@ class CommandObjectProcessStatus : public CommandObjectParsed {
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_verbose = false;
+      m_dump = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
@@ -1394,6 +1398,7 @@ class CommandObjectProcessStatus : public CommandObjectParsed {
 
     // Instance variables to hold the values for command options.
     bool m_verbose = false;
+    bool m_dump = false;
   };
 
 protected:
@@ -1448,6 +1453,14 @@ class CommandObjectProcessStatus : public CommandObjectParsed {
         crash_info_sp->GetDescription(strm);
       }
     }
+
+    if (m_options.m_dump) {
+      StateType state = process->GetState();
+      if (state == eStateStopped) {
+        ProcessModID process_mod_id = process->GetModID();
+        process_mod_id.Dump(result.GetOutputStream());
+      }
+    }
   }
 
 private:
@@ -1827,33 +1840,6 @@ class CommandObjectMultiwordProcessTrace : public CommandObjectMultiword {
   ~CommandObjectMultiwordProcessTrace() override = default;
 };
 
-// CommandObjectProcessDumpModificationId
-class CommandObjectProcessDumpModificationId : public CommandObjectParsed {
-public:
-  CommandObjectProcessDumpModificationId(CommandInterpreter &interpreter)
-      : CommandObjectParsed(interpreter, "process dump-modification-id",
-                            "Dump the state of the ProcessModID. Intended to "
-                            "be used for debugging LLDB itself.",
-                            "process dump-modification-id",
-                            eCommandRequiresProcess) {}
-
-  ~CommandObjectProcessDumpModificationId() override = default;
-
-protected:
-  void DoExecute(Args &command, CommandReturnObject &result) override {
-    Process *process = m_exe_ctx.GetProcessPtr();
-    StateType state = process->GetState();
-    if (state != eStateStopped) {
-      result.SetStatus(eReturnStatusFailed);
-      return;
-    }
-
-    ProcessModID process_mod_id = process->GetModID();
-    process_mod_id.Dump(result.GetOutputStream());
-    result.SetStatus(eReturnStatusSuccessFinishResult);
-  }
-};
-
 // CommandObjectMultiwordProcess
 
 CommandObjectMultiwordProcess::CommandObjectMultiwordProcess(
@@ -1893,9 +1879,6 @@ CommandObjectMultiwordProcess::CommandObjectMultiwordProcess(
   LoadSubCommand(
       "trace",
       CommandObjectSP(new CommandObjectMultiwordProcessTrace(interpreter)));
-  LoadSubCommand(
-      "dump-modification-id",
-      CommandObjectSP(new CommandObjectProcessDumpModificationId(interpreter)));
 }
 
 CommandObjectMultiwordProcess::~CommandObjectMultiwordProcess() = default;

lldb/source/Commands/Options.td

@@ -784,6 +784,8 @@ let Command = "process handle" in {
 let Command = "process status" in {
   def process_status_verbose : Option<"verbose", "v">, Group<1>,
     Desc<"Show verbose process status including extended crash information.">;
+  def process_status_dump : Option<"dump-modification-id", "d">, Group<1>,
+    Desc<"Dump the state of the ProcessModID of the stopped process.">;
 }
 
 let Command = "process save_core" in {

lldb/source/Target/TargetProperties.td

@@ -298,7 +298,7 @@ let Definition = "process" in {
     Desc<"Debugger's behavior upon fork or vfork.">;
   def TrackMemoryCacheChanges: Property<"track-memory-cache-changes", "Boolean">,
     DefaultTrue,
-    Desc<"If true, memory cache modifications (which happen often during expressions evaluation) will bump process state ID (and invalidate all synthetic children). Disabling this option helps to avoid synthetic children reevaluation when pretty printers heavily use expressions, but convenience variables won't reevaluate synthetic children automatically.">;
+    Desc<"If true, memory cache modifications (which happen often during expressions evaluation) will bump process state ID (and invalidate all synthetic children). Disabling this option helps to avoid synthetic children reevaluation when pretty printers heavily use expressions. The downside of disabled setting is that convenience variables won't reevaluate synthetic children automatically.">;
 }
 
 let Definition = "platform" in {

lldb/test/Shell/Expr/TestProcessModificationIdOnExpr.cpp

@@ -7,15 +7,15 @@
 // RUN: %lldb %t \
 // RUN:   -o "settings set target.process.track-memory-cache-changes false" \
 // RUN:   -o "run" \
-// RUN:   -o "process dump-modification-id" \
+// RUN:   -o "process status -d" \
 // RUN:   -o "expr x.i != 42" \
-// RUN:   -o "process dump-modification-id" \
+// RUN:   -o "process status -d" \
 // RUN:   -o "expr x.get()" \
-// RUN:   -o "process dump-modification-id" \
+// RUN:   -o "process status -d" \
 // RUN:   -o "expr x.i = 10" \
-// RUN:   -o "process dump-modification-id" \
+// RUN:   -o "process status -d" \
 // RUN:   -o "continue" \
-// RUN:   -o "process dump-modification-id" \
+// RUN:   -o "process status -d" \
 // RUN:   -o "exit" | FileCheck %s -dump-input=fail
 
 class X {
@@ -34,34 +34,34 @@ int main() {
   return 0;
 }
 
-// CHECK-LABEL: process dump-modification-id
+// CHECK-LABEL: process status -d
 // CHECK: m_stop_id: 2
 // CHECK: m_memory_id: 0
 
 // CHECK-LABEL: expr x.i != 42
 // IDs are not changed when executing simple expressions
 
-// CHECK-LABEL: process dump-modification-id
+// CHECK-LABEL: process status -d
 // CHECK: m_stop_id: 2
 // CHECK: m_memory_id: 0
 
 // CHECK-LABEL: expr x.get()
 // Expression causes ID to be bumped because LLDB has to execute function
 
-// CHECK-LABEL: process dump-modification-id
+// CHECK-LABEL: process status -d
 // CHECK: m_stop_id: 3
 // CHECK: m_memory_id: 1
 
 // CHECK-LABEL: expr x.i = 10
 // Expression causes MemoryID to be bumped because LLDB writes to non-cache memory
 
-// CHECK-LABEL: process dump-modification-id
+// CHECK-LABEL: process status -d
 // CHECK: m_stop_id: 3
 // CHECK: m_memory_id: 2
 
 // CHECK-LABEL: continue
 // Continue causes StopID to be bumped because process is resumed
 
-// CHECK-LABEL: process dump-modification-id
+// CHECK-LABEL: process status -d
 // CHECK: m_stop_id: 4
 // CHECK: m_memory_id: 2