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

[Joel E. Denny via Phabricator via llvm-commits]

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


================
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";
----------------
thopre wrote:
> jdenny wrote:
> > thopre wrote:
> > > Could you make ValidateCheckPrefix throw a DiagnosticError and print the error here?
> > That seems ok, but what's the motivation?  Is it a preferred style, or is there a technical advantage?
> Future proofing: the function could start getting used elsewhere so having the error message there makes sense. Feel free to disagree.
So the goal is to put the diagnostic that describes the constraint in the function that checks the constraint.  Makes sense to me.

Do you think it's worthwhile for `ValidateCheckPrefix` to be a separate function? It's so small, I'm thinking it does more to obfuscate the logic in `ValidatePrefixes` than to improve its readability.


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

https://reviews.llvm.org/D79375