Improve error message of FileCheck when stdin is empty

[davidb]

Summary

Replace '-' in the error message with <stdin>. This is also consistent with another error message in the code.

[davidb]

llvm/utils/FileCheck/FileCheck.cpp

619

I found it the least intrusive and confusing to change the InputFilename of stdin here, than in the other 3 places that the InputFilename is printed.

[jhenderson]

I have a vague and probably unreasonable concern that changing from the beahviour for a filename "<stdin>" could cause somebody some confusion somewhere, since it's non-standard. I do agree with changing the error message though (we do similar things already in other tools, IIRC). What are others' thoughts?

[grimar]

In D73793#1854135, @jhenderson wrote:

I have a vague and probably unreasonable concern that changing from the beahviour for a filename "<stdin>" could cause somebody some confusion somewhere, since it's non-standard.

FWIW on windows file names seems can't contain "<" or ">" characters. On ubuntu it seems to be OK.
But my non-very-strong-but-still-feeling here is that your concern is valid.

I do agree with changing the error message though (we do similar things already in other tools, IIRC).

Improving the error message sounds good to me too.

[arichardson]

llvm/utils/FileCheck/FileCheck.cpp

619

Instead of changing the option default to "<stdin>, may could you add:

if (InputFilename == "-")
  InputFilename = "<stdin>"; // Overwrite for improved diagnostic messages

Just after the the getFileOrSTDIN() call?

[davidb]

• review comments

[davidb]

In D73793#1854135, @jhenderson wrote:

I have a vague and probably unreasonable concern that changing from the beahviour for a filename "<stdin>" could cause somebody some confusion somewhere, since it's non-standard. I do agree with changing the error message though (we do similar things already in other tools, IIRC). What are others' thoughts?

The initial change would never treat <stdin> as a file name, it would special case this on open and use the correct - value. We only try to open a file once, but use the filename in multiple places, hence this to me was the simplest way to achieve this.

I've updated the diff based on the review comments, which changes the name of the file after opening. I like this less as it changes the state, and in my opinion just as confusing, if not a bit more vulnerable... However I also recognise that the size of this file is small, it appears to be reasonably tested, and I am not likely to be making many more changes to this file in the immediate future, so I can tolerate this.

[jhenderson]

This looks fine to me, although having looked at what other tools do, it might be better to create a reportError type of function that takes an input filename as an argument and does the conversion from '-' if needed. See for example llvm-readobj.cpp reportWarning etc. I'm happy either way, as a quick glance suggests this may not be straightforward here.

llvm/test/FileCheck/check-empty.txt

 //
 // RUN: %ProtectFileCheckOutput \
 // RUN: FileCheck -check-prefix=NOFOO -allow-empty %s </dev/null 2>&1 \
 // RUN: | FileCheck -allow-empty -check-prefix=NO-EMPTY-ERR %s
 
 ; FOO: foo
 ; NOFOO-NOT: foo
 
-; EMPTY-ERR: FileCheck error: '-' is empty.
+; EMPTY-ERR: FileCheck error: '<stdin>' is empty.
 ; EMPTY-ERR-NEXT: FileCheck command line: {{.*}}{{F|f}}ile{{C|c}}heck{{.*}}-check-prefix={{.*}}FOO {{.*}}check-empty.txt
-; NO-EMPTY-ERR-NOT: FileCheck error: '-' is empty.
+; NO-EMPTY-ERR-NOT: FileCheck error: '<stdin>' is empty.
 ; NOT-FOUND: error: FOO: expected string not found in input

llvm/utils/FileCheck/FileCheck.cpp

static cl::extrahelp FileCheckOptsEnv(
     "\nOptions are parsed from the environment variable FILECHECK_OPTS and\n"
     "from the command line.\n");
 
 static cl::opt<std::string>
     CheckFilename(cl::Positional, cl::desc("<check-file>"), cl::Optional);
 
 static cl::opt<std::string>
     InputFilename("input-file", cl::desc("File to check (defaults to stdin)"),
-                  cl::init("-"), cl::value_desc("filename"));
+                  cl::init("<stdin>"), cl::value_desc("filename"));
 
 static cl::list<std::string> CheckPrefixes(
     "check-prefix",
     cl::desc("Prefix to use from check file (defaults to 'CHECK')"));
 static cl::alias CheckPrefixesAlias(
     "check-prefixes", cl::aliasopt(CheckPrefixes), cl::CommaSeparated,
     cl::NotHidden,
     cl::desc(
SM.AddNewSourceBuffer(MemoryBuffer::getMemBuffer(
                             CheckFileText, CheckFile.getBufferIdentifier()),
                         SMLoc());
 
   if (FC.readCheckFile(SM, CheckFileText, PrefixRE))
     return 2;
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr<std::unique_ptr<MemoryBuffer>> InputFileOrErr =
-      MemoryBuffer::getFileOrSTDIN(InputFilename);
+      MemoryBuffer::getFileOrSTDIN(
+          (InputFilename == "<stdin>" ? "-" : InputFilename.getValue()));
   if (std::error_code EC = InputFileOrErr.getError()) {
     errs() << "Could not open input file '" << InputFilename
            << "': " << EC.message() << '\n';
     return 2;
   }
   MemoryBuffer &InputFile = *InputFileOrErr.get();
 
   if (InputFile.getBufferSize() == 0 && !AllowEmptyInput) {
int main(int argc, char **argv) {
   int ExitCode = FC.checkInput(SM, InputFileText,
                                DumpInput == DumpInputNever ? nullptr : &Diags)
                      ? EXIT_SUCCESS
                      : 1;
   if (DumpInput == DumpInputAlways ||
       (ExitCode == 1 && DumpInput == DumpInputFail)) {
     errs() << "\n"
            << "Input file: "
-           << (InputFilename == "-" ? "<stdin>" : InputFilename.getValue())
+           << InputFilename
            << "\n"
            << "Check file: " << CheckFilename << "\n"
            << "\n"
            << "-dump-input=help describes the format of the following dump.\n"
            << "\n";
     std::vector<InputAnnotation> Annotations;
     unsigned LabelWidth;
     BuildInputAnnotations(Diags, Annotations, LabelWidth);
     DumpAnnotatedInput(errs(), Req, InputFileText, Annotations, LabelWidth);
   }
 
   return ExitCode;
 }