[PATCH] D79375: [FileCheck] Make invalid prefix diagnostics more precise

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: llvm/lib/Support/FileCheck.cpp:1905
+
+static const char *DefaultCheckPrefixes[] = {"CHECK"};
 
----------------
jdenny wrote:
> jhenderson wrote:
> > This is only used in one place. Why not just explicitly provide it there? Alternatively, should this be defined in the top-level bit of FileCheck, so that client libraries can have different default check prefixes?
> > This is only used in one place. Why not just explicitly provide it there?
> 
> I changed it.  It was that way because I extracted it from D79276, which still uses it in two places.
> 
> > Alternatively, should this be defined in the top-level bit of FileCheck, so that client libraries can have different default check prefixes?
> 
> Perhaps so.  But I feel like that's an orthogonal issue and belongs in a separate patch.  What do you think?
I don't think we should allow customized default directive until the need arises.


================
Comment at: llvm/lib/Support/FileCheck.cpp:1891-1893
+      errs() << "error: supplied check prefix must start with a letter and "
+             << "contain only alphanumeric characters, hyphens, and "
+             << "underscores: '" << Prefix << "'\n";
----------------
Could you make ValidateCheckPrefix throw a DiagnosticError and print the error here?


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

https://reviews.llvm.org/D79375