[Lldb-commits] [PATCH] D145609: [lldb] Change dwim-print to default to disabled persistent results

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added a comment.

> Are you using persistent results? If not, how much effort is it to either 1) change the tools/code that examine the output to not look for $\d+, or 2) use a custom print/p alias? Honest question.

I'm not using them myself, and it's not much effort to fix the problems I've seen because of this. It just seemed weird to have a user-facing behavior change like this being committed without a rationale given.

I assumed switching to `dwim-print` would be transparent for users, and I'm not sure how a user that relies on these convenience variables is supposed to discover what happened when `print` stops producing them.

> I'd be curious to learn about some example you have in mind here.

We have a python test harness that drives a debugger session that we mainly use for prettyprinter tests. Then we have tests that look like:

  self.ContinueTo('StopHereForDebugger')
  self.TestPrintOutputMatches('my_variable', 'Cord of length 4: "foo\\n"')

when I ported this from gdb I couldn't find an equivalent to gdb's `output` command (that prints just the value), so I reached for `print` which printed `(Type) $0 = value` instead, and kept everything after the ' = '. It wasn't even using the persistent values, but the change in the output format broke it.

I also found a failure in a one-off test that worked as a minimal sanity check for debug builds: build a debug binary, then run the debugger on it and try to print some variables in order to verify that debug builds do have debug info that the debugger can use (i.e. debug info is not completely missing, or corrupted). This is a shell script that basically does:

  OUTPUT=$("$LLDB" -b -s "$SCRIPT_FILE" "$TEST_PROGRAM")

and then used the conveniently sequentially-numbered persistent results to grep the output:

  # Check that `argc` is 1 and has type `int`.
  (echo "$OUTPUT" | grep "(int).*[$]0 = 1") || die

I know they're not the most robust tests in the world, and as I said, it was not much effort to fix (they're already fixed!). But it still feels like a user-facing regression, and the consistency argument can go both ways: you could similarly argue that `dwim-print` should put the result of `frame variable` into a convenience variable to keep it consistent with the cases where the expression evaluator is used. So the real argument is that most users don't need it and it shouldn't be enabled by default. Which is a fine argument to make (although it's sad that we don't have data to support it one way or the other), but I had to ask to know because the commit description didn't mention any reasons at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145609