[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=<prefixes>

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I've not done a detailed review of the string manipulation here, but this looks like a great feature, thanks!



================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:342
+
+def note_drv_verify_prefix_spelling : Note<"-verify prefixes must start with a letter and contain only alphanumeric characters, hyphens, and underscores">;
 }
----------------
Please wrap this to 80 columns.


================
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">,
----------------
jdenny wrote:
> hfinkel wrote:
> > "Similar to" seems unfortunately vague. Can it say, "Equivalent to ..."?
> I agree I should have made it clearer.
> 
> "Equivalent to -verify=expected"  works if we decide that duplicate explicit prefixes are permitted, as you've suggested in a later comment.
> 
> With the current implementation, it should be "All occurrences together are equivalent to one occurrence of -verify=expected".  That is, I chose to permit multiple occurrences of -verify without explicit prefixes for backward compatibility, but I chose not to permit duplicate explicit prefixes for reasons I'll discuss in the other comment.
> 
> I'll clean up the documentation once we agree on the right behavior.
> 
> 
I don't think we need to worry about backwards compatibility with people passing `-verify` more than once; it seems OK to disallow that if we need to.


https://reviews.llvm.org/D39694