[PATCH] D142099: 'on_line' parameter added to DexExpectStepOrder in the Dexter syntax

Orlando Cazalet-Hyams via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 19 03:17:21 PST 2023


Orlando added a comment.

We do need to refactor and clean up the delineation between command line of declaration and target line in the code at some point. This is fine in the meantime though and the patch looks good.

LGTM (with an inline question), thank you @BenJMudd!



================
Comment at: cross-project-tests/debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ControllerHelpers.py:22-23
 def have_hit_line(watch, loc):
-  if hasattr(watch, '_on_line'):
-    return watch._on_line == loc.lineno
+  if hasattr(watch, 'on_line'):
+    return watch.on_line == loc.lineno
   elif hasattr(watch, '_from_line'):
----------------
This change seems a little bit suspicious - as mentioned offline perhaps this was doing the wrong thing before though. It does certainly look like no command classes ever set `_on_line`, so it looks like this is doing the right thing with your patch.

Have you checked the llvm-lit tests in `cross-project-tests/debuginfo-tests/dexter/feature_tests`  pass (preferably on Linux as it has greater coverage)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142099



More information about the llvm-commits mailing list