This revision was automatically updated to reflect the committed changes.
Closed by commit rC320908: [VerifyDiagnosticConsumer] support
-verify=prefixes (authored by hfinkel, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D39694
Files:
jdenny added a comment.
In https://reviews.llvm.org/D39694#956661, @hfinkel wrote:
> LGTM
Thanks for accepting. I don't have commit privileges. Would you please commit
for me? This depends on https://reviews.llvm.org/D40995, which also needs to
be committed.
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D39694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
jdenny marked an inline comment as done.
jdenny added inline comments.
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:398
+// DToken is foo-bar-warning, but foo is the only -verify prefix).
+if (Prefixes.end() == std::find(Prefixes.begin(), Prefixes.end(),
jdenny updated this revision to Diff 127088.
jdenny added a comment.
Herald added a subscriber: mgrang.
1. Use std::binary_search, as suggested by Hal.
2. Fix another case of line wrapping.
3. Rebase onto a recent master, and remove rewrites of tests that have recently
changed.
hfinkel added inline comments.
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:398
+// DToken is foo-bar-warning, but foo is the only -verify prefix).
+if (Prefixes.end() == std::find(Prefixes.begin(), Prefixes.end(), DToken))
+ continue;
>
jdenny updated this revision to Diff 126086.
jdenny added a comment.
This update does the following:
1. Fixes the line wrapping that Richard pointed out.
2. Converts from std::vector to std::set for more efficient prefix lookup.
3. Grows a dependence on https://reviews.llvm.org/D40995 (which
jdenny added a comment.
In https://reviews.llvm.org/D39694#949066, @rsmith wrote:
> I've not done a detailed review of the string manipulation here, but this
> looks like a great feature, thanks!
Hi Richard. Thanks for your feedback. I'll fix the line wrapping you pointed
out.
jdenny marked 13 inline comments as done.
jdenny added a comment.
I marked the comments related to Hal's suggestions as done to avoid confusion
for future reviews. I'm not used to using this sort of tool for reviews.
Hopefully it's appropriate for the author to do that rather than the
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
jdenny updated this revision to Diff 125640.
jdenny edited the summary of this revision.
jdenny added a comment.
This update includes all of Hal's suggestions.
I'm also thinking of converting prefix storage from a std::vector to a std::map
so that lookup should be faster during parsing.
jdenny added a comment.
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);
hfinkel wrote:
> jdenny
hfinkel added inline comments.
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);
jdenny wrote:
>
jdenny added a comment.
In https://reviews.llvm.org/D39694#944642, @hfinkel wrote:
> I think this is a good idea.
Thanks for the review. I've replied to each comment and will make revisions
after we agree on the behavioral issue you raised.
Comment at:
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
jdenny updated this revision to Diff 125223.
jdenny added a comment.
Rebased on master/trunk fetched today.
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticOptions.h
include/clang/Driver/CC1Options.td
jdenny updated this revision to Diff 123027.
jdenny added a comment.
1. Capitalized some of the new local variables according to coding standards.
2. Rebased on master/trunk fetched today.
https://reviews.llvm.org/D39694
Files:
include/clang/Basic/DiagnosticDriverKinds.td
jdenny updated this revision to Diff 122172.
jdenny retitled this revision from "[VerifyDiagnosticConsumer] support
-verify=PREFIX" to "[VerifyDiagnosticConsumer] support -verify=".
jdenny edited the summary of this revision.
jdenny added a comment.
1. Extended -verify to accept multiple
18 matches
Mail list logo
