User Details
- User Since
- Jan 31 2016, 7:15 AM (328 w, 6 d)
Yesterday
LGTM
Please let me know if you need someone to land this on your behalf.
Thanks!
LGTM
No worries, thank you for your patience. This LGTM.
LGTM. Passes the test suite on macOS.
Thu, May 19
Wed, May 18
LGTM with the decorator
Separating the notion of an expression and an expression list seems worthwhile. I did a very high level pass but I'll take a more detailed look tomorrow.
LGTM
Still a few places where we have a dedicated assert
LGTM
LGTM
Tue, May 17
Minor nit but LGTM
Mon, May 16
LGTM
Sat, May 14
Fri, May 13
LGTM
LGTM
LGTM
LGTM
PS: Small typo in your summary: s/abot/about/.
Wed, May 11
Fri, May 6
Thu, May 5
Does that mean that the interactive crashlogs (crashlog -i) do this correctly?
I'm (still) very excited about this. I'm really happy that this lives behind a flag which addresses most of my original concerns. Making this the default in dsymutil would require a bunch of qualification but this approach allows me to do that gradually. One thing I suggested earlier I think was to run the lldb test suite with the new flag and see if that breaks anything. Did you ever get around to that? That should give us an initial level of confidence. Jim expressed some concerns about trying to squeeze everything in the same compilation unit. Greg, do you share that concern? Do we expect this will negatively impact lldb's performance? It would probably be worthwhile to do a few performance experiments here as well to make sure this doesn't regress debugging performance.
When you used this to compare the output of dsymutil changes, what was the input binary? Does this generate meaningful output for bigger programs like dsymutil itself?
This looks reasonable to me, but I'm not sure if there's anything special about the executable module that would result in us doing the wrong thing. I'm sure Jim or Pavel will know.
Wed, May 4
I'm far from an expert on this but the change makes sense to me and has good test coverage. LGTM.
LGTM
Tue, May 3
IsInteractiveSession -> IsInteractiveGraphicSession
Typo
Mon, May 2
Can we add a pexpect test for this?
Fri, Apr 29
Could this go in LLDBStandalone.cmake?
Good catch. Seems like there's another reference to this in llvm/utils/gn/secondary/lldb/include/lldb/Version/BUILD.gn but otherwise LGTM.
Thu, Apr 28
LGTM