[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

Mon Jul 20 02:24:48 PDT 2020

labath accepted this revision.
labath added a subscriber: aprantl.
labath added a comment.
This revision is now accepted and ready to land.

In D83425#2161255 <https://reviews.llvm.org/D83425#2161255>, @bbli wrote:

> > Setting this no longer makes sense, as it will always be empty. Please remove that. Maybe also rename stdout_content to indicate it also contains stderr. Just plain "output" might suffice?
>
> Ok, deleted it and changed "stdout_content" to "combined_content".

Thanks this looks good now. I presume you don't have commit access and need someone to commit this for you?

>> this error result also doesn't make sense. It looks like you'll also need to update the usage in `getCompilerVersion`
> 
> Ohh your right. I should have check whether other functions depended on stderr stream. So checking the rest of the codebase, it seems the output of `system` is also used in `TestDataFormatterSkipSummary.py`, but b/c it only triggers on Macs, I didn't see a failure when I made the change.

LOL, I wonder when was the last time anyone ran the lldb test suite against gcc on a mac. @aprantl, can we just delete the version check blurb in `TestDataFormatterSkipSummary.py`?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83425