Oh, yeah, that should probably
be clamped.
I didn’t have a specific use case in mind, but in general I find that tools
that emit “richer” information can be used for more things, hence returning
the count, and not some fixed value.
On Sat, Jun 13, 2020 at 12:58 AM Nathan James via Phabricator <
r
njames93 added inline comments.
Herald added a subscriber: arphaman.
Comment at: clang-tidy/tool/ClangTidyMain.cpp:362
+ << Plural << "\n";
+return WErrorCount;
+ }
Was there any specific reason for returning the error count instead of
retur
alexfh added a subscriber: jroelofs.
alexfh added a comment.
In http://reviews.llvm.org/D15528#326031, @jroelofs wrote:
> Hmm. There's no "close revision" button. Abandoning it in lieu of closing it.
Weird. That's probably because I've not marked it "Accepted". Now I tried a few
other actions
jroelofs abandoned this revision.
jroelofs added a comment.
Hmm. There's no "close revision" button. Abandoning it in lieu of closing it.
http://reviews.llvm.org/D15528
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
jroelofs accepted this revision.
jroelofs added a reviewer: jroelofs.
jroelofs marked 2 inline comments as done.
jroelofs added a comment.
Thanks for the review! Committed r257642.
http://reviews.llvm.org/D15528
___
cfe-commits mailing list
cfe-comm
jroelofs updated this revision to Diff 44759.
jroelofs added a comment.
clang-format the patch, and rename method.
http://reviews.llvm.org/D15528
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
alexfh added a comment.
Looks good with a couple of nits. I'll be happy to submit your patch once you
address the comments.
Also, please include more context in your patches. See
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.
Comment at: clan
jroelofs updated this revision to Diff 44650.
jroelofs added a comment.
Added docs.
http://reviews.llvm.org/D15528
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/ClangTidyOptions.cpp
jroelofs added a comment.
In http://reviews.llvm.org/D15528#312031, @alexfh wrote:
> Thank you for explaining. The use case seems to be important enough to
> support it. And the solution seems to be good for now. A few concerns:
>
> 1. `Werrors` isn't a good name for this. The only reason why a
jroelofs marked 9 inline comments as done.
jroelofs added a comment.
http://reviews.llvm.org/D15528
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jroelofs updated this revision to Diff 44648.
jroelofs added a comment.
Address review comments.
http://reviews.llvm.org/D15528
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/ClangTi
jroelofs added a comment.
Thanks for the review! I'll rework this a bit early next week.
http://reviews.llvm.org/D15528
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh added a comment.
Thank you for explaining. The use case seems to be important enough to support
it. And the solution seems to be good for now. A few concerns:
1. `Werrors` isn't a good name for this. The only reason why a similar thing is
called `-Werror` in compilers is that they use `-
jroelofs added a comment.
In http://reviews.llvm.org/D15528#311135, @alexfh wrote:
> In http://reviews.llvm.org/D15528#311053, @jroelofs wrote:
>
> > In http://reviews.llvm.org/D15528#311019, @alexfh wrote:
> >
> > > Jonathan, can you explain what specific use case does this patch address?
> > >
jroelofs added inline comments.
Comment at: tools/llvm-project/extra/test/clang-tidy/werrors-plural.cpp:7
@@ +6,3 @@
+// CHECK-WARN: warning: namespace 'j' not terminated with a closing comment
[llvm-namespace-comment]
+// CHECK-WERR: error: namespace 'j' not terminated with a cl
alexfh added a comment.
In http://reviews.llvm.org/D15528#311053, @jroelofs wrote:
> In http://reviews.llvm.org/D15528#311019, @alexfh wrote:
>
> > Jonathan, can you explain what specific use case does this patch address?
> > Why one severity level of native clang-tidy warnings (the current
> >
aaron.ballman added inline comments.
Comment at: tools/llvm-project/extra/test/clang-tidy/werrors-plural.cpp:7
@@ +6,3 @@
+// CHECK-WARN: warning: namespace 'j' not terminated with a closing comment
[llvm-namespace-comment]
+// CHECK-WERR: error: namespace 'j' not terminated with
jroelofs added a comment.
In http://reviews.llvm.org/D15528#311019, @alexfh wrote:
> Jonathan, can you explain what specific use case does this patch address? Why
> one severity level of native clang-tidy warnings (the current situation) is
> not enough, and two levels are enough?
I have out-
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Jonathan, can you explain what specific use case does this patch address? Why
one severity level of native clang-tidy warnings (the current situation) is not
enough, and two levels a
aaron.ballman added inline comments.
Comment at: tools/llvm-project/extra/test/clang-tidy/werrors-plural.cpp:7
@@ +6,3 @@
+// CHECK-WARN: warning: namespace 'j' not terminated with a closing comment
[llvm-namespace-comment]
+// CHECK-WERR: error: namespace 'j' not terminated with
jroelofs added inline comments.
Comment at: tools/llvm-project/extra/clang-tidy/ClangTidy.cpp:119
@@ +118,3 @@
+ if (Error.IsWError) {
+Name += ", -Werrors=";
+Level = DiagnosticsEngine::Error;
hardcoding happens here ^
Comme
aaron.ballman accepted this revision.
aaron.ballman added a reviewer: alexfh.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Aside from two tiny nits, LGTM. Thank you for working on this, it's a great
feature! I think we should add something to the release notes t
jroelofs created this revision.
jroelofs added a reviewer: aaron.ballman.
jroelofs added a subscriber: cfe-commits.
http://reviews.llvm.org/D15528
Files:
tools/llvm-project/extra/clang-tidy/ClangTidy.cpp
tools/llvm-project/extra/clang-tidy/ClangTidy.h
tools/llvm-project/extra/clang-tidy/Cla
23 matches
Mail list logo
