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

carlosgalvezp updated this revision to Diff 493110. carlosgalvezp added a comment. Wrap trimming functionality in desc function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.org/D141144 Files: clang-tools-extra/

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

carlosgalvezp marked an inline comment as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:34 +static StringRef TrimFirstChar(StringRef x) { return x.substr(1); } + ClockMan wrote: > Looks like usecase o

[PATCH] D142816: [clang-tidy] Add --list-unique-checks command

carlosgalvezp added a comment. This is not as trivial as one might think. Check aliases aren't necessarily identical, as they can differ based on configuration settings, becoming essentially different checks. I have a WIP patch to allow users to avoid running

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

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:81 + Lambda->getCaptureDefaultLoc(), + "lambdas that capture 'this' should not specify a capture default"); + -

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

carlosgalvezp accepted this revision. carlosgalvezp added a comment. Thanks for updating the comment! Since I marked it as approved last time, I will land this in the next couple of days if no more comments come up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

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

carlosgalvezp added a comment. @Sockke ping, can you land this or should we do it for you? If so, please let us know what name and email we should use for attribution. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138655/new/ https://reviews.llvm.org/D138655 __

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG8a8f77c1b849: [clang-tidy] Implement CppCoreGuideline F.54 (authored by ccotter, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

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

carlosgalvezp marked an inline comment as done. carlosgalvezp added a comment. All comments have been addressed, I intend to land the patch by February 9th unless more feedback is received. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp added a comment. Friendly ping. I realized @aaron.ballman you probably have very good insights into this use case, since (AFAICT) you wrote a guideline about anonymous namespaces in headers, which is very closely related to this: https://wiki.sei.cmu.edu/confluence/display/cplusplu

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:27 + // Consider only explicitly or implicitly inline functions. + if (!FuncDecl->isInlined()) +return; Check if FuncDecl is not a nullptr bef

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvmlibc/inline-function-decl-check.rst:8 +tagged with the ``LIBC_INLINE`` macro. See the `libc style guide +`_ for more information about this macro

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp:28 +CheckFactories.registerCheck( +"llvmlibc-inline-function-decl-check"); CheckFactories.registerCheck( Remove Comme

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:88 + + if (!Param) { +return; Nit: the style convention in LLVM is to not use braces with one-line `if/else`. ===

[PATCH] D142655: [WIP][clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 494885. carlosgalvezp added a comment. Herald added a subscriber: arphaman. - Apply fix to remaining checks. - Fix documentation and release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142655/new/

[PATCH] D142655: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 494886. carlosgalvezp retitled this revision from "[WIP][clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options" to "[clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options". carlosgalvezp edited

[PATCH] D142655: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 494888. carlosgalvezp added a comment. Use get functions from ClangTidyCheck instead of Context Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142655/new/ https://reviews.llvm.org/D142655 Files: clang-t

[PATCH] D142655: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 494889. carlosgalvezp edited the summary of this revision. carlosgalvezp added a comment. Update commit message to clarify the difference between the first attempt and this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D142655: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 494892. carlosgalvezp added a comment. Remove unneeded newlines and braces in single-line if/else statements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142655/new/ https://reviews.llvm.org/D142655 Fi

[PATCH] D142655: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 494893. carlosgalvezp edited the summary of this revision. carlosgalvezp added a comment. Update commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142655/new/ https://reviews.llvm.org/D142655

[PATCH] D142655: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 494894. carlosgalvezp added a comment. Fix release notes, ImplementationFileExtensions only applies to one check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142655/new/ https://reviews.llvm.org/D142655

[PATCH] D142655: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:31 +: ClangTidyCheck(Name, Context) { + std::optional HeaderFileExtensionsOption = + Options.get("HeaderFileExtensions"); Eugene.Z

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp updated this revision to Diff 494929. carlosgalvezp added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/ https://reviews.llvm.org/D140290 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt clang-

[PATCH] D143342: [clang-tidy] Support std::format and std::print in readability-redundant-string-cstr

carlosgalvezp added a comment. Please document change in Release Notes, as well as in the check documentation, together with its limitations (can only handle 1 argument at a time). Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:182 + + Find

[PATCH] D143342: [clang-tidy] Support std::format and std::print in readability-redundant-string-cstr

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the fix! Might be good to wait a few days for other reviewers to have a look or give suggestions on how to improve the issue of having to do one parameter at a t

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

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:45 + } else { +Note << FixItHint::CreateInsertion(MatchedExpr->getBeginLoc(), "(int)"); + } pfultz2 wrote: > carlosgalvezp wrote: > > I don't think we

[PATCH] D141583: [clang-tidy][doc] Deprecate the AnalyzeTemporaryDtors option

carlosgalvezp created this revision. Herald added a subscriber: 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's not used anywhere, and

[PATCH] D141583: [clang-tidy][doc] Deprecate the AnalyzeTemporaryDtors option

carlosgalvezp added a comment. If we could get this in before 24th of January (release cut) it would be great, then we don't need to remove it in clang-tidy 19 :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141583/new/ https://reviews.llvm.org/D

[PATCH] D141583: [clang-tidy][doc] Deprecate the AnalyzeTemporaryDtors option

carlosgalvezp updated this revision to Diff 488657. carlosgalvezp added a comment. Update clang-tidy formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141583/new/ https://reviews.llvm.org/D141583 Files: clang-tools-extra/clang-tidy/Clang

[PATCH] D141583: [clang-tidy][doc] Deprecate the AnalyzeTemporaryDtors option

carlosgalvezp marked an inline comment as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:106 +- Deprecate the global configuration file option `AnalyzeTemporaryDtors`, + which is no longer in use. The option will be fully removed i

[PATCH] D141583: [clang-tidy][doc] Deprecate the AnalyzeTemporaryDtors option

carlosgalvezp updated this revision to Diff 488663. carlosgalvezp marked an inline comment as done. carlosgalvezp added a comment. Fix name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141583/new/ https://reviews.llvm.org/D141583 Files: clang-t

[PATCH] D141583: [clang-tidy][doc] Deprecate the AnalyzeTemporaryDtors option

carlosgalvezp added a comment. Thanks for reviewing! I can also add that the actual code using the option was removed around 5 years ago, so it's about time :) @njames93 Do you have any concerns with this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D141583: [clang-tidy][doc] Deprecate the AnalyzeTemporaryDtors option

This revision was automatically updated to reflect the committed changes. Closed by commit rG0f5eb3190cfc: [clang-tidy][doc] Deprecate the AnalyzeTemporaryDtors option (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141583/

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

carlosgalvezp updated this revision to Diff 42. carlosgalvezp added a comment. Add ExtraArgs and ExtraArgsBefore Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.org/D141144 Files: clang-tools-extra/clang-tidy/t

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

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:17 + +static const llvm::StringSet<> ValueTraits = { +"alignment_of", I'm guessing this list was obtained in some automatic way - could we document th

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

carlosgalvezp added a comment. In D141133#4051068 , @ccotter wrote: > My latest update to the diff was the result of a rebase, then addressing the > final comments. What I've been doing is something like > > git pull upstream main --rebase > # make m

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

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:172 +void TypeTraitsCheck::registerMatchers(MatchFinder *Finder) { + static const ast_matchers::internal::VariadicDynCastAllOfMatcher< + Stmt, `stati

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 489213. carlosgalvezp added a comment. Rebase and fix formatting of "clang-tidy" in the docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.llvm.org/D141000 Files: clang-too

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp added a comment. @Eugene.Zelenko @njames93 I believe I have addressed all the outstanding comments, is there anything else you think needs fix? Otherwise I would like to land the patch before January 24th when they will start a new release. Repository: rG LLVM Github Monorepo

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 489263. carlosgalvezp added a comment. Merge release notes into one line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.llvm.org/D141000 Files: clang-tools-extra/clang-tidy/C

[PATCH] D141769: [clang-tidy][NFC] Use C++17 nested namespaces in add_new_check.py and rename_check.py

carlosgalvezp created this revision. Herald added a subscriber: 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. Repository: rG LLVM Githu

[PATCH] D141769: [clang-tidy][NFC] Use C++17 nested namespaces in add_new_check.py and rename_check.py

carlosgalvezp updated this revision to Diff 489271. carlosgalvezp added a comment. Update release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141769/new/ https://reviews.llvm.org/D141769 Files: clang-tools-extra/clang-tidy/add_new_check.

[PATCH] D141769: [clang-tidy][NFC] Use C++17 nested namespaces in add_new_check.py and rename_check.py

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/rename_check.py:311-314 + # TODO: remove below replacement when all clang-tidy checks have been + # updated with C++17 nested namespaces. replaceInFileRegex(filename, 'namespace ' + old

[PATCH] D141769: [clang-tidy][NFC] Use C++17 nested namespaces in add_new_check.py and rename_check.py

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/rename_check.py:311-314 + # TODO: remove below replacement when all clang-tidy checks have been + # updated with C++17 nested namespaces. replaceInFileRegex(filename, 'namespace ' + old

[PATCH] D141769: [clang-tidy][NFC] Use C++17 nested namespaces in add_new_check.py and rename_check.py

carlosgalvezp abandoned this revision. carlosgalvezp added a comment. Merged into https://reviews.llvm.org/D141770 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141769/new/ https://reviews.llvm.org/D141769 _

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:151-152 Options.WarningsAsErrors = ""; + Options.HeaderFileExtensions = {"h", "hh", "hpp", "hxx"}; + Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"}; Option

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 489289. carlosgalvezp added a comment. Add missing empty header extension to the default list. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.llvm.org/D141000 Files: clang-to

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp updated this revision to Diff 489291. carlosgalvezp added a comment. Remove buggy empty extension for ImplementationFilesExtension Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.llvm.org/D141000 Files: cl

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp added a comment. @njames93 Let me know if you are happy with the patch, I'd like to land before 24th of January :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.llvm.org/D141000 __

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

carlosgalvezp added a comment. In D141000#4071793 , @Eugene.Zelenko wrote: > D142123 also cries for it :-) Somehow I > was not able to make comments there :-( Yep, one more reason to get this in asap :) Repository:

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

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

[PATCH] D140434: readability-const-return-type: don't diagnose a template function returning T, even if sometimes instantiated with e.g. T = const int.

carlosgalvezp added a comment. In D140434#4053053 , @suertreus wrote: > Thanks for reviewing. > > I don't have commit access; can someone who does please do the thing? @suertreus I can help you land the patch, what user name and email should I use for a

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

carlosgalvezp requested changes to this revision. carlosgalvezp added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:19 + +namespace clang { +namespace tidy

[PATCH] D142123: [clang-tidy] Add check to suggest use of #pragma once

carlosgalvezp added a comment. Please note, `HeaderFileExtensions` is becoming a global option in this patch to avoid duplication, which will land in the next 2 days. https://reviews.llvm.org/D141000 Therefore you'll need to update the patch based on that. CHANGES SINCE LAST ACTION https://r

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

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h:41-43 +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang Forgot to comment but you'll need it he

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

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:19 + +namespace clang { +namespace tidy { ccotter wrote: > carlosgalvezp wrote: > > We recently switched to using C++17

[PATCH] D142307: [clang-tidy][NFC] Use C++17 nested namespaces in clang-tidy headers

carlosgalvezp added a comment. In D142307#4071952 , @Eugene.Zelenko wrote: > Looks OK for me, but please fix small formatting issues. Will be good idea to > await for other eyes. Thanks for the quick review! I applied clang-format to the patch; any ad

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

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the contribution! Let's give a couple days for the others to have a last look Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D140434: readability-const-return-type: don't diagnose a template function returning T, even if sometimes instantiated with e.g. T = const int.

carlosgalvezp added a comment. In D140434#4073212 , @ymandel wrote: > In D140434#4071823 , @carlosgalvezp > wrote: > >> In D140434#4053053 , @suertreus >> wrote: >> >>>

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

This revision was automatically updated to reflect the committed changes. Closed by commit rG4240c9146248: [clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions… (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D141463: [clang-tidy] Improve rename_check.py

This revision was automatically updated to reflect the committed changes. Closed by commit rG7718422d3b78: [clang-tidy] Improve rename_check.py (authored by ccotter, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141463/ne

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

carlosgalvezp added a comment. @Sockke do you need help landing the patch? If so, let me know user and email for attribution :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138655/new/ https://reviews.llvm.org/D138655 ___ cfe-commits mailin

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp added a comment. Kind ping to reviewers :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126880/new/ https://reviews.llvm.org/D126880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp updated this revision to Diff 439973. carlosgalvezp added a comment. Rebase and fix directory structure for doc and test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126880/new/ https://reviews.llvm.org/D126880 Files: clang-tool

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp updated this revision to Diff 439974. carlosgalvezp added a comment. Fix test name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126880/new/ https://reviews.llvm.org/D126880 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/Av

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp added a comment. @LegalizeAdulthood I've addressed your comments, thanks for the clear instructions! One thing I didn't manage to do is build the target `docs-clang-tools-html`, it says it doesn't exist. I've enabled `LLVM_BUILD_DOCS=ON` in the CMake call - do you know if I need t

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp created this revision. Herald added subscribers: shchenz, kbarton, xazax.hun, mgorny, nemanjai. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Flags uses of const-qualifi

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp updated this revision to Diff 433743. carlosgalvezp added a comment. Fix FIXME Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126880/new/ https://reviews.llvm.org/D126880 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidC

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp updated this revision to Diff 433757. carlosgalvezp added a comment. Move logic from check to registerMatchers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126880/new/ https://reviews.llvm.org/D126880 Files: clang-tools-extra/cla

[PATCH] D126891: [clang-tidy] The check should ignore final classes

carlosgalvezp added a comment. Hmm, `MostDerived` **does** have a public virtual destructor in your example already - if the base class destructor is virtual, the child class destructor is virtual. In that sense the check should not warn. Seems like there's some deeper problem in the check? R

[PATCH] D126891: [clang-tidy] The check should ignore final classes

carlosgalvezp added a comment. In D126891#3555456 , @steakhal wrote: > In D126891#3554039 , @carlosgalvezp > wrote: > >> Hmm, `MostDerived` **does** have a public virtual destructor in your example >> already -

[PATCH] D126891: [clang-tidy] The check should ignore final classes

carlosgalvezp added a comment. Oh, I see the unit test now, indeed `Base` does not have a virtual destructor. LGTM then! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126891/new/ https://reviews.llvm.org/D126891 __

[PATCH] D126891: [clang-tidy] The check should ignore final classes

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:170 + ` involving + `final` classes. The check will not diagnose `final` marked classes, since + those cannot be used as base classes, consequently they can not violate the ---

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp updated this revision to Diff 433989. carlosgalvezp marked 4 inline comments as done. carlosgalvezp added a comment. Fix review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126880/new/ https://reviews.llvm.org/D126880 Files

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp added a comment. Fixed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126880/new/ https://reviews.llvm.org/D126880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp updated this revision to Diff 435474. carlosgalvezp marked 2 inline comments as done. carlosgalvezp added a comment. Address review comments. Rebase on latest main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126880/new/ https://rev

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h:29 + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + Eugene.Zelenko wrote: > Please add `isLanguag

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

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

[PATCH] D127446: [clang-tidy] Add `-verify-config` command line argument

carlosgalvezp added a comment. This sounds like great functionality, surely saving a lot of headaches! Any reason why we wouldn't want this active by default? I'd personally even go one step further and make it hard errors - warnings are easy to miss and ignore - but I can see how it can be pro

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

carlosgalvezp added a comment. @shafik @aaron.ballman @dblaikie Hello! Just wanted to check if there's any blockers for merging this patch? We are now on Clang 18, i.e. 2 releases after the warning was introduced, so IMO I believe it's a good time to turn it into a hard error and test it in the

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

carlosgalvezp added a comment. > I still think even if we can subset this, for whatever we're going to turn > into a hard error, it should be a warning-as-error in system headers first > for at least a release. (so perhaps the transition should look like: null (no > diagnostic) -> warning -> wa

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

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] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

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

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp updated this revision to Diff 541624. carlosgalvezp added a comment. Fix typo in release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155625/new/ https://reviews.llvm.org/D155625 Files: clang-tools-extra/clang-tidy/cppcore

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp updated this revision to Diff 541626. carlosgalvezp added a comment. Remove extra newline Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155625/new/ https://reviews.llvm.org/D155625 Files: clang-tools-extra/clang-tidy/cppcoreguidel

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp updated this revision to Diff 541684. carlosgalvezp added a comment. Split getInfo function into 2 functions, remove structured bindings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155625/new/ https://reviews.llvm.org/D155625 File

[PATCH] D153423: [clang-tidy] Allow explicit throwing in bugprone-exception-escape for special functions

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/D153423/new/ https://reviews.llvm.org/D153423

[PATCH] D147955: [clang-tidy] Extend CheckOptions to support grouping checks options

carlosgalvezp requested changes to this revision. carlosgalvezp added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:94-95 +// Nested option names need to be converted to null terminated strings fo

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp added a comment. In D155625#4512123 , @PiotrZSL wrote: > LGTM, but I'm not sure if isCopyableOrMovable will always work correctly, for > a user defined special members it looks ok, but for some implicit ones I > worry it may not always wor

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp updated this revision to Diff 541961. carlosgalvezp marked 3 inline comments as done. carlosgalvezp added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155625/new/ https://reviews.llvm.org/D155625 F

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp added a comment. Using `hasSimpleCopyConstructor` and so on greatly simplifies the logic, great! Let me know if you are happy with it or I should go ahead and merge. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp:9

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp updated this revision to Diff 541963. carlosgalvezp added a comment. Merge matchers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155625/new/ https://reviews.llvm.org/D155625 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp:91-102 + Finder->addMatcher( + fieldDecl(unless(isMemberOfLambda()), +hasDeclContext(cxxRecordDecl(isCopyableOrMovable())), +

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb70e6e968192: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid… (authored by carlosgalvezp). Repository: rG LLVM Git

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp requested changes to this revision. carlosgalvezp added a comment. This revision now requires changes to proceed. This should be a configuration option, we should not hardcore project-specific things in the source code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp added a comment. In D155890#4523262 , @gribozavr2 wrote: > In D155890#4523243 , @adukeman > wrote: > >> In D155890#4522266 , @ymandel >> wrote: >> >>> In

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp resigned from this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. > I think it'd be good to add reviewers there I realize the CodeOwners for Analysis are already in the list of reviewers, I won't interfere then :) Repository: rG LLVM G

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:62 +return N != nullptr && (isTopLevelNamespaceWithName(*N, "base") || +isTopLevelNamespaceWithName(*N, "folly")); } -

[PATCH] D156031: [clang-tidy] Ignore implcit casts in cppcoreguidelines-owning-memory

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/D156031/new/ https://reviews.llvm.org/D156031

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp added a comment. I have opened a refactoring ticket here: https://github.com/llvm/llvm-project/issues/64037 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890 _

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:83 Diags.setSourceManager(&Sources); + ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts()); A bit unclear to me why we should add this l

<    1   2   3   4   5   6   7   8   9   >