[PATCH] D49328: [FileCheck] Provide an option for FileCheck to dump original input to stderr on failure

[George Karpenkov via Phabricator via llvm-commits]

george.karpenkov added inline comments.


================
Comment at: llvm/utils/FileCheck/FileCheck.cpp:1515
+  errs() << "Var value = " << std::getenv(DumpInputEnv);
+  if (Out == 1 && std::getenv(DumpInputEnv))
+    errs() << "Full input:\n" << InputFileText;
----------------
thopre wrote:
> Out is a boolean, why not just "if (out && ...)"?
It's an exit code. Saying `if (out)` seems to have wrong connotations.
Renamed and change type.


================
Comment at: llvm/utils/FileCheck/FileCheck.cpp:1516
+  if (Out == 1 && std::getenv(DumpInputEnv))
+    errs() << "Full input:\n" << InputFileText;
+
----------------
dblaikie wrote:
> Perhaps some clear markers should be used before/after the output so it can be easily cut out by tools (like buildbot) or eyeballed by users?
Seems like a great idea!


================
Comment at: llvm/utils/lit/lit/TestingConfig.py:29
+                     'TEMPDIR', 'AVRLIT_BOARD', 'AVRLIT_PORT',
+                     'FILECHECK_DUMP_INPUT_ON_FAILURE']
         for var in pass_vars:
----------------
dblaikie wrote:
> I expect this probably shouldn't be the default - maybe on buildbots, but probably not for the average user. And if it should be the default, maybe FileCheck itself should have it as the default, rather than lit here?
> 
> Probably worth an llvm-dev discussion if the default is going to change in FileCheck or in lit. (I know there was a discussion about adding the feature - but changing the default behavior is a bit more involved)
This code is not setting the default.
It just tells LIT to propagate the environment variable.
Otherwise, it will not get to FileCheck.


https://reviews.llvm.org/D49328