clayborg added inline comments.

================
Comment at: lldb/source/Commands/CommandObjectThread.cpp:2201
+    // command
+    if (cmd.find(" repeat") == std::string::npos)
+      cmd += " repeat";
----------------
This " repeat" is pretty hacky here. If we can't get away without adding 
anything, I would prefer the --continue option we had before


================
Comment at: lldb/source/Commands/CommandObjectThread.cpp:2254-2256
+      m_options.m_dumper_options.skip = 1;
+      m_options.m_dumper_options.id = m_last_id;
     }
----------------
to make repeat work, you are filling in the dumper options, can't we just store 
a copy of the old thread options and update the "id" field?

You also mentioned:



> I couldn't find a simple way to create the repeat command directly from 
> GetRepeatCommand because it's not easy to quickly find what the next 
> instruction is going to be in the repeat command. In fact, finding the id of 
> that future instruction requires to actually traverse the instructions up to 
> that point, but the traversal only happens in DoExecute, which is invoked 
> after GetRepeatCommand. As a simple workarou nd, I'm adding a " repeat" 
> positional argument that won't be parsed by CommandOptions but will still 
> indicate that a repeat is happening. I was able to remove the --continue flag 
> from Options.td, thus reducing the amount of code needed to handle the 
> repeat. Not only that, I was able to get rid of the map<tid, 
> TraceInstructionDumper> that I had in the CommandObject that I was using to 
> continue the iteration in the repeat commands. Now I'm using the last 
> iterated id to computed where the next one will be, thus creating a new brand 
> new dumper with each command making this class simpler.

You seem to have the right "id" to start at here with m_last_id? Why can't we 
just store this in an extra copy of TraceInstructionDumperOptions and have it 
create the right command?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122254

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to