[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 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] D7982: [clang-tidy] Add readability-duplicate-include check

2022-01-23 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D7982#3264009 , @carlosgalvezp wrote: > In D7982#3263920 , @LegalizeAdulthood > wrote: > >> In D7982#3263790 , @carlosgalvezp >> wrote: >

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

2022-01-23 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. OK, removing Alex has cleared it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

2022-01-23 Thread Richard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd2e8fb331835: [clang-tidy] Add readability-duplicate-include check (authored by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/

[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

2022-01-23 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood abandoned this revision. LegalizeAdulthood added a comment. Abandoning this in favor of https://reviews.llvm.org/D117409 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117129/new/ https://reviews.llvm.org/D117129 ___ cfe-comm

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2022-01-23 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood abandoned this revision. LegalizeAdulthood added a comment. Abandoning this in favor of a private matcher where it is needed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116328/new/ https://reviews.llvm.org/D116328 ___ cfe

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

2022-01-23 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402370. LegalizeAdulthood retitled this revision from "[clang-tidy] Handle case/default statements when simplifying boolean expressions" to "[clang-tidy] Recognize labelled statements when simplifying boolean exprs". LegalizeAdulthood edited the sum

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402415. LegalizeAdulthood added a comment. Testing on iterated dynamics revealed a number of shortcomings; update the code to address those: - Improve handling of include guards - Handle conditional compilation blocks better - Reject macros that exp

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402416. LegalizeAdulthood added a comment. Switch back to SmallVector after debugging with std::vector Drop unnecessary includes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117522/new/ https://reviews.llvm.org/D117522 Files: clang-too

[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 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] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402716. LegalizeAdulthood added a comment. Update from additional testing: - reject floating-point literals (they can't be enumerations) - add more test cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117522/new/ https://reviews.llvm.

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp:1 +// RUN: %check_clang_tidy -std c++14-or-later %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum + Ah, this leake

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-01-24 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 402725. LegalizeAdulthood added a comment. Remove -std for test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117522/new/ https://reviews.llvm.org/D117522 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt clang-tools-extra/

[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 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] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

2022-01-25 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D56303#3266252 , @njames93 wrote: > A large portion of the changes, especially in the checks implementation file, > appear to be NFC stylistic or formatting only fixes. While these changes are > generally good, they

[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 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] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

2022-01-25 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:72-73 -const Expr *getBoolLiteral(const MatchFinder::MatchResult &Result, - StringRef Id) { +static const Expr *getBoolLiteral(c

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

2022-01-25 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:72-73 -const Expr *getBoolLiteral(const MatchFinder::MatchResult &Result, - StringRef Id) { +static const Expr *getBoolLiteral(c

[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 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] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

2022-01-26 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D56303#3272711 , @aaron.ballman wrote: > The rule of thumb is: if you have significant NFC changes, > do them first and push the changes (often doesn't even > require a review because it's NFC, but that depends on > w

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

2022-01-26 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 403390. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56303/new/ https://reviews.llvm.org/D56303 Files: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

[PATCH] D118010: [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check

2022-01-26 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood accepted this revision. LegalizeAdulthood added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118010/new/ https://reviews.llvm.org/D118010 _

[PATCH] D118010: [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check

2022-01-26 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. If there is an open github issue on this, please put "Fixes #" in the commit description before pushing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118010/new/ https://reviews.llvm.org/D118010 ___

[PATCH] D118104: Make run-clang-tidy.py print the configured checks correctly

2022-01-26 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:163 def run_tidy(args, tmpdir, build_path, queue, lock, failed_files): """Takes filenames out of queue and runs clang-tidy on them.""" Am I missing some

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

2022-01-26 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang/lib/CodeGen/Address.h:70 +unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt(); +return CharUnits::fromQuantity(1UL << AlignLog); + } This is causing warnings to be emitted: ``` cla

[PATCH] D118010: [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check

2022-01-26 Thread Richard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG836950c4e602: [clang-tidy] Fix nested namespaces in `readability-static-definition-in… (authored by Izaron, committed by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[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] D118370: [clang-tidy] bugprone-signal-handler: Code refactor (NFC)

2022-01-27 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:146 + if (!HandlerDecl->hasBody()) { +checkFunction(HandlerDecl, HandlerExpr); return; Why do we ignore the return value of `checkFunction`

[PATCH] D118370: [clang-tidy] bugprone-signal-handler: Message improvement and code refactoring.

2022-01-28 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:146 + if (!HandlerDecl->hasBody()) { +checkFunction(HandlerDecl, HandlerExpr); return; Can we put `(void) checkFunction(...)` here to make

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

2022-01-28 Thread Richard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG99217fa8a027: [clang-tidy] Recognize labelled statements when simplifying boolean exprs (authored by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D118519: [clang-tidy] Organize the release notes a little better

2022-01-28 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. LegalizeAdulthood requested review of this revision. - Sort new checks by check name - Sort changes to existing checks by check name - Link check names to the check'

[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

2022-01-28 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:201 +} // namespace clang ClangTidyError::ClangTidyError(StringRef CheckName, ClangTidyError::Level DiagLevel, Inser

[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

2022-01-28 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:181 + } + void onResultEntry(StringRef CheckName, + const ast_matchers::BoundNodes &BoundNodes) { blank line before `onResultEn

[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

2022-01-28 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood accepted this revision. LegalizeAdulthood added a comment. This revision is now accepted and ready to land. LGTM with a few whitespace nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118520/new/ https://reviews.llvm.org/D11852

[PATCH] D118519: [clang-tidy] Organize the release notes a little better

2022-01-29 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:221-227 Removed checks ^^ Improvements to include-fixer - The improvements are...

[PATCH] D118519: [clang-tidy] Organize the release notes a little better

2022-01-29 Thread Richard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. LegalizeAdulthood marked an inline comment as done. Closed by commit rG994802068267: [clang-tidy] Organize the release notes a little better (authored by LegalizeAdulthood). Changed prior to commit: https://reviews.llvm.o

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-01-30 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. This situation isn't properly diagnosed (false positive): #ifdef USE_FOO #if USE_FOO // code #endif #endif CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117522/new/ https://reviews.llvm.org/D117522 __

[PATCH] D118520: [clang-tidy] Output currently processing check and nodes on crash

2022-01-31 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood accepted this revision. LegalizeAdulthood added a comment. This revision is now accepted and ready to land. Still LGTM and my comments are just thought experiments, not must haves before pushing. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticCons

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-31 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp:16 + int value_int = 42; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'value_int' of type 'int' can be declared 'const' +}

[PATCH] D118711: [hack] Build a tree of preprocessing directives

2022-02-01 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: ymandel. Herald added subscribers: carlosgalvezp, kbarton, mgorny, nemanjai. LegalizeAdulthood requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. This cod

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-02-01 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp:12 + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double *' can be declared 'const' +

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-02-01 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Ping. This check has gone for a week without review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117522/new/ https://reviews.llvm.org/D117522 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

2022-02-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. You've probably had this check in development for some time, but we just updated the documentation for contributing to clang-tidy and it might help to go over it for your new check to see if there is anything that stands out. https://clang.llvm.org/extra/clang-ti

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

2022-02-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp:64-68 + unless(hasType(isVolatileQualified())), // non-volatile + unless(isTemplateVariable()), // no

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-02-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. I think it helps to understand this check better if we consider what we would do manually if we were modernizing code with a lot of macro-style enums in it. I've done this myself; in fact all the checks I've written for clang-tidy have been motivated by moderniz

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-02-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Oh, I think I misinterpreted what you were saying in the last bit: In D117522#3290604 , @carlosgalvezp wrote: > Another point is that macros typically have a different naming > convention than enums or constants, so us

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-02-25 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked 5 inline comments as done. LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:382 + + if (std::strncmp("pragma", Text, 6) != 0) +return; carlosgalvezp wrote: > Maybe wrap

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-02-25 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 411555. LegalizeAdulthood marked 3 inline comments as done. LegalizeAdulthood added a comment. - Add intention revealing functions for details of pragma once parsing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117522/new/ https://reviews

[PATCH] D119949: [Clang-tidy] Check the existence of ElaboratedType's qualifiers

2022-03-01 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood accepted this revision. LegalizeAdulthood added a comment. This revision is now accepted and ready to land. Fix one nit and ship Comment at: clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:74 + PrintingPolicyWithSupressedTag.P

[PATCH] D120947: [tooling] Explain how to create a compilation database on Windows [NFC]

2022-03-03 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added reviewers: njames93, aaron.ballman, ymandel. LegalizeAdulthood added a project: clang-tools-extra. Herald added a project: All. LegalizeAdulthood requested review of this revision. Herald added a project: clang. Document how to creat

[PATCH] D120947: [tooling] Explain how to create a compilation database on Windows [NFC]

2022-03-04 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 413093. LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added a comment. Update from review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120947/new/ https://reviews.llvm.org/D120947 Files: clang/docs/HowT

[PATCH] D120947: [tooling] Explain how to create a compilation database on Windows [NFC]

2022-03-04 Thread Richard via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd5d03135a716: [tooling] Explain how to create a compilation database on Windows [NFC] (authored by LegalizeAdulthood). Repository: rG LLVM Github

[PATCH] D121019: [clang-tools-extra] Document clang tidy unit tests target

2022-03-04 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added reviewers: aaron.ballman, njames93, ymandel. LegalizeAdulthood added a project: clang-tools-extra. Herald added a project: All. LegalizeAdulthood requested review of this revision. Repository: rG LLVM Github Monorepo https://revie

[PATCH] D121019: [clang-tools-extra] Document clang tidy unit tests target

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

[PATCH] D121019: [clang-tools-extra] Document clang tidy unit tests target

2022-03-07 Thread Richard via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGde29719af2c7: [clang-tools-extra] Document clang tidy unit tests target (authored by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

2022-03-07 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Herald added a project: All. Ping. Another week waiting for reviews... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117522/new/ https://reviews.llvm.org/D117522 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

2021-12-30 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396683. LegalizeAdulthood marked an inline comment as not done. LegalizeAdulthood added a comment. Improve name of predicate function. Use standard algorithms instead of raw loops CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116386/new/ h

[PATCH] D116425: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses

2021-12-30 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, xazax.hun. LegalizeAdulthood requested review of this revision. Sometimes a macro invocation will look like an argumen

[PATCH] D116328: [ast-matchers] Add hasSubstmt() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2021-12-31 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:358 REGISTER_MATCHER(hasStructuredBlock); + REGISTER_MATCHER(hasSubstmt); REGISTER_MATCHER(hasSyntacticForm); I see there is another matcher called hasAnySubsta

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2022-01-01 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396878. LegalizeAdulthood retitled this revision from "[ast-matchers] Add hasSubstmt() traversal matcher for caseStmt(), defaultStmt(), labelStmt()" to "[ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt

[PATCH] D56343: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py

2022-01-01 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396909. LegalizeAdulthood marked 4 inline comments as done. LegalizeAdulthood edited the summary of this revision. LegalizeAdulthood set the repository for this revision to rG LLVM Github Monorepo. LegalizeAdulthood added a comment. Rebase on top of

[PATCH] D56343: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py

2022-01-01 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. This was accepted before in August, 2019, but the script changed in the meantime. I've repeated the Extract Class on top-of-tree, so presumably this is still acceptable. However, I don't know how commit access has changed. (I don't think I have it anymore.)

[PATCH] D7982: Add readability-duplicate-include check to clang-tidy

2022-01-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396932. LegalizeAdulthood added a comment. Herald added a subscriber: carlosgalvezp. Herald added a project: clang-tools-extra. Revive review with updated diff on top-of-tree Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2022-01-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396933. LegalizeAdulthood added a comment. Document the new matcher in the clang release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116328/new/ https://reviews.llvm.org/D116328 Files: clang/docs/LibASTMatchersReference.html c

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

2022-01-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396941. LegalizeAdulthood added a comment. Move included headers to clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include Stub out included headers to isolate from system headers Run test for C++98 or later CHANGES SINCE

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

2022-01-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D7982#3216332 , @Eugene.Zelenko wrote: > Functionality-wise this check is superseded by Include What You Use > . That's not part of clang-tidy and has it'

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

2022-01-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: klimek. LegalizeAdulthood added a project: clang. LegalizeAdulthood requested review of this revision. This allows the matching of two specific statements in sequence within a compound statement. For instance `if (x) ret

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

2022-01-02 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 397004. LegalizeAdulthood added a comment. Improve matching when the first matcher matches multiple times, but the 2nd matcher doesn't match at first. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116518/new/ https://reviews.llvm.org/D1165

[PATCH] D116550: [clang-tidy] Recognize transformer checks as providing fixits

2022-01-03 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, xazax.hun. LegalizeAdulthood requested review of this revision. [clang-tidy] Recognize transformer checks as providing

[PATCH] D116550: [clang-tidy] Recognize transformer checks as providing fixits

2022-01-03 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 397115. LegalizeAdulthood added a comment. Formatting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116550/new/ https://reviews.llvm.org/D116550 Files: clang-tools-extra/clang-tidy/add_new_check.py clang-tools-extra/clang-tidy/utils/T

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

2022-01-03 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5435-5442 +/// Matches two consecutive statements within a compound statement. +/// +/// Given +/// \code +/// { if (x > 0) ret

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

2022-01-03 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 397121. LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added a comment. Add `stmtExpr` test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116518/new/ https://reviews.llvm.org/D116518 Files: clang/docs/LibASTM

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

2022-01-04 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/boost/CMakeLists.txt:8 BoostTidyModule.cpp + UseRangeBasedForLoopCheck.cpp UseToStringCheck.cpp I am wondering if this check is better placed in the modernize module? Maybe a

[PATCH] D56343: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py

2022-01-04 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. If there are no new comments by the end of the week, I'm going to accept this revision and commit it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56343/new/ https://reviews.llvm.org/D56343

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

2022-01-04 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. I opened a similar issue for converting Qt's foreach to a range for loop . However this check lands, it should be a simple generalization to have it process Qt foreach loops as well, so perhaps a check name li

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

2022-01-04 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5435-5442 +/// Matches two consecutive statements within a compound statement. +/// +/// Given +/// \code +/// { if (x > 0) ret

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2022-01-05 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added a comment. In D116328#3221995 , @aaron.ballman wrote: >> Previously if you wanted to match the statement associated with a case, >> default, or labelled statement, you had to us

[PATCH] D116425: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses

2022-01-05 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp:561 +#define return_t(T) T +return_t(void) func(void); +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: redundant void argument list in function declara

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

2022-01-05 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5435-5442 +/// Matches two consecutive statements within a compound statement. +/// +/// Given +/// \code +/// { if (x > 0) return true; return false; } +/// \endcode +/// compoun

[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

2022-01-05 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5435-5442 +/// Matches two consecutive statements within a compound statement. +/// +/// Given +/// \code +/// { if (x > 0) return true; return false; } +/// \endcode +/// compoun

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2022-01-05 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D116328#3223299 , @aaron.ballman wrote: > In D116328#3223268 , > @LegalizeAdulthood wrote: > >> In D116328#3221995 , >> @aaron.bal

[PATCH] D116425: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses

2022-01-05 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp:561 +#define return_t(T) T +return_t(void) func(void); +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: redundant void argument list in function declara

[PATCH] D116550: [clang-tidy] Recognize transformer checks as providing fixits

2022-01-05 Thread Richard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd7b6574c3bf6: [clang-tidy] Recognize transformer checks as providing fixits (authored by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11

[PATCH] D116425: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses

2022-01-05 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp:561 +#define return_t(T) T +return_t(void) func(void); +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: redundant void argument list in function declara

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

2022-01-06 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Can someone look at this review please, it's not that long and/or complicated. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116386/new/ https://reviews.llvm.org/D116386 ___ cfe-commits mailing lis

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

2022-01-06 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Aaron, I think your comments are useful and I would be inclined to agree with you if I was the original author of this check. I treat the guidelines as just that: guidelines, not rules. In the context of clang-tidy I think you're correct that some guidelines

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

2022-01-06 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Also, re-reading the C++ guidelines, I agree that this check is handling the two cases ES.31 (Don't use macros for constants and functions) ES.33 (Don't use macros that aren't all caps) I guess previously it **technically** handled ES.30 (don't use macros for t

[PATCH] D116425: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses

2022-01-07 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp:561 +#define return_t(T) T +return_t(void) func(void); +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: redundant void argument list in function declara

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

2022-01-07 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D116386#3227236 , @aaron.ballman wrote: > We're in strong agreement that guideline checkers with untenable false > positive rates are a > very bad thing. I think this particular check is of insufficient quality to

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

2022-01-07 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a reviewer: JonasToth. LegalizeAdulthood added a comment. I'm adding the original author of this check as a reviewer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116386/new/ https://reviews.llvm.org/D116386 ___ cfe-c

[PATCH] D116425: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses

2022-01-07 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 398281. LegalizeAdulthood edited the summary of this revision. LegalizeAdulthood added a comment. Add more test cases and improve the Lexing state machine CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116425/new/ https://reviews.llvm.org/D

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

2022-01-09 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Herald added a subscriber: carlosgalvezp. Comment at: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt:19 ReplaceAutoPtrCheck.cpp + ReplaceMemcpyByStdCopy.cpp ReplaceRandomShuffleCheck.cpp In English, it's more

[PATCH] D116577: [clang-tidy] Added "boost-use-range-based-for-loop" check

2022-01-10 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/boost/UseRangeBasedForLoopCheck.cpp:68 + ColToken->getLocation()), + (llvm::Twine("for(") + VariableString + " : ").str()); + } ---

[PATCH] D97625: fix check-clang-tools tests that fail due to Windows CRLF line endings

2022-01-10 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. I have run into this line ending problem with python scripts as well. If you follow the advice on the "getting started" pages, then you will have checked out the entire repo with Unix line endings, but when you run python scripts like add_new_check.py for clang-t

[PATCH] D72940: Add a support for clang tidy to import another configurations files from .clang-tidy

2022-01-10 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood requested changes to this revision. LegalizeAdulthood added a comment. This revision now requires changes to proceed. Herald added a subscriber: carlosgalvezp. Please upload a full context diff so that the changes can be properly viewed. Thanks. Repository: rG LLVM Github M

<    1   2   3   4   5   >