[PATCH] D81422: Change filecheck default to dump input on failure

Fri Jun 12 14:14:51 PDT 2020

jdenny added a comment.

In D81422#2090725 <https://reviews.llvm.org/D81422#2090725>, @mehdi_amini wrote:

> In D81422#2090618 <https://reviews.llvm.org/D81422#2090618>, @jdenny wrote:
>
> > In D81422#2090425 <https://reviews.llvm.org/D81422#2090425>, @mehdi_amini wrote:
> >
> > > I'm thinking about a possible improvement here: we could have FileCheck dump the input for the current CHECK-LABEL section only: it seems like it could reduce the output drastically while still preserving how useful the information is?
> >
> >
> > One FileCheck invocation can report multiple errors, one per CHECK-LABEL section, and I prefer to see all errors every time.
>
>
> Yeah I had in mind to show all the errors for the current section, then the input for the section, before moving to the next section.

I'm not sure I understand. Currently, (1) FileCheck prints all errors up front in case that's sufficient without the dump, and then (2) FileCheck prints the dump with annotations showing the errors again (and successful directives, including labels, if -vv).  Are you talking about interleaving 1 and 2?  Does that actually make it easier to read?  Maybe just omit lines from the dump instead?

>> I also prefer to see the entire input dump (with annotations produced by -vv) as sometimes the error FileCheck reports is far away from the actual error.  For example, maybe the CHECK-LABEL directives matched text at unexpected locations in the input, and then the directives in their sections failed as a result. It might be hard to determine that the CHECK-LABEL directives were at fault without seeing the text it should have matched outside of its (incorrect) section.
> 
> Sure: but that seems like not the most common case to me: if we start by showing the section we matched it provides already a pretty good indication.
> 
>> If you choose to limit the dump to one CHECK-LABEL section, please make that behavior optional via a command-line option (which can be set via FILECHECK_OPTS).
> 
> I thought it would be a better default actually, leaving the full-full as opt-in

I'm not sure how common it is, and I don't know what the default should be.  From the command line, I can always run again with different options to get missing info.  From the bots, it's harder to get missing info (which is why I suggested -vv for the bots).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81422