[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-27 Thread Richard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8ce99dadb007: [clang-tidy] Add more documentation about check development (NFC) (authored by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for these fantastic improvements to the docs! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 ___ cfe-commits mail

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-26 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 403386. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 Files: clang-tools-extra/docs/clang-tidy/Contributing.rst Index: clang-to

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-26 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked 2 inline comments as done. LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:389-390 +- Test your check under both Windows and Linux environments. +- Watch out for high false positive rates; ideally th

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:362 +Private custom matchers are a good example of auxiliary support code for your check +that is best tested with a unit test. It will be easier to test your matchers or +other suppo

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:362 +Private custom matchers are a good example of auxiliary support code for your check +that is best tested with a unit test. It will be easier to test your matchers or +other

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-25 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 403118. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 Files: clang-tools-extra/docs/clang-tidy/Contributing.rst Index: clang-to

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-25 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:362 +Private custom matchers are a good example of auxiliary support code for your check +that is best tested with a unit t

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Overall, this looks fantastic! You may want to consider (in a separate patch) mentioning godbolt.org, which is a great UI for interacting with clang-query and the AST. Example configuration: https://godbolt.org/z/v88W8ET19 In D117939#3266234

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Aside from the unit testing bit, I think this is fantastic! (And the unit testing bit may also be fantastic, I just think it needs more explicit discussion with a wider audience.) Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:30

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402741. LegalizeAdulthood marked 2 inline comments as done. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 Files: clang-tools-ext

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked 16 inline comments as done. LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:301-317 +The Transformer library allows you to write a check that transforms source code by +expressing the transformation

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D117939#3266228 , @njames93 wrote: > @aaron.ballman I think this has exposed some limitations with the > add-new-check script. Maybe there is merit for the script to be updated to > support preprocessor callbacks and op

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D117939#3266228 , @njames93 wrote: > @aaron.ballman I think this has exposed some limitations with the > add-new-check script. Maybe there is merit for the script to be updated to > support preprocessor callbacks an

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:229 +If you need to interact macros and preprocessor directives, you will want to +override the method ``registerPPCallbacks``. The ``add_new_check.py`` script +does not gener

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. @aaron.ballman I think this has exposed some limitations with the add-new-check script. Maybe there is merit for the script to be updated to support preprocessor callbacks and options, WDYT? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://re

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you so much for working on this documentation, I really like the direction it's going! Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:78 +build :program:`clang-tidy`. +Since your new check will have associated documentation,

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-23 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402317. LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 Files: clang-tools-ext

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-23 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked 2 inline comments as done. LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:232 + +If your check applies only to specific dialect of C or C++, you will +want to override the method ``isLanguageVersion

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-23 Thread Salman Javed via Phabricator via cfe-commits
salman-javed-nz added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:76 +When you `configure the CMake build `_, +make sure that you enable the ``clang-tools-extra`` project to b

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-22 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:232 + +If your check applies only to specific dialect of C or C++, you will +want to override the method ``isLanguageVersionSupported`` to reflect that. Objective

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-22 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402284. LegalizeAdulthood added a comment. Clarify ninja build example CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 Files: clang-tools-extra/docs/clang-tidy/Contributing.rst Index: clang-to

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-22 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402283. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 Files: clang-tools-extra/docs/clang-tidy/Contributing.rst Index: clang-to

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-22 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:232 + +If your check applies only to specific versions of the C++ standard, you will +want to override the method ``isLanguageVersionSupported`` to reflect that.

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-22 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:295 +Reference `_ to understand +the relationship between the different matcher funtions. + Should b

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-22 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402238. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 Files: clang-tools-extra/docs/clang-tidy/Contributing.rst Index: clang-to

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-22 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked 7 inline comments as done. LegalizeAdulthood added a comment. In D117939#3263180 , @Eugene.Zelenko wrote: > It's also make sense to mention `isLanguageVersionSupported`. Good idea. CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-21 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. It's also make sense to mention `isLanguageVersionSupported`. Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:79 +`Sphinx `_ and enable it in the CMake configuration. +To save build time of t

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-21 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402153. LegalizeAdulthood added a comment. Spelling CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 Files: clang-tools-extra/docs/clang-tidy/Contributing.rst Index: clang-tools-extra/docs/clan

[PATCH] D117939: [clang-tidy] Add more documentation about check development

2022-01-21 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added reviewers: alexfh, aaron.ballman, ymandel, njames93. LegalizeAdulthood added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. LegalizeAdulthood requested review of this revision. - Mention pp-trace - CMake configur