hfinkel added a comment.
I think this is a good idea.
================
Comment at: include/clang/Driver/CC1Options.td:407
def verify : Flag<["-"], "verify">,
- HelpText<"Verify diagnostic output using comment directives">;
+ HelpText<"Similar to -verify=expected">;
def verify_ignore_unexpected : Flag<["-"], "verify-ignore-unexpected">,
----------------
"Similar to" seems unfortunately vague. Can it say, "Equivalent to ..."?
================
Comment at: lib/Frontend/CompilerInvocation.cpp:1081
+ [](char C){return !isAlphanumeric(C)
+ && C!='-' && C!='_';});
+ if (BadChar != Prefix.end() || !isLetter(Prefix[0])) {
----------------
I'd prefer to have spaces around the != operators.
================
Comment at: lib/Frontend/CompilerInvocation.cpp:1095
+ if (Diags) {
+ Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix;
+ Diags->Report(diag::note_drv_verify_prefix_unique);
----------------
If this were going to be an error, then it should have error message that
explains the problem (e.g., duplicate --verify prefix). I don't believe that we
should make this an error (i.e., I think that we should allow duplicate
--verify options with the same prefixes).
================
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:233
// Return true if string literal is found.
// When true, P marks begin-position of S in content.
+ bool Search(StringRef S, bool EnsureStartOfWord = false,
----------------
Please document here what FinishDirectiveWord means (and, while you're at it,
documenting what EnsureStartOfWord means would be nice too).
================
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:370
+ bool NoDiag = false;
+ {
+ StringRef DType;
----------------
This block is to limit the scope of the DType StringRef? That doesn't seem
worthwhile.
https://reviews.llvm.org/D39694
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
