[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

My main question is why do we need the separate `SBDebugger::RunReplay` API for this. Shouldn't the record/replay be as transparent as possible? I would expect that `SBDebugger::RunCommandInterpreter` will detect that it is running in replay mode and use the recorded input command stream instead of the real one.



================
Comment at: source/Interpreter/CommandInterpreter.cpp:131
+    // Explicitly ignore reproducer commands.
+    if (command.find("reproducer") == 0)
+      return;
----------------
This seems pretty hacky, and easy to break in multiple ways:
- `alias rep reproducer`; `rep generate`
- `file /tmp/reproducer/bin/lldb`
Couldn't you just have the actual reproducer CommandObject check what mode we're in, and behave like a nop if there is nothing to do? (So, the commands would still be captured, but they wouldn't do anything when replaying.)


================
Comment at: source/Interpreter/CommandInterpreter.cpp:2154
+  options.SetSilent(false);
+  options.SetStopOnError(true);
+  options.SetStopOnContinue(false);
----------------
Is this correct? What if the command originally has ended with an error (perhaps because the user made a typo)? Will that abort the replay here?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55582