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

2022-01-10 Thread Richard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa2c33b0ec976: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py (authored by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

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

2022-01-10 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 398754. LegalizeAdulthood added a comment. clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116518/new/ https://reviews.llvm.org/D116518 Files: clang/docs/LibASTMatchersReference.html clang/docs/ReleaseNotes.rst clang/incl

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

2022-01-10 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 398756. LegalizeAdulthood added a comment. clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy

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

2022-01-11 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp:157-161 + if (ProtoToken.is(tok::TokenKind::l_paren)) { +State = TokenState::LeftParen; + } else if (isMacroIdentifier(Idents, ProtoToken)) { +

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

2022-01-11 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 399076. LegalizeAdulthood added a comment. - clang-format - use `const IdentifierTable &` to avoid accidental modification - drop braces from simple block statements as per style guide CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116425/ne

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

2022-01-11 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] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

2022-01-11 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 399186. LegalizeAdulthood added a comment. - clang-format - use Token::isLiteral instead of homebrew token predicate - rename predicate on token list to reflect isLiteral CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116386/new/ https://re

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

2022-01-12 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, arphaman, kbarton, xazax.hun, mgorny, nemanjai. LegalizeAdulthood requested review of this revision. IncluderClangTid

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

2022-01-12 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D116518#3237927 , @ymandel wrote: > [...] In the meantime, I'll note that we have something similar internally > which I never got around to upstreaming. That's very interesting and is certainly more general than wha

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

2022-01-12 Thread Richard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfff59f48173d: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses (authored by LegalizeAdulthood). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

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

2022-01-12 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D117129#3238433 , @ymandel wrote: > Overall, this change looks good and makes sense. But, it doesn't scale past 1 > feature > [...] Might there be some other way to get the benefits of saving > implementers the > cu

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

2022-01-12 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Another option would be to migrate this to `ClangTidyCheck` itself and add a flag/option passed to the ClangTidyCheck c'tor to opt-in to the feature? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117129/new/ https

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

2022-01-12 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D117129#3239143 , @njames93 wrote: > My proposed idea was putting the `IncludeInserter` instance > inside the `ClangTidyContext`, but then only registering it if > any checks actually want to use it. Use of the `Incl

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

2022-01-13 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Ping. This review has been waiting for a week without any comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116386/new/ https://reviews.llvm.org/D116386 ___ cfe-commits mailing list cfe-commits@lists.l

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

2022-01-13 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Ping. This review has been waiting for a week without any comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116328/new/ https://reviews.llvm.org/D116328 ___ cfe-commits mailing list cfe-commits@lists.l

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

2022-01-13 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Ping. This review has been waiting for a week without any comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7982/new/ https://reviews.llvm.org/D7982 ___ cfe-commits mailing list cfe-commits@lists.llvm.

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

2022-01-13 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D116328#3241329 , @aaron.ballman wrote: > In D116328#3223344 , > @LegalizeAdulthood wrote: > >> My takeaway: >> >> - if `has` isn't expensive, I can either ditch this public

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

2022-01-14 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. OK, I'm going to migrate this to a private matcher in the check that needs it. A related question is if it is possible to register private matchers with the parsing framework? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116518/new/ https://reviews.l

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

2022-01-15 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 400352. LegalizeAdulthood added a comment. Eliminate an intermediate class and move the IncludeInserter functionality into ClangTidyCheck with an enum to indicate opt-in to use of the IncludeInserter. CHANGES SINCE LAST ACTION https://reviews.llv

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

2022-01-15 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D117129#3246403 , @njames93 wrote: > There is a hurdle to overcome with this, Currently each check can have its > own include style - which doesn't really make sense. Hopefully people > actually just use the global

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

2022-01-16 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. I added some comments on the other review. Is constructing an `IncludeInserter` particularly expensive? I'm just curious. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117129/new/ https://reviews.llvm.org/D117129 _

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

2022-01-16 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp:21-26 +SharedPtrArrayMismatchCheck::SharedPtrArrayMismatchCheck( +StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context) {} + +void

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

2022-01-16 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. Except for a couple nits, LGTM. Comment at: clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.h:25-26 +public: + InefficientArrayTraversalCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Co

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

2022-01-16 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. In D117306#3245915 , @njames93 wrote: > How does this check play with the `modernize-make-shared` check? Wouldn't it be orthogonal? This check looks for existing `make_shared` usages, whereas modernize-make-shared adds

[PATCH] D117409: [clang-tidy] Move IncludeInserter into ClangTidyContext

2022-01-17 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:220-238 +llvm::Optional +ClangTidyCheck::createMainFileIncludeInsertion(StringRef Include) { + if (!Context->hasIncludeInserter()) { +// Only crash on debug builds +asser

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

2022-01-17 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. LGTM, but there are still some outstanding review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117306/new/ https://reviews.llvm.org/D117306 ___ cfe-commits m

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

2022-01-17 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added reviewers: alexfh, njames93, aaron.ballman, JonasToth. LegalizeAdulthood added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, xazax.hun, mgorny. LegalizeAdulthood requested review of this revision. This check

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

2022-01-17 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. RFC: Should we create a cppcoreguidelines alias since this implements "Enum.1: Prefer enumerations over macros "? This check tries to be very conservative so as to not generate false posi

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

2021-12-27 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: klimek. LegalizeAdulthood requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously if you wanted to match the statement associated with a case or default block,

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

2021-12-27 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment. I think while implementing this I got a little confused as to whether or not it was a narrowing or traversal matcher. It feels like a traversal matcher to me. If someone would confirm my reasoning, I'd appreciate it. That means I should probably move the uni

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

2021-12-28 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396392. LegalizeAdulthood added a comment. Update based on comments from lint check CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116328/new/ https://reviews.llvm.org/D116328 Files: clang/docs/LibASTMatchersReference.html clang/includ

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

2021-12-29 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396542. LegalizeAdulthood retitled this revision from "[ast-matchers] Add hasSubstmt() traversal matcher for caseStmt(), defaultStmt()" to "[ast-matchers] Add hasSubstmt() traversal matcher for caseStmt(), defaultStmt(), labelStmt()". LegalizeAdulth

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

2021-12-29 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396543. LegalizeAdulthood added a comment. Correct HTML copy/paste error for labelStmt() traversal CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116328/new/ https://reviews.llvm.org/D116328 Files: clang/docs/LibASTMatchersReference.html

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

2021-12-29 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 396556. LegalizeAdulthood added a comment. clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116328/new/ https://reviews.llvm.org/D116328 Files: clang/docs/LibASTMatchersReference.html clang/include/clang/ASTMatchers/ASTMatch

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

2021-12-29 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. Herald added subscribers: carlosgalvezp, kbarton, xazax.hun, nemanjai. LegalizeAdulthood requested review of this revision. Herald added a project: clang-tools-extra. Previously, any macro that didn't look like a

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

2021-12-30 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:83-105 +bool isConstantToken(const MacroDirective *MD) { + for (const auto &Token : MD->getMacroInfo()->tokens()) { +switch (Token.getKind()) { +case

<    1   2   3   4   5