[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 clang-tidy/ClangTidyDiagnosticConsumer

[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

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: clang-tidy/

[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

xgsa marked 5 inline comments as done. xgsa added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:276 + + using NolintMap = std::unordered_map xgsa wrote: > > aaron.ballman wrote: > > > Is there a better LLVM ADT to use here? > > This data structure provi

[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 clang-tidy/ClangTidyDiagn

[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 clang-tidy/Cla

[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. Commen

[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 war

[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 test/clang-tid

[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 test/clan

[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), E

[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 test/cl

[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 wit

[PATCH] D39622: Fix type name generation in DWARF for template instantiations with enum types and template specializations

xgsa added a comment. In https://reviews.llvm.org/D39622#954585, @probinson wrote: > Philosophically, mangled names and DWARF information serve different > purposes, and I don't think you will find one true solution where both of > them can yield the same name that everyone will be happy with.

[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] D39622: Fix type name generation in DWARF for template instantiations with enum types and template specializations

xgsa updated this revision to Diff 126826. xgsa retitled this revision from "Fix type debug information generation for enum-based template specialization" to "Fix type name generation in DWARF for template instantiations with enum types and template specializations". xgsa edited the summary of th

[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

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

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

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

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

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

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

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

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

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

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

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 categories for NOLINT directive

xgsa added a comment. In https://reviews.llvm.org/D40671#941676, @JonasToth wrote: > Could you please explain what category means? Could i disable all of > `cppcoreguidelines` with something like `// NOLINT (cppcoreguidelines-*)`? No, only individual checks can be disabled. You are right, by "

[PATCH] D40671: Support specific categories for NOLINT directive

xgsa updated this revision to Diff 125008. xgsa added a reviewer: dcoughlin. xgsa added a comment. Update the diff to contain the full context Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40671 Files: clang-tidy/ClangTidyDiagnosticConsumer.cpp test/clang-tidy/nolint.cpp

[PATCH] D40671: Support specific categories for NOLINT directive

xgsa created this revision. xgsa added a project: clang-tools-extra. The NOLINT directive was extended to support the "NOLINT(category)" and "NOLINT(*)" syntax. Also it is possible to specify a few categories separated with comma "NOLINT(cat1, cat2)" Repository: rCTE Clang Tools Extra https

[PATCH] D39622: Fix type debug information generation for enum-based template specialization

xgsa added a comment. In https://reviews.llvm.org/D39622#919579, @aprantl wrote: > For clarification: what is the "symbols table" you are referring to in the > description? I meant the data dumped with "nm -an ./test". By the way, I haven't abandoned the patch, I have found one more case when

[PATCH] D39622: Fix type debug information generation for enum-based template specialization

xgsa added inline comments. Comment at: include/clang/AST/PrettyPrinter.h:227 + + /// \brief Use formatting compatible with ABI specification. It is necessary for + /// saving entities into debug tables which have to be compatible with aprantl wrote: > xgsa wr

[PATCH] D39633: Remove \brief from doxygen comments in PrettyPrinter.h

xgsa created this revision. xgsa added a project: clang. For consistency with the code proposed in https://reviews.llvm.org/D39622 Repository: rL LLVM https://reviews.llvm.org/D39633 Files: include/clang/AST/PrettyPrinter.h Index: include/clang/AST/PrettyPrinter.h

[PATCH] D39622: Fix type debug information generation for enum-based template specialization

xgsa added inline comments. Comment at: include/clang/AST/PrettyPrinter.h:68 /// \brief The number of spaces to use to indent each line. - unsigned Indentation : 8; + unsigned Indentation : 7; aprantl wrote: > this change looks like it has the potential to

[PATCH] D39622: Fix type debug information generation for enum-based template specialization

xgsa added a comment. In https://reviews.llvm.org/D39622#915722, @aprantl wrote: > Can you add a testcase? Definitely, I just wanted to know if such fix is correct at all. Moreover, could you please help me with the correct place for a test case? Possibly, there are some similar test case I c

[PATCH] D39622: Fix type debug information generation for enum-based template specialization

xgsa created this revision. xgsa added a project: clang. Herald added a subscriber: aprantl. Currently, there is an inconsistency between type name record in ".apple_types" section and entry in symbols table for the same type. In particular, for such types lldb is unable to resolve the real type