[PATCH] D98497: [ASTMatchers] Don't forward matchers in MapAnyOf

njames93 created this revision. njames93 added reviewers: aaron.ballman, steveire, klimek. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Forwarding these means that if an r-value reference is passed, the matcher will be moved. H

[PATCH] D98521: [clang-tidy] Fix readability-identifer-naming duplicating prefix or suffix for replacements.

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh. Herald added a subscriber: xazax.hun. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. If a identifier has a correct prefix/suffix but a bad case, the

[PATCH] D98556: [ASTMatchers][Dynamic] Add missing matchers from Registry

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, steveire. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add the `fixedPointLiteral`, `hasAnyBody` and `templateArgumentLoc` to the dynamic matcher

[PATCH] D98583: [ASTMatchers] Fix documentation for hasAnyBody matcher

njames93 created this revision. njames93 added reviewers: baloghadamsoftware, aaron.ballman. Herald added a subscriber: rnkovacs. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Looks like a oversight when the matcher was added.

[PATCH] D97563: [clang-tidy] Enable modernize-concat-nested-namespaces also on headers

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. Herald added projects: LLVM, clang-tools-extra. Herald added a subscriber: llvm-commits. Is DAG required because the header file warnings are printed in a different order depending on thing

[PATCH] D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

njames93 marked 6 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:217 bool AllowEnablingAnalyzerAlphaCheckers; + bool SelfContainedDiags; }; steveire wrote: > The order of member

[PATCH] D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

njames93 updated this revision to Diff 330511. njames93 added a comment. Update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97121/new/ https://reviews.llvm.org/D97121 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.h clang-tools-extra/cl

[PATCH] D93164: [AST] Add generator for source location introspection

njames93 added a comment. It may be wise to alter this so there is no need for the python script. How about altering the dump tool to support outputting both json files and the cpp code needed for node introspection. Maybe have arguments to the tool `--json-output-path` and `--introspection-outp

[PATCH] D97563: [clang-tidy] Enable modernize-concat-nested-namespaces also on headers

njames93 added a comment. In D97563#2625513 , @lebedev.ri wrote: > I think the implicit question is: won't this regress headers that are meant > to be compatible with earlier standards? > Did the original review mention anything about this? Clang-tidy al

[PATCH] D98583: [ASTMatchers] Fix documentation for hasAnyBody matcher

This revision was automatically updated to reflect the committed changes. Closed by commit rG6badd3c52dc8: [ASTMatchers] Fix documentation for hasAnyBody matcher (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98583/new/ https:

[PATCH] D98556: [ASTMatchers][Dynamic] Add missing matchers from Registry

This revision was automatically updated to reflect the committed changes. Closed by commit rGff9120636e9c: [ASTMatchers][Dynamic] Add missing matchers from Registry (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98556/new/ htt

[PATCH] D98497: [ASTMatchers] Don't forward matchers in MapAnyOf

This revision was automatically updated to reflect the committed changes. Closed by commit rG74c270f33eb1: [ASTMatchers] Don't forward matchers in MapAnyOf (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98497/new/ https://revi

[PATCH] D98521: [clang-tidy] Fix readability-identifer-naming duplicating prefix or suffix for replacements.

This revision was automatically updated to reflect the committed changes. Closed by commit rG0333dde923c4: [clang-tidy] Fix readability-identifer-naming duplicating prefix or suffix for… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D131678: hicpp-signed-bitwise - Return location of the operand (and not of the operator beginning)

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:100 "%select{binary|unary}0 bitwise operator") - << IsUnary << SignedOperand->getSourceRange(); + << IsUnary << Location; } Seems pr

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 updated this revision to Diff 451856. njames93 added a comment. Refactor some of the impl. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130181/new/ https://reviews.llvm.org/D130181 Files: clang-tools-extra/clang-tidy/readability/CMakeL

[PATCH] D131623: [clang-tidy] Improve modernize-use-emplace check

njames93 added a comment. Just a general drive by comment here and doesn't affect this patch. This specifying containers logic is a little verbose, There may be a case to deprecate most of these options and just detect containers with an equivalent emplace* method at runtime. ===

[PATCH] D131590: Fixed page title for abseil-no-internal-dependencies check documentation

njames93 added a comment. In D131590#3713824 , @vladimir.plyashkun wrote: > In D131590#3713731 , @njames93 > wrote: > >> Can this be backported as well. > > I guess someone needs to do it, because i haven't got

[PATCH] D131590: Fixed page title for abseil-no-internal-dependencies check documentation

njames93 added a comment. In D131590#3716358 , @vladimir.plyashkun wrote: > Sure it's up to you, it's my github > account , but i don't think it's linked to JetBrains... Which email address would you like me to us

[PATCH] D131590: Fixed page title for abseil-no-internal-dependencies check documentation

This revision was automatically updated to reflect the committed changes. Closed by commit rG6c7b049f6eb7: [clang-tidy][docs] Fixed page title for abseil-no-internal-dependencies check… (authored by vladimir.plyashkun, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D131678: hicpp-signed-bitwise - Return location of the operand (and not of the operator beginning)

njames93 added a comment. The pre-merge bot is failing because clang-format hasn't been run. Can you please update the diff by running git clang-format first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131678/new/ https://reviews.llvm.org/D1316

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

njames93 removed reviewers: artem.tamazov, yaxunl, carlosgalvezp, MaskRay, mstorsjo, balazske. njames93 added a comment. This revision now requires review to proceed. What is the motivation for requiring these to be the arguments of call or construct expressions? Would be nice to add a note wit

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:31 + diag(MatchedExpr->getBeginLoc(), + "Enum is implictly converted to an integral.") + << MatchedExpr->getSourceRange(); diagnostics in clang aren't

[PATCH] D131859: [clang-tidy] Adds QualifierAlignment to misc-const-correctness.

njames93 added a comment. Duplicate of D131386 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131859/new/ https://reviews.llvm.org/D131859 ___ cfe-commits mailing list c

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

njames93 added a reviewer: Mordante. njames93 added a comment. In D131386#3722749 , @aaron.ballman wrote: > We leave formatting decisions in clang-tidy to clang-format and I don't think > we should deviate from that policy here without a very clear unde

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

njames93 updated this revision to Diff 452630. njames93 marked an inline comment as done. njames93 added a comment. Remove unneeded include and add Release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131386/new/ https://reviews.llvm.org/D1

[PATCH] D131678: hicpp-signed-bitwise - Return location of the operand (and not of the operator beginning)

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:100 + "%select{binary|unary}0 bitwise operator") + << IsUnary << OperatorLoc; } We still need the source range of the o

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

njames93 added a comment. Please can you run git clang-format over the patch to keep the pre-merge bot happy. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp:236-238 - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'si

[PATCH] D131985: clang-tidy: strip useless parens from `return` statements

njames93 added a comment. The idea of this check is good, but restricting it to only return statements seems baffling. A general check that could remove useless parens would have a lot more value. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1319

[PATCH] D131678: hicpp-signed-bitwise - Return location of the operand (and not of the operator beginning)

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. Would you like me to commit it on your behalf again? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131678/new/ https://reviews.llvm.org/D131

[PATCH] D131780: [clang-tidy] Do not trigger cppcoreguidelines-avoid-const-or-ref-data-members on lambda captures

njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp:191 + auto r5 = [&x5]{}; +} Can you add some cases with implicit capture (using [=] and [&]) Repository: rG LLVM Git

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:24-34 +using transformer::applyFirst; +using transformer::cat; +using transformer::changeTo; +using transformer::insertBefore; +using transformer::makeRule

[PATCH] D131780: [clang-tidy] Do not trigger cppcoreguidelines-avoid-const-or-ref-data-members on lambda captures

njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp:191 + auto r5 = [&x5]{}; +} njames93 wrote: > Can you add some cases with implicit capture (using [=] and [&]) I should

[PATCH] D131678: hicpp-signed-bitwise - Return location of the operand (and not of the operator beginning)

This revision was automatically updated to reflect the committed changes. Closed by commit rGfa8f8616028a: [clang-tidy] hicpp-signed-bitwise - Return location of the operand (and not of… (authored by vladimir.plyashkun, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D131623: [clang-tidy] Improve modernize-use-emplace check

njames93 added a comment. In D131623#3730833 , @joeywatts wrote: > Thanks for the review @njames93! This is my first contribution so I don't > think I have permission to land this myself, is there someone that can do > that for me? Sure thing, In D131

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 added a comment. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130181/new/ https://reviews.llvm.org/D130181 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D131623: [clang-tidy] Improve modernize-use-emplace check

This revision was automatically updated to reflect the committed changes. Closed by commit rGb8655f7ff286: [clang-tidy] Improve modernize-use-emplace check (authored by joeywatts, committed by njames93). Changed prior to commit: https://reviews.llvm.org/D131623?vs=451960&id=453899#toc Reposito

[PATCH] D131780: [clang-tidy] Do not trigger cppcoreguidelines-avoid-const-or-ref-data-members on lambda captures

njames93 accepted this revision. njames93 added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp:191 + auto r5 = [&x5]{}; +} carlosgalv

[PATCH] D132290: [clang-tidy] Skip unions in use-equals-default

njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp:38 + NU(const NU &Other) : Field(Other.Field) {} + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: use '= default' + // CHECK-FIXES: NU(const NU &Other

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

njames93 added a comment. Why are we only targeting sprintf, vsprintf would also suffer from the same pitfalls. Is there any hard reason that this check always suggests `snprintf` instead of `sprintf_s`, maybe have that dynamically controlled with an option. Comment at: clan

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 updated this revision to Diff 454275. njames93 added a comment. Add NestedControlFlow options This option can be used to silence diagnostics when there aren't any nested blocks inside the if statement. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:56 +bool isComparisonOperator(OverloadedOperatorKind Kind) { + return llvm::is_contained( + std::initializer_list{ 5chmidti wrote: >

[PATCH] D131686: [clang-tidy] Add test to cert-dcl58-cpp (NFC).

njames93 accepted this revision. njames93 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/D131686/new/ https://reviews.llvm.org/D131686 ___

[PATCH] D128401: [clang-tidy] Fixing a bug raising false alarms on static local variables in the Infinite Loop Checker

njames93 accepted this revision. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:166 + } + if (const auto *Call = dyn_cast(StmtNode)) { +const Decl *Callee = Call->getMethodDecl();

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

njames93 added a comment. The AllowWhileFlase option seems the wrong way to go about silencing do while(false) macros. Would it not make more sense to just ignore macros, or if you want more specificty don't warn on macros with a false condition? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

njames93 added a comment. Can you mention this change in the Release Notes (clang-tools-extra/docs/ReleaseNotes.rst). Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp:1059 priority_queue.emplace(Foo(13)); // CHECK-MESSAGES: :[[@LINE-1]]

[PATCH] D132294: [clang-tidy] Add check of sprintf with fixed size buffer

njames93 added a comment. Should this check be renamed to bugprone-sprintf-with-fixed-size-buffer? Have you thought about an option to flag all calls to sprintf as they are typically not recommended by a lot of coding practice guides. Obviously we couldn't generate a fix-it as the buffer size ma

[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp:27-30 + for (const int *p_local1 : p_local0) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local1' of type 'const int *' can be dec

[PATCH] D132786: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood, gribozavr2, dodohand. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald a

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

njames93 added a comment. Would I be correct in assuming you have uploaded the wrong diff here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133942/new/ https://reviews.llvm.org/D133942 ___ cfe-commits

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

njames93 added a comment. In D132461#3773792 , @carlosgalvezp wrote: > @njames93 Friendly ping. The patch has addressed all comments and remained > unchanged for 2 weeks. I would like to land it latest next week. If you are > happy with the patch, plea

[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

njames93 added a comment. I feel like this matcher could do with some unit testing. I'd say create a test file in `clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp` Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:145 auto UnusedInCompou

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

njames93 requested changes to this revision. njames93 added a comment. This revision now requires changes to proceed. There's some merit in this, like wrapping the previous statement in braces.. However, clang-tidy should not focus on the formatting aspect as we have clang-format built in to add

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

njames93 added a comment. In D122078#3797519 , @Izaron wrote: > The new file was failing with message > > CHECK-FIXES, CHECK-MESSAGES or CHECK-NOTES not found in the input > > So I had to add an additional urrelevant check. > I added a check for constev

[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM, just with one small testing nit Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp:533 + int *np_local0[2] = {nullptr, nullp

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stddef.h:12 + +using size_t = long long unsigned; + If this is all the file is used for, and it's only used for this one test file, can probably remove this he

[PATCH] D133371: [clang-tidy][WIP] create API for extracting option types.

njames93 updated this revision to Diff 461950. njames93 added a comment. Add a test to demonstraight behaviour Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133371/new/ https://reviews.llvm.org/D133371 Files: clang-tools-extra/clang-tidy/ClangTi

[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

njames93 added a subscriber: kwk. njames93 added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp:2 +#include "utils/Matchers.h" +#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h" + @kwk I seem to recall you saying

[PATCH] D134549: [clang] Add fix-it note to defaulted-function-deleted warning

njames93 created this revision. njames93 added a reviewer: aaron.ballman. Herald added a subscriber: martong. Herald added a reviewer: shafik. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Adds a fix

[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

njames93 added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp:2 +#include "utils/Matchers.h" +#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h" + kwk wrote: > njames93 wrote: > > @kwk I seem to recall you saying

[PATCH] D134590: [clang-tidy] Fix a false positive in readability-simplify-boolean-expr

njames93 created this revision. njames93 added reviewers: aaron.ballman, LegalizeAdulthood. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.

[PATCH] D134592: [clang-tidy] Add option to control floating point binary operators in readability-simplify-boolean-expr

njames93 created this revision. njames93 added reviewers: aaron.ballman, LegalizeAdulthood. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.

[PATCH] D134590: [clang-tidy] Fix a false positive in readability-simplify-boolean-expr

This revision was automatically updated to reflect the committed changes. Closed by commit rG8c783b8ec78e: [clang-tidy] Fix a false positive in readability-simplify-boolean-expr (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D134549: [clang] Add fix-it note to defaulted-function-deleted warning

njames93 added inline comments. Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2291 Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // LocEnd + Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 3)); // Default/DeleteLoc Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp:

[PATCH] D134549: [clang] Add fix-it note to defaulted-function-deleted warning

njames93 updated this revision to Diff 462684. njames93 marked an inline comment as done. njames93 added a comment. Add release notes and fix issue in ASTWriterDecl. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134549/new/ https://reviews.llvm.org

[PATCH] D134549: [clang] Add fix-it note to defaulted-function-deleted warning

njames93 updated this revision to Diff 462685. njames93 added a comment. Rebase fixing merge conflicts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134549/new/ https://reviews.llvm.org/D134549 Files: clang/docs/ReleaseNotes.rst clang/include

[PATCH] D134592: [clang-tidy] Add option to control floating point binary operators in readability-simplify-boolean-expr

njames93 added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:155 + + Added an option SafeFloatComparisons to control negating binary operators + with floating point operands. Eugene.Zelenko wrote: > Please enclose `SafeFloatComparisons`

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

njames93 added a comment. I have a feeling the default should be to only warm in loops otherwise this could get noisy. Though setting it as the default you'd likely want to change the name to something along the lines of WarmOutsideLoops. Comment at: clang-tools-extra/clang-

[PATCH] D133413: [clang-tidy] Fix crashes on `if consteval` in readability checks

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:471 auto *If = cast(*First); if (!If->hasInitStorage() && !If->hasVarStorage()) { ExprAndBool ThenReturnBool = Probably

[PATCH] D133413: [clang-tidy] Fix crashes on `if consteval` in readability checks

njames93 accepted this revision. njames93 added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133413/new/ https://reviews.llvm.org/D133413 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D134549: [clang] Add fix-it note to defaulted-function-deleted warning

njames93 updated this revision to Diff 465042. njames93 added a comment. Address @martong's comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134549/new/ https://reviews.llvm.org/D134549 Files: clang/docs/ReleaseNotes.rst clang/include/cl

[PATCH] D134549: [clang] Add fix-it note to defaulted-function-deleted warning

njames93 updated this revision to Diff 465045. njames93 added a comment. Rebase to trunk Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134549/new/ https://reviews.llvm.org/D134549 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/Decl

[PATCH] D134549: [clang] Add fix-it note to defaulted-function-deleted warning

This revision was automatically updated to reflect the committed changes. Closed by commit rG1376c73927dc: [clang] Add fix-it note to defaulted-function-deleted warning (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134549/new/

[PATCH] D135404: [clang-tidy] Add a checker for converting into C++17 variable template type traits

njames93 added a comment. Why limit this to just the value type traits, makes sense to also support type type traits. Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:73 +const auto *RecordQualifier = +cast_if_present( +Qualifier->

[PATCH] D135367: [clang-tidy] Dump effective diagnostics level in YAML output

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM, just maybe include a test case where a warning from a clang-tidy check is promoted to an error as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM, just one small nit. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp:3 + +// Note: this test expects no diagnostics, but

[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:81 + isBaseOf(TPointeeUQTy, BPointeeUQTy)) && + (TCVR == 0 || (BCVR ^ TCVR) == 0 || (BCVR & TCVR) > BCVR)) { +TypesToDelete.push_back(T); --

[PATCH] D140018: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

njames93 accepted this revision. njames93 added a comment. LGTM, thank you for the patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140018/new/ https://reviews.llvm.org/D140018 ___ cfe-commits mailing

[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

njames93 added a comment. In D138655#4007822 , @carlosgalvezp wrote: > Now that I think a bit better about this I wonder - does it really make sense > that we increase the complexity of the check to cover for cases where code > does not compile? If it

[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

njames93 added a comment. In D138655#4012557 , @Sockke wrote: > Improved the test file. Just a general workflow comment, can you please mark comments as done when they are addressed, or, if you feel the comment in unjustified, explain why. CHANGES SIN

[PATCH] D140968: [clang-tidy] Add check for passing the result of `std::string::c_str` to `strlen`

njames93 requested changes to this revision. njames93 added a comment. This revision now requires changes to proceed. I don't see the appear of this check as its a situation that I doubt ever appears in code bases. If there are open source code bases where this is a known problem can you please

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

njames93 added a comment. Can you explain the reasoning of why this approach is better than current approach where checks can use global options(`Options.getLocalOrGlobal("HeaderFileExtensions", utils::defaultHeaderFileExtensions())`) to access the same information? Some checks also have the g

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

njames93 added a comment. You do raise a good point about duplication, Therefore does it make sense to also do the same with the IncludeStyle as lots of checks add new includes. Though there is a precedent to do away with LLVM/Google style, as clang-format should be responsible for reordering in

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:34 +static StringRef TrimFirstChar(StringRef x) { return x.substr(1); } + This seems a bit of a needless change, if you want to remove the newline, just in date th

[PATCH] D135405: fix handling of braced-init temporaries for modernize-use-emplace

njames93 added a comment. In D135405#4039820 , @BigPeet wrote: > Ping. > > (Sorry, it's my first time contributing to LLVM and I simply don't know what > happens next. Do I need to do anything? Or is it just waiting to get merged > at some point?) My a

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

njames93 requested changes to this revision. njames93 added a comment. This revision now requires changes to proceed. Can you run this through clang format to make sure the pre merge is happy and address those nits Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/Av

[PATCH] D135404: [clang-tidy] Add a checker for converting into C++17 variable template type traits

njames93 added a comment. I'd also suggest using the `IgnoreUnlessSpelledInSource` traversal kind here. Given that most of the time these will be instantiated we don't want to warn and emit a fix for each instantiation, Or you could just add `unless(isInTemplateInstantiation())` to your matcher

[PATCH] D135822: [clang-tidy] Add option `RewriteVoidReturnType` to `modernize-use-trailing-return-type`

njames93 added a comment. The default behaviour of this check should be transform void return types as that's how it has been since the check was first created. Adding an option which defaults to changing this behaviour would be harmful to current users of this check. Commen

[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, alexfh. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp:7 +// +// Author: Fabian Keßler "Febbe" +//===--===// Author ditto.

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

njames93 added a comment. I'm not entirely sure why this belongs in the modernize module given anonymous namespaces have been in c++ forever, maybe its more of a misc check? Also the modernize checks are meant to actually emit fixes(ignore the c arrays check :) ), Right now, this only warn, it

[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

njames93 updated this revision to Diff 473028. njames93 marked 3 inline comments as done. njames93 added a comment. Address documentation comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137302/new/ https://reviews.llvm.org/D137302 Files:

[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

njames93 updated this revision to Diff 473427. njames93 added a comment. Fix various crashes on real world code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137302/new/ https://reviews.llvm.org/D137302 Files: clang-tools-extra/clang-tidy/modern

[PATCH] D137491: [clang][NFC] Use c++17 style variable type traits

njames93 created this revision. njames93 added reviewers: aaron.ballman, rsmith. Herald added subscribers: steakhal, martong. Herald added a reviewer: shafik. Herald added a reviewer: NoQ. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang. Herald

[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

njames93 updated this revision to Diff 473439. njames93 added a comment. Fix crash when running over llvm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137302/new/ https://reviews.llvm.org/D137302 Files: clang-tools-extra/clang-tidy/modernize/CM

[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.h:17 +namespace tidy { +namespace modernize { + tschuett wrote: > Why do you refuse to use nested namespaces in `modernize` ? Because the add_new_check script hasn'

[PATCH] D137491: [clang][NFC] Use c++17 style variable type traits

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG108e41d96246: [clang][NFC] Use c++17 style variable type traits (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D137491?

[PATCH] D137794: [clangd] Enable configuring include insertions

njames93 created this revision. njames93 added reviewers: sammccall, kadircet, nridge. Herald added a subscriber: arphaman. Herald added a project: All. njames93 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-ext

[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

njames93 added inline comments. Comment at: clang/include/clang/AST/Decl.h:546 + + enum Flags : unsigned { F_Inline = 1 << 0, F_Nested = 1 << 1 }; + aaron.ballman wrote: > I'm not really loving the `F_` prefixes -- the enumerators are already > privately scoped

[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

njames93 added inline comments. Comment at: clang/test/AST/ast-dump-decl.cpp:1 -// Test without serialization: -// RUN: %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -fms-extensions \ -// RUN: -ast-dump -ast-dump-filter Test %s \ -// RUN: | FileCheck --strict-whitespace %s -// -

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

njames93 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-copy-on-last-use.rst:50 + + A string specifying which include-style is used, `llvm` or `google`. Default + is `llvm`. Febbe wrote: > Eugene.Zelenko w

<    9   10   11   12   13   14   15   16   17   18   >