[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

alexfh added a comment. In https://reviews.llvm.org/D40671#954906, @xgsa wrote: > In https://reviews.llvm.org/D40671#954661, @alexfh wrote: > > > In https://reviews.llvm.org/D40671#953888, @aaron.ballman wrote: > > > > > FWIW, I think we should do something about unknown check names in NOLINT >

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r320713. https://reviews.llvm.org/D40671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman added a comment. In https://reviews.llvm.org/D40671#954906, @xgsa wrote: > In https://reviews.llvm.org/D40671#954661, @alexfh wrote: > > > In https://reviews.llvm.org/D40671#953888, @aaron.ballman wrote: > > > > > FWIW, I think we should do something about unknown check names in NOL

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa added a comment. In https://reviews.llvm.org/D40671#954661, @alexfh wrote: > In https://reviews.llvm.org/D40671#953888, @aaron.ballman wrote: > > > FWIW, I think we should do something about unknown check names in NOLINT > > comments, but that can be done as a follow-up patch. If we're igno

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

alexfh accepted this revision. alexfh added a comment. In https://reviews.llvm.org/D40671#949732, @xgsa wrote: > In https://reviews.llvm.org/D40671#949687, @alexfh wrote: > > > How are unknown check names handled? More specifically: will the `// > > NOLINT(runtime/explicit)` comment disable all

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman added a comment. In https://reviews.llvm.org/D40671#953291, @xgsa wrote: > @aaron.ballman, sorry for my insistence, but it seems all the comments are > fixed and the patch is ready for commit or am I missing something? Could you > please commit it on my behalf, as I don't have rig

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa added a comment. @aaron.ballman, sorry for my insistence, but it seems all the comments are fixed and the patch is ready for commit or am I missing something? Could you please commit it on my behalf, as I don't have rights to do that? https://reviews.llvm.org/D40671 ___

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa added a comment. In https://reviews.llvm.org/D40671#949687, @alexfh wrote: > How are unknown check names handled? More specifically: will the `// > NOLINT(runtime/explicit)` comment disable all clang-tidy checks or none? None. If comment is syntactically correct and contains parenthesis,

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

alexfh added a comment. In https://reviews.llvm.org/D40671#945158, @xgsa wrote: > In https://reviews.llvm.org/D40671#944906, @alexfh wrote: > > > BTW, how will this feature interact with cpplint.py's way of handling > > specific NOLINT directives that use different lint rule names, which > > so

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 126058. xgsa added a comment. Documentation update https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp docs/ReleaseNotes.rst docs/clang-tidy/index.rst test/clang-tidy/nolint.cpp test/clang-tidy/nolintnextline.cpp Index:

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa marked an inline comment as done. xgsa added a comment. In https://reviews.llvm.org/D40671#948826, @aaron.ballman wrote: > There are still some outstanding concerns around the documentation wording, > but once those are resolved it should be ready to commit. If you don't have > commit acce

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D40671#947762, @xgsa wrote: > So can this patch be submitted? Should I do something to make it happen? There are still some outstanding con

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa added a comment. So can this patch be submitted? Should I do something to make it happen? https://reviews.llvm.org/D40671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa added a comment. In https://reviews.llvm.org/D40671#944906, @alexfh wrote: > BTW, how will this feature interact with cpplint.py's way of handling > specific NOLINT directives that use different lint rule names, which > sometimes refer to the same rule (e.g. `// NOLINT(runtime/explicit)`

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125517. xgsa added a comment. Updated documentation https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp docs/ReleaseNotes.rst docs/clang-tidy/index.rst test/clang-tidy/nolint.cpp test/clang-tidy/nolintnextline.cpp Index

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa marked 2 inline comments as done. xgsa added inline comments. Comment at: docs/clang-tidy/index.rst:254-255 +While :program:`clang-tidy` diagnostics are intended to call out code that does +not adhere to a coding standard, or is otherwise problematic in some way, there +are

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

alexfh added a comment. BTW, how will this feature interact with cpplint.py's way of handling specific NOLINT directives that use different lint rule names, which sometimes refer to the same rule (e.g. `// NOLINT(runtime/explicit)` suppresses the `runtime/explicit` cpplint rule that enforces th

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman added inline comments. Comment at: docs/clang-tidy/index.rst:280 +lint-command +lint-command lint-args + xgsa wrote: > aaron.ballman wrote: > > This should have a subscript `opt` following `lint-args` to denote that the > > args are optiona

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125328. https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp docs/ReleaseNotes.rst docs/clang-tidy/index.rst test/clang-tidy/nolint.cpp test/clang-tidy/nolintnextline.cpp Index: docs/clang-tidy/index.rst ==

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa marked 2 inline comments as done. xgsa added inline comments. Comment at: docs/clang-tidy/index.rst:253 +Generally, there is no need to suppress :program:`clang-tidy` diagnostics. If +there are false positives, either a bug should be reported or the code should be ---

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

alexfh added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:294 -static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc) { +static bool IsNOLINTFound(StringRef NolintMacro, StringRef Line, + unsigned DiagID, co

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

alexfh added inline comments. Comment at: docs/clang-tidy/index.rst:253 +Generally, there is no need to suppress :program:`clang-tidy` diagnostics. If +there are false positives, either a bug should be reported or the code should be aaron.ballman wrote: > I do

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa added inline comments. Comment at: docs/clang-tidy/index.rst:280 +lint-command +lint-command lint-args + aaron.ballman wrote: > This should have a subscript `opt` following `lint-args` to denote that the > args are optional. I think you can achieve

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor commenting nit, this LGTM. Thanks! Comment at: docs/clang-tidy/index.rst:280 +lint-command +lint-command lint-args +

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125292. xgsa added a comment. A typo in documentation was fixed. https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp docs/ReleaseNotes.rst docs/clang-tidy/index.rst test/clang-tidy/nolint.cpp test/clang-tidy/nolintnextli

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125291. xgsa added a comment. Updated documentation and comments in code. https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp docs/ReleaseNotes.rst docs/clang-tidy/index.rst test/clang-tidy/nolint.cpp test/clang-tidy/nol

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa marked 2 inline comments as done. xgsa added inline comments. Comment at: docs/clang-tidy/index.rst:270 +// Skip only the specified diagnostics for the next line +// NOLINTNEXTLINE (google-explicit-constructor, google-runtime-int) +Foo(bool param); -

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:299 +size_t BracketIndex = NolintIndex + NolintMacro.size(); +// Check if the specific checks are specified in brackets +if (BracketIndex < Line.size() && Line[BracketIndex] ==

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added a comment. I am happy with everything now. But one of the reviewers must accept. https://reviews.llvm.org/D40671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added inline comments. Comment at: test/clang-tidy/nolintnextline.cpp:23 + +// NOLINTNEXTLINE without-brackets-skip-all, another-check +class C5 { C5(int i); }; xgsa wrote: > JonasToth wrote: > > Ian confused now. The NOLINTNEXTLINE with incorrect paren

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa added inline comments. Comment at: test/clang-tidy/nolintnextline.cpp:23 + +// NOLINTNEXTLINE without-brackets-skip-all, another-check +class C5 { C5(int i); }; JonasToth wrote: > Ian confused now. The NOLINTNEXTLINE with incorrect parents should not > sile

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added a comment. Its good that you added code examples to the clang-tidy documentation page. I think that feature was not documented well before. Comment at: test/clang-tidy/nolintnextline.cpp:23 + +// NOLINTNEXTLINE without-brackets-skip-all, another-check +class C5

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125258. xgsa added a comment. Release note item was reworded https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp docs/ReleaseNotes.rst docs/clang-tidy/index.rst test/clang-tidy/nolint.cpp test/clang-tidy/nolintnextline.c

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

lebedev.ri added inline comments. Comment at: docs/ReleaseNotes.rst:259 +- The ``NOLINT`` and ``NOLINTNEXTLINE`` suppression comments were extended to support the list of checks + to disable in parentheses. This reads strange, maybe it can be reworded somehow

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125216. xgsa added a comment. Minor change: update default value of SmallVector of check names. https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp docs/ReleaseNotes.rst docs/clang-tidy/index.rst test/clang-tidy/nolint.cpp

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125190. xgsa added a comment. An item to release notes was added. Also I have added a paragraph about NOLINT to the main documentation page, because I suppose it's useful information and it's related to the feature. Please, let me know if it should be added wi

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman added a comment. This feature should probably be mentioned in the release notes. https://reviews.llvm.org/D40671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125140. https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp test/clang-tidy/nolint.cpp test/clang-tidy/nolintnextline.cpp Index: test/clang-tidy/nolint.cpp === -

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

Eugene.Zelenko added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:293 } - -static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc) { +static bool IsNOLINTFound(StringRef NolintMacro, StringRef Line, + unsigned

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:295 + unsigned DiagID, const ClangTidyContext &Context) { + const auto NolintIndex = Line.find(NolintMacro); + if (NolintIndex != StringRef::npos) {

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125096. xgsa added a comment. A few additional test cases were added. https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp test/clang-tidy/nolint.cpp test/clang-tidy/nolintnextline.cpp Index: test/clang-tidy/nolint.cpp =

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added inline comments. Comment at: test/clang-tidy/nolintnextline.cpp:14 + +// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all +class C2 { C2(int i); }; xgsa wrote: > JonasToth wrote: > > xgsa wrote: > > > JonasToth wrote: > > > > missing `)` >

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa updated this revision to Diff 125084. xgsa added a comment. Herald added a subscriber: xazax.hun. Add comments to code as it was recommended. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp test/clang-tidy/nolint.c

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

xgsa added inline comments. Comment at: test/clang-tidy/nolintnextline.cpp:14 + +// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all +class C2 { C2(int i); }; JonasToth wrote: > xgsa wrote: > > JonasToth wrote: > > > missing `)` > > No, it's intentionally

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

JonasToth added inline comments. Comment at: test/clang-tidy/nolintnextline.cpp:14 + +// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all +class C2 { C2(int i); }; xgsa wrote: > JonasToth wrote: > > missing `)` > No, it's intentionally: it's a test for ca