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

Wed Jan 9 07:25:30 PST 2019

probinson 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

----------------
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.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55825