[PATCH] D147062: [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this

carlosgalvezp updated this revision to Diff 509064. carlosgalvezp added a comment. Fix typo in doc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147062/new/ https://reviews.llvm.org/D147062 Files: clang-tools-extra/clang-tidy/cppcoreguidelines

[PATCH] D146875: [clang-tidy] Fix example provided by add_new_check.py

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:278 with io.open(filename, 'w', encoding='utf8', newline='\n') as f: -f.write("""// RUN: %%check_clang_tidy %%s %(check_name_dashes)s %%t +f.write("""// RUN: %%check_clang_ti

[PATCH] D146887: [clang-tidy] Fix if-constexpr false-positive in readability-misleading-indentation

carlosgalvezp requested changes to this revision. carlosgalvezp added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation.cpp:1 -// RUN: %check_clang_tidy %s readability-mislead

[PATCH] D147062: [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this

carlosgalvezp added a comment. In D147062#4228403 , @ccotter wrote: > If https://github.com/isocpp/CppCoreGuidelines/issues/2060 is accepted to > only consider `[=]`, then I assume we'd want to change the default value of > IgnoreCaptureDefaultByReferen

[PATCH] D147062: [clang-tidy] Add option to ignore capture default by reference in cppcoreguidelines-avoid-capture-default-when-capturing-this

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4d4c0f973460: [clang-tidy] Add option to ignore capture default by reference in… (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo

[PATCH] D146887: [clang-tidy] Fix if-constexpr false-positive in readability-misleading-indentation

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Looks great, thanks for fixing! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146887/new/ https://reviews.llvm.org/D146887 __

[PATCH] D146875: [clang-tidy] Fix example provided by add_new_check.py

carlosgalvezp added a comment. Thanks for digging into this! It looks a bit strange to me: > Usually developers have to preview a code diff (before vs after apply the > fix) to understand what the fix does before applying a fix. This is not quite true - clang-tidy displays the fix it hint when

[PATCH] D144217: [clang-tidy] Fix false-positive in readability-container-size-empty

carlosgalvezp added a comment. Thanks for the fix! Looks good, have a couple minor comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:88-89 } + AST_MATCHER(CXXConstructExpr, isDefaultConstruction) { return Node.getConstructor()->isD

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

carlosgalvezp added a comment. Thanks for the fix! I have some suggestions for improved readability. Comment at: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:97 /// Returns the magnitude bits of an integer type. +static std::pair Havi

[PATCH] D144790: [clang-tidy] readability-identifier-naming.HungarianNotation: rename CharPrinter to CharPointer

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Good catch, thanks for fixing! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144790/new/ https://reviews.llvm.org/D144790 ___

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

carlosgalvezp added a comment. In D142587#4152943 , @Eugene.Zelenko wrote: > @carlosgalvezp: Sorry, there are too much Clang specifics in this patch, so I > could not be reviewer. No prob, thanks for letting us know :) Repository: rG LLVM Github Mo

[PATCH] D143851: [clang-tidy] Tweak 'rule of 3/5' checks to allow defaulting a destructor outside the class.

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the contribution! Do you have commit rights or would you like that we land it for you? If so, please provide name and email for attribution. Repository: rG LL

[PATCH] D144790: [clang-tidy] readability-identifier-naming.HungarianNotation: rename CharPrinter to CharPointer

This revision was automatically updated to reflect the committed changes. Closed by commit rG00a71cb47c36: [clang-tidy] readability-identifier-naming.HungarianNotation: rename… (authored by amurzeau, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

carlosgalvezp added a comment. LGTM, minor comments. I'm not familiar with the implementation so I'm not very confident reviewing it, would be good to get some more expert eyes on it. Tests look solid! Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:31

[PATCH] D144594: [clang-tidy] Fix bugprone-copy-constructor-init documentation

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Great improvement, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144594/new/ https://reviews.llvm.org/D144594 ___

[PATCH] D143375: clang-tidy: Count template constructors in modernize-use-default-member-init

carlosgalvezp added a comment. Looks good, thanks! Please document change in the Release Notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143375/new/ https://reviews.llvm.org/D143375 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142587/new/ https://reviews.llvm.org/D142587

[PATCH] D145303: clang-tidy altera-id-dependent-backward-branch: print notes after warning

carlosgalvezp added a comment. Thanks for the fix! Can you land the patch or would need that we do it for you? If so please provide Github name and email for attribution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145303/new/ https://reviews.ll

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

carlosgalvezp accepted this revision. carlosgalvezp added a comment. Sounds good, should we land this? If you don't have commit rights, please let us know Github name and email for attribution. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144216/new/ https://reviews.llvm.org/D144216

[PATCH] D143375: clang-tidy: Count template constructors in modernize-use-default-member-init

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Thanks for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143375/new/ https://reviews.llvm.org/D143375 ___ cfe-commi

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string:4-5 + +// For size_t +#include + Sorry I just noticed this - should we keep using the `__SYZE_TYPE__` macro that existed in the previous patch? The

[PATCH] D145303: clang-tidy altera-id-dependent-backward-branch: print notes after warning

This revision was automatically updated to reflect the committed changes. Closed by commit rGc6d195b366c8: clang-tidy altera-id-dependent-backward-branch: print notes after warning (authored by yeputons-gh, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

carlosgalvezp added a comment. In D144216#4169764 , @mikecrowe wrote: >> I will double check that this is true once my current build is complete. > > Yes, it's true. I stuck a `#error` in > `clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/strin

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

This revision was automatically updated to reflect the committed changes. Closed by commit rGae25e2f19dec: [clang-tidy] Extract string header from redundant-string-cstr checker (authored by mikecrowe, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

carlosgalvezp added a comment. In D144216#4169781 , @mikecrowe wrote: > In D144216#4169772 , @carlosgalvezp > wrote: > >> In D144216#4169764 , @mikecrowe >> wrote: >> >>

[PATCH] D145310: [clang-tidy] Make readability-container-data-pointer use header

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, would you mind rebasing to get the pre-merge jobs green? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145310/new/ https://reviews.llvm.org/D145310 __

[PATCH] D145313: [clang-tidy] Make readability-container-size-empty check using header

carlosgalvezp added a comment. Could you upload the patch with full context? I believe you need to do something like `git show HEAD -U99` as per the guidelines . Otherwise `arc diff` should do the job automatically. Reason I ask is that I cannot see t

[PATCH] D145312: [clang-tidy] Make readability-string-compare check use header

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp:4-8 -template -class allocator {}; -template -class char_traits {}; -template , typename A = std::allocator> Would it make sense to add

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

carlosgalvezp accepted this revision. carlosgalvezp 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/D145304/new/ https://reviews.llvm.org/D145304

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

carlosgalvezp added a comment. It seems I'm having trouble to download the patch with arcanist, would you mind rebasing on top of the main branch? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145304/new/ https://reviews.llvm.org/D145304

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9fda83222431: [clang-tidy] altera-id-dependent-backward-branch: refactor test (authored by yeputons-gh, committed by carlosgalvezp). Repository:

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

carlosgalvezp added a comment. In D145304#4169810 , @yeputons-gh wrote: > Absolutely, give we few minutes. Maybe it conflicts with the previous > revision that has already landed? I believe so. I'm quite new to arcanist so I might be missing some trick

[PATCH] D145477: run-clang-tidy.py should only search for the clang-apply-replacements if really needed

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, `clang_apply_replacements_binary` is only used in a `if args.fix` block. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145477

[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing

carlosgalvezp added a comment. Looks good, just a small comment! Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp:102 + +struct BaseA { +virtual ~BaseA() {} Please briefly document this test case, i.e. why in this case this

[PATCH] D145310: [clang-tidy] Make readability-container-data-pointer use header

This revision was automatically updated to reflect the committed changes. Closed by commit rGefda335bf5be: [clang-tidy] Make readability-container-data-pointer use header (authored by mikecrowe, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144206/new/ https://reviews.llvm.org/D144206

[PATCH] D145312: [clang-tidy] Make readability-string-compare check use header

carlosgalvezp added a comment. @PiotrZSL I believe you have landed some patches of this chain. Would you mind sharing how you do it? I'm not an expert in Phabricator and simply doing `arc patch D145312` leads to cherry-pick conflicts, and when I solve them I don't end up having this patch as `H

[PATCH] D145312: [clang-tidy] Make readability-string-compare check use header

carlosgalvezp added a comment. @mikecrowe I seems I cannot land this until it's rebased on top of `main`, would you be able to do that? Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145312/new/ https://reviews.llvm.org/D145312 ___ cf

[PATCH] D145312: [clang-tidy] Make readability-string-compare check use header

carlosgalvezp added a comment. In D145312#4187247 , @PiotrZSL wrote: > In D145312#4187195 , @carlosgalvezp > wrote: > >> @PiotrZSL I believe you have landed some patches of this chain. Would you >> mind sharing

[PATCH] D121584: [clang-format] Correctly recognize arrays in template parameter list.

carlosgalvezp added a comment. Herald added a project: clang-format. In D121584#3404704 , @curdeius wrote: > Yes, let's revert this. I should have a fix soon though. @curdeius Did you manage to find a fix suitable for both use cases? Repository: rG L

[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the check! Please fix the missing space comment before landing. Comment at: clang-tools-extra/clang-tidy/readability/OperatorsRepresentationCh

[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

carlosgalvezp added a comment. In D144522#4236636 , @PiotrZSL wrote: > @carlosgalvezp > Thank you, by any chance would you be able to look into other reviews ? Doing my best to catch up, I have a long list of patches to review! Unfortunately very limite

[PATCH] D146881: [clang-tidy] Fix findNextTokenSkippingComments & rangeContainsExpansionsOrDirectives

carlosgalvezp added a comment. I'm surprised we don't have unit tests that catch this? Comment at: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp:84-85 +Lexer::findNextToken(Start, SM, LangOpts); +if (!CurrentToken || !CurrentToken->is(tok::comment)) + retur

[PATCH] D146881: [clang-tidy] Fix findNextTokenSkippingComments & rangeContainsExpansionsOrDirectives

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. > It's usually tested indirectly by checks... But this code is used I think > only in 1-2 checks. I see. I'm still trying to understand why we haven't seen this before. If the f

[PATCH] D147379: [clang-tidy] Disable misc-definitions-in-headers for declarations in anonymous namespaces

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Anonymous namespac

[PATCH] D147315: [clang-tidy] support unscoped enumerations in readability-static-accessed-through-instance

carlosgalvezp added a comment. Would it make sense to add a scoped enum to the test, to ensure it doesn't warn/fix? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147315/new/ https://reviews.llvm.org/D147315 ___

[PATCH] D147379: [clang-tidy] Disable misc-definitions-in-headers for declarations in anonymous namespaces

This revision was automatically updated to reflect the committed changes. Closed by commit rG2469bd98a772: [clang-tidy] Disable misc-definitions-in-headers for declarations in anonymous… (authored by carlosgalvezp). Changed prior to commit: https://reviews.llvm.org/D147379?vs=510233&id=510257#

[PATCH] D145138: [clang-tidy] Implement FixIts for C arrays

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp:588 +: ClangTidyCheck(Name, Context), + IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSo

[PATCH] D147411: [clang-tidy] Fix readability-static-accessed-through-instance check for anonymous structs

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:78 + // Ignore anonymous structs/classes + if (StringRef(BaseTypeName).contains("(unnamed ")) +return; AMS21 wrote: > I wonder

[PATCH] D147411: [clang-tidy] Fix readability-static-accessed-through-instance check for anonymous structs

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:78 + // Ignore anonymous structs/classes + if (StringRef(BaseTypeName).contains("(unnamed ")) +return; carlosgalvezp wrote: > A

[PATCH] D147563: [clang-tidy] Deprecate cert-dcl21-cpp

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. It is no longer pa

[PATCH] D147563: [clang-tidy] Deprecate cert-dcl21-cpp

This revision was automatically updated to reflect the committed changes. Closed by commit rG712dfec1781d: [clang-tidy] Deprecate cert-dcl21-cpp (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147563/new/ https://reviews.l

[PATCH] D147563: [clang-tidy] Deprecate cert-dcl21-cpp

carlosgalvezp added a comment. In D147563#4244227 , @Eugene.Zelenko wrote: > I think will be good idea to track deprecated features on GitHub issues with > something like `deprecate in XYZ` label(s). Great idea, I started to have the feeling we have qu

[PATCH] D144036: [clang-tidy] Add bugprone-enum-to-bool-conversion check

carlosgalvezp added a comment. Such a great check, thanks! I have very minor comments, the most debatable one about the name of the check (but no strong opinion). Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:107 +CheckFactories.registerCheck( +

[PATCH] D98416: [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Herald added a subscriber: PiotrZSL. Herald added a project: All. LGTM, is there anything that should be fixed before merging? Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D146520: [clang-tidy] Fix checks filter with warnings-as-errors

carlosgalvezp accepted this revision. carlosgalvezp added a comment. Having another look and reading your comments I got a better understanding of the situation: - YAML output did not have correct warnings as errors. - Thus a fix was introduced in ClangTidyDiagnosticConsumer, however this was a

[PATCH] D146904: [clang-tidy] Fix extern fixes in readability-redundant-declaration

carlosgalvezp added a comment. Looks good, thanks for the fix! Do we think it's worth documenting in the Release Notes? Comment at: clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp:86 + Result.Nodes.getNodeAs("extern"); + Extern &&

[PATCH] D146904: [clang-tidy] Fix extern fixes in readability-redundant-declaration

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Looks great, thanks! Comment at: clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp:86 + Result.Nodes.getNodeAs("extern"); +

[PATCH] D146520: [clang-tidy] Fix checks filter with warnings-as-errors

carlosgalvezp added a comment. It would still be good to add some test that ensures this does not happen again in the future, though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146520/new/ https://reviews.llvm.org/D146520 _

[PATCH] D147876: [WIP][clang-tidy] Support introducing checks as a list in the config file

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Fixes https://gith

[PATCH] D147876: [WIP][clang-tidy] Support introducing checks as a list in the config file

carlosgalvezp updated this revision to Diff 511970. carlosgalvezp added a comment. Add tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147876/new/ https://reviews.llvm.org/D147876 Files: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

[PATCH] D147876: [WIP][clang-tidy] Support specifying Checks as a list in the config file

carlosgalvezp updated this revision to Diff 511972. carlosgalvezp retitled this revision from "[WIP][clang-tidy] Support introducing checks as a list in the config file" to "[WIP][clang-tidy] Support specifying Checks as a list in the config file". carlosgalvezp added a comment. Update commit me

[PATCH] D147876: [WIP][clang-tidy] Support specifying Checks as a list in the config file

carlosgalvezp updated this revision to Diff 512036. carlosgalvezp added a comment. Herald added a subscriber: arphaman. - Simplify code with llvm:join. - Add documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147876/new/ https://reviews.l

[PATCH] D147876: [clang-tidy] Support introducing checks as a list in the config file Specifying checks as a string is convenient for quickly using clang-tidy to run a handful of checks. However it i

carlosgalvezp updated this revision to Diff 512037. carlosgalvezp retitled this revision from "[WIP][clang-tidy] Support specifying Checks as a list in the config file" to "[clang-tidy] Support introducing checks as a list in the config file Specifying checks as a string is convenient for quickl

[PATCH] D147876: [clang-tidy] Support introducing checks as a list in the config file Specifying checks as a string is convenient for quickly using clang-tidy to run a handful of checks. However it

carlosgalvezp updated this revision to Diff 512038. carlosgalvezp retitled this revision from "[clang-tidy] Support introducing checks as a list in the config file Specifying checks as a string is convenient for quickly using clang-tidy to run a handful of checks. However it is not suitable for

[PATCH] D147876: [clang-tidy] Support introducing checks as a list in the config file

carlosgalvezp marked an inline comment as done. carlosgalvezp added a comment. In D147876#4254451 , @njames93 wrote: > From an architectural point of view, is there any reason we don't change the > backend to treat checks internally as a vector? > > `cla

[PATCH] D147876: [clang-tidy] Support specifying checks as a list in the config file

carlosgalvezp updated this revision to Diff 512114. carlosgalvezp marked 2 inline comments as done. carlosgalvezp retitled this revision from "[clang-tidy] Support introducing checks as a list in the config file" to "[clang-tidy] Support specifying checks as a list in the config file". carlosgalv

[PATCH] D147876: [clang-tidy] Support specifying checks as a list in the config file

carlosgalvezp marked an inline comment as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:129-141 +// Special case for reading from YAML +// Must support reading from both a string or a list +Input &I = reinterp

[PATCH] D147918: [clang-tidy] Added IgnoreVirtual option to misc-unused-parameters

carlosgalvezp added a comment. Looks good, small comments! Comment at: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp:183 +if (Method->isLambdaStaticInvoker() || +(IgnoreVirtual && Method->isVirtual())) return; Since these 2 cond

[PATCH] D147906: [clang-tidy] Avoid float compare in bugprone-incorrect-roundings

carlosgalvezp added a comment. Looks good, small suggestions! I appreciate having this small patch prior to the next one fixing the actual issue. Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectRoundingsCheck.cpp:28 + if ((&Node.getSemantics()) == &llvm::APFloat::I

[PATCH] D147908: [clang-tidy] Add support for long double in bugprone-incorrect-roundings

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Nice cleanup and increased generality, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147908/new/ https://reviews.llvm.o

[PATCH] D147876: [clang-tidy] Support specifying checks as a list in the config file

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:129-141 +// Special case for reading from YAML +// Must support reading from both a string or a list +Input &I = reinterpret_cast(IO); +if (isa(I.getCurrentNode()) |

[PATCH] D147929: [clang-tidy] Fix handling of UseAssignment option in cppcoreguidelines-prefer-member-initializer

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:134 + Options.get("UseAssignment", + OptionsView("modernize-use-default-member-init", +

[PATCH] D147876: [clang-tidy] Support specifying checks as a list in the config file

This revision was automatically updated to reflect the committed changes. Closed by commit rG132f1d31fd66: [clang-tidy] Support specifying checks as a list in the config file (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D147876: [clang-tidy] Support specifying checks as a list in the config file

carlosgalvezp added a comment. Thanks for the review @njames93 ! Do you think it makes sense that we deprecate the string format, so that we only support the list format? To be fully removed in clang-tidy 19. Only for config file, for `--checks` we can still support string. This would allow us

[PATCH] D147876: [clang-tidy] Support specifying checks as a list in the config file

carlosgalvezp added a comment. In D147876#4256182 , @njames93 wrote: > In D147876#4256155 , @carlosgalvezp > wrote: > >> Thanks for the review @njames93 ! Do you think it makes sense that we >> deprecate the str

[PATCH] D147876: [clang-tidy] Support specifying checks as a list in the config file

carlosgalvezp added a comment. > When the `.clang-tidy` file is checked into the source control(git). It means > that anyone who contributes to the project will need to ensure that they have > a version of clang-tidy that will be able to read the file. > This can cause problems as binaries of cl

[PATCH] D148318: [clang-tidy] Add `performance-dont-use-endl` check

carlosgalvezp added a comment. For consistency with other checks, please rename it to `performance-avoid-endl`. You can use the `rename_check.py` to easily do this :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148318/new/ https://reviews.llvm.o

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, shchenz, kbarton, xazax.hun, nemanjai. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp updated this revision to Diff 513584. carlosgalvezp added a comment. Remove excessive newlines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148340/new/ https://reviews.llvm.org/D148340 Files: clang-tools-extra/clang-tidy/cppcoreg

[PATCH] D148318: [clang-tidy] Add `performance-avoid-endl` check

carlosgalvezp added a comment. Thank you for the contribution! Looks good in general, have minor comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance/avoid-endl.rst:1 +.. title:: clang-tidy - performance-avoid-endl + Please wrap file to 8

[PATCH] D148318: [clang-tidy] Add `performance-avoid-endl` check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance/avoid-endl.cpp:47 + std::cout << "World" << std::endl; + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not use std::endl with iostreams; use '\n' instead + std::cerr <<

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp updated this revision to Diff 513705. carlosgalvezp added a comment. Shorten check name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148340/new/ https://reviews.llvm.org/D148340 Files: clang-tools-extra/clang-tidy/cppcoreguidelin

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp marked an inline comment as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:55 +AvoidByValueCaptureDefaultWhenCapturingThisCheck>( + "cppcoreguidelines-avoid-by-va

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:55 +AvoidByValueCaptureDefaultWhenCapturingThisCheck>( + "cppcoreguidelines-avoid-by-value-capture-default-when-capturing-this");

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp marked 3 inline comments as done. carlosgalvezp added a comment. > To be honest for me this check still looks too strict, for example it will > warn when we capture this explicitly, and we don't use any class fields, but > we use some local variables captured by value and for examp

[PATCH] D147924: [clang-tidy] Exclude template instantiations in modernize-use-override

carlosgalvezp added a comment. Looks good, some small comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-templates.cpp:15 + virtual void foo(); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final

[PATCH] D147081: [clang-tidy] Fix issues in bugprone-assert-side-effect

carlosgalvezp added a comment. You mention in the commit message that it fixes #25569, but that seems unrelated (and is already closed)? Personally I find it hard to review 2 github issue fixes + 1 NFC (moving the assert code to assert.h) on the same patch, would it be possible to split to hav

[PATCH] D147924: [clang-tidy] Exclude template instantiations in modernize-use-override

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-templates.cpp:46 + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override] + // CH

[PATCH] D147924: [clang-tidy] Exclude template instantiations in modernize-use-override

carlosgalvezp accepted this revision. carlosgalvezp added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-templates.cpp:46 + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp updated this revision to Diff 513873. carlosgalvezp marked 2 inline comments as done. carlosgalvezp added a comment. Rename check to latest proposal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148340/new/ https://reviews.llvm.org/D1

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp updated this revision to Diff 513874. carlosgalvezp added a comment. Fix check ordering. Align check documentation across header, .rst file and Release Notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148340/new/ https://reviews.

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp updated this revision to Diff 513875. carlosgalvezp added a comment. Remove excessive whitespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148340/new/ https://reviews.llvm.org/D148340 Files: clang-tools-extra/clang-tidy/cppcor

[PATCH] D148418: [clang-tidy] Fix typedefs handling in bugprone-dangling-handle

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp:55-56 - operator basic_string_view() const noexcept; + typedef basic_string_view str_view; + operator str_view() const noexcept; Right

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

This revision was automatically updated to reflect the committed changes. Closed by commit rGeedbe81b1c6d: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this… (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D148424: [clang-tidy][NFC] Improve doc of cppcoreguidelines-misleading-capture-default-by-value

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, shchenz, kbarton, xazax.hun, nemanjai. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-

[PATCH] D148418: [clang-tidy] Fix typedefs handling in bugprone-dangling-handle

carlosgalvezp accepted this revision. carlosgalvezp added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp:55-56 - operator basic_string_view() const noexcept; + typedef basi

[PATCH] D148424: [clang-tidy][NFC] Improve doc of cppcoreguidelines-misleading-capture-default-by-value

This revision was automatically updated to reflect the committed changes. Closed by commit rGfa3de2ed2964: [clang-tidy][NFC] Improve doc of cppcoreguidelines-misleading-capture-default… (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp marked an inline comment as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.h:1 +//===--- MisleadingCaptureDefaultByValueCheck.h - clang-tidy*- C++ +//-*-===// --

[PATCH] D98416: [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive

This revision was automatically updated to reflect the committed changes. Closed by commit rG4530c3bc4897: [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive (authored by njames93, committed by carlosgalvezp). Changed prior to commit: https://reviews.llvm.org/D98416?vs=329

<    1   2   3   4   5   6   7   8   9   >