[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/conflicts/TestStdModuleWithConflicts.py:35
+
+        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
----------------
teemperor wrote:
> aprantl wrote:
> > teemperor wrote:
> > > aprantl wrote:
> > > > what's that for?
> > > I think that's how we set our executable as the target? It's frankly cargo-culted setup code that we use in a few hundred other tests.
> > lldbutil.run_break_set_by_file_and_line should be all you need. It has a.out as a default argument. So I don't think you need this line or the one defining exe, or the dictionary=... argument to build().
> When I remove this line, I actually get `AssertionError: False is not True : Expecting 1 locations, got 0` from the `lldbutil.run_break_set_by_file_and_line`.
> 
> I can't remember if I added the `dictionary=` argument for a specific reason or if that was just a side effect when debugging tests. Doesn't seem to break anything though, so I'll remove it.
Interesting. Looks like there are several very similarly named helper functions in lldbutil. There are several testcases that only use `run_to_source/line_breakpoint without running `file` manually, so perhaps those functions do more work than run_break_set_by_file_and_line. Since the line number comes from a regex anyway, I'd recommend using `run_to_source_breakpoint instead and also deleting the regex matching from setUp().


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

https://reviews.llvm.org/D58125