[PATCH] D55825: [FileCheck] Suppress old -v/-vv diags if dumping input

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added inline comments.


================
Comment at: llvm/test/FileCheck/dump-input-enable.txt:46
+; RUN:           -match-full-lines -dump-input=never -v 2>&1 \
+; RUN: | FileCheck %s -match-full-lines -check-prefixes=TRACE,NODUMP
 
----------------
probinson wrote:
> jdenny wrote:
> > probinson wrote:
> > > Offhand it looks like every use of `-dump-input` in this file now also has `-v` which seems excessive. Also like you're losing coverage of that combination. Think about using `-v` more judiciously here.
> > > Offhand it looks like every use of -dump-input in this file now also has -v which seems excessive.
> > 
> > The purpose of this test file is to check the effects of various ways of disabling/enabling input dumps.  Checking for a trace seems important because disabling/enabling input dumps in any manner is now intended to permit/suppress the trace.  I don't see a reason to skip that check for just some ways of disabling/enabling input dumps.
> > 
> > > Also like you're losing coverage of that combination.
> > 
> > Combinations without -v?  I feel like related dimensions of functionality are tested well elsewhere, such as verbose.txt and dump-input-annotations.txt.
> > 
> > > Think about using -v more judiciously here.
> > 
> > If the above explanation of my testing strategy isn't satisfying, please explain your objection a little more.  Maybe I'm misunderstanding.
> verbose.txt looks at -v/-vv without -dump-input.  dump-input-annotations.txt uses only -dump-input=always and not the other flavors.  The behavior of -dump-input=never/fail without -v is not verified anywhere that I can find.
Ah, I see.  I don't want to lose coverage of trace suppression by -dump-input, so I'll add a few new checks here for the cases you've mentioned.  Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55825