[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

alexfh added a comment. A high-level comment: NOLINT category parsing and warning on incorrect NOLINT categories makes it more difficult for code to comply both with clang-tidy and cpplint (https://github.com/cpplint/cpplint), since: 1. the category names in these tools are different, 2.

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa updated this revision to Diff 128893. xgsa added a comment. Fixed showing the check in -list-checks. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41326 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidyDiagnosticConsumer.cpp

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:839-840 +case NolintCommentType::Nolint: + Message = "there is no diagnostics on this line, " +"the NOLINT comment is redundant"; + break;

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:839-840 +case NolintCommentType::Nolint: + Message = "there is no diagnostics on this line, " +"the NOLINT comment is redundant"; + break;

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa updated this revision to Diff 128431. xgsa added a comment. Rename the check `nolint-usage` => `readability-nolint-usage` for consistency. Update diagnostics message according to the review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41326 Files:

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:839-840 +case NolintCommentType::Nolint: + Message = "there is no diagnostics on this line, " +"the NOLINT comment is redundant"; + break;

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:424 +// This method is not const, because it fills cache on first demand. +// It is possible to fill cache in constructor or make cache volatile +// and make this method const, but they

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa marked 5 inline comments as done. xgsa added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:276 + + using NolintMap = std::unordered_map

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa updated this revision to Diff 128144. xgsa marked an inline comment as done. xgsa added a comment. Herald added a subscriber: mgrang. Review comments applied. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41326 Files: clang-tidy/ClangTidy.cpp

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:161 + + const SmallVector& getCheckNames() const { +assert(isNolintFound()); aaron.ballman wrote: > The `&` should bind to the right. However, instead

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa updated this revision to Diff 128092. xgsa added a comment. The full diff (but not only the incremental one) was uploaded. Please, skip previous revision. Sorry. https://reviews.llvm.org/D41326 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidyDiagnosticConsumer.cpp

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa updated this revision to Diff 128091. xgsa marked an inline comment as done. xgsa added a comment. Review comments applied. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41326 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tidy/ClangTidyDiagnosticConsumer.h

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa marked 13 inline comments as done. xgsa added a comment. Aaron, thank you for your review and sorry for the coding convention mistakes -- I still cannot get used to the llvm coding convention, because it quite differs from the one I have been using in my projects.

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:21-22 #include "ClangTidyOptions.h" +#include "ClangTidy.h" +#include "ClangTidyModule.h" #include "clang/AST/ASTDiagnostic.h" Please keep the includes in sorted order.

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa added a comment. Ping. https://reviews.llvm.org/D41326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa added a comment. In https://reviews.llvm.org/D41326#957749, @JVApen wrote: > I'm missing some documentation to understand the corner cases. How does this > check behave with suppressed warnings for checks which ain't currently > checked. (Using -no-... on a code base or suppressing the

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

JVApen added a comment. I'm missing some documentation to understand the corner cases. How does this check behave with suppressed warnings for checks which ain't currently checked. (Using -no-... on a code base or suppressing the warnings via the pragmas) https://reviews.llvm.org/D41326

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa updated this revision to Diff 127268. xgsa added a comment. Review comments applied. https://reviews.llvm.org/D41326 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tidy/ClangTidyDiagnosticConsumer.h test/clang-tidy/nolint-usage.cpp

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:339 std::unique_ptr OptionsProvider) : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)), + Profile(nullptr), xgsa wrote: > Eugene.Zelenko wrote: > >

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa updated this revision to Diff 127267. xgsa added a comment. Review comments were applied. https://reviews.llvm.org/D41326 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tidy/ClangTidyDiagnosticConsumer.h test/clang-tidy/nolint-usage.cpp

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa marked 3 inline comments as done. xgsa added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:339 std::unique_ptr OptionsProvider) : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)), + Profile(nullptr),

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

Eugene.Zelenko added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:169 + } +private: + SmallVector CheckNames; Please separate with empty line. Comment at:

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa updated this revision to Diff 127260. xgsa added a comment. A few minor coding style fixes. https://reviews.llvm.org/D41326 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tidy/ClangTidyDiagnosticConsumer.h test/clang-tidy/nolint-usage.cpp

[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment

xgsa created this revision. xgsa added reviewers: aaron.ballman, alexfh. xgsa added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun. As discussed in the previous review [1], diagnostics about incorrect usage of NOLINT comment was added, i.e..: - usage of NOLINT