[PATCH] D98699: [dexter] Fix DexLimitSteps when breakpoint can't be set at requested location

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

In D98699#2644229 <https://reviews.llvm.org/D98699#2644229>, @TWeaver wrote:

> Hey Orlando! thanks for this. A really nice, solid reimplementation and improvement of the original work. Great code comments too.
>
> thanks again, LGTM.

Thanks @TWeaver! And thank you everyone for the reviews.



================
Comment at: debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py:99
         self.debugger.launch()
-        time.sleep(self._pause_between_steps) 
+        time.sleep(self._pause_between_steps)
         while not self.debugger.is_finished:
----------------
Orlando wrote:
> chrisjackson wrote:
> > _pause_between_steps seems odd. Maybe 'time_between_steps_ms' is more decriptive. I realise that variable wasn't introduced in this patch
> I agree that the name could be better. It looks like the name is used elsewhere too though. In `ConditionalController.__init__` we have
> `self._pause_between_steps = context.options.pause_between_steps`.  I think it makes sense to keep the name the same as other uses, and I'm not sure updating the name elsewhere should be in the scope of this patch. So I don't think it's worth changing here. Wdyt?
> 
> 
I landed this just now, and in coming back to comment realise that we didn't resolve our conversation @chrisjackson, sorry! Happy to follow up with a quick patch to address this if you still think it should be changed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98699