[PATCH] D90281: [FileCheck] Report missing prefixes when more than one is provided.

Mon Nov 2 10:28:28 PST 2020

mtrofin marked an inline comment as done.
mtrofin added inline comments.

================
Comment at: llvm/test/FileCheck/allow-unused-prefixes.txt:1
+; RUN: not FileCheck --allow-unused-prefixes=false --check-prefixes=P1,P2 --input-file %S/Inputs/one-check.txt %S/Inputs/one-check.txt 2>&1 | FileCheck --check-prefix=MISSING-ONE %s
+; RUN: not FileCheck --allow-unused-prefixes=false --check-prefixes=P1,P2,P3 --input-file %S/Inputs/one-check.txt %S/Inputs/one-check.txt 2>&1 | FileCheck --check-prefix=MISSING-MORE %s
----------------
mtrofin wrote:
> dblaikie wrote:
> > mtrofin wrote:
> > > jhenderson wrote:
> > > > Should this test be using `%ProtectFileCheckOutput`?
> > > Not sure why, we're not setting env variables, or am I missing something?
> > The test isn't setting environment variables, but users can (see: https://reviews.llvm.org/D65121 ) to configure FileCheck's behavior for their convenience/to produce more actionable errors (I think this is to allow customization both by users, but also on buildbots so buildbots fail with more context that makes it easier to debug from the logs when you can't necessarily reproduce the command locally). So any test that relies on the output of FileCheck probably wants to ensure FILECHECK_OPTS can't effect the output of the FileCheck command under test and can/should do so using `%ProtectFileCheckOutput`
> Thanks for the explanation, that is the bit that I couldn't grok from %ProtectFileCheckOutput, somehow I read "developers" as "developers writing tests and setting variables in their tests' RUN line". 
> 
https://reviews.llvm.org/D90631

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90281