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

2022-12-19 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst:39 + A semicolon-separated list of filename extensions of header files (the filename + extensions should not include "." prefix). Default is ";h;h

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

2022-12-19 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 484000. carlosgalvezp added a comment. Replace quotes with single backticks. 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-ti

[PATCH] D140328: [ASTMatchers] Add isInAnonymousNamespace narrowing matcher

2022-12-19 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp created this revision. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Used in a couple clang-tidy checks so it could be extracted out as its own matcher. Repository: rG LLVM Gith

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

2022-12-19 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision. carlosgalvezp added a comment. LGTM, perhaps give a couple days to @njames93 to give a final look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140018/new/ https://reviews.llvm.org/D140018 ___

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

2022-12-19 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:140 +const internal::VariadicDynCastAllOfMatcher +dependentNameTypeLoc; // NOLINT(readability-identifier-naming) + I don't see a reason why the naming

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

2022-12-20 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. 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 fails to include a header, there's many other things that can possibly go wrong - shoul

[PATCH] D140328: [ASTMatchers] Add isInAnonymousNamespace narrowing matcher

2022-12-23 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG125ccd375147: [ASTMatchers] Add isInAnonymousNamespace narrowing matcher (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140328/ne

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

2022-12-25 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 485235. carlosgalvezp added a comment. Warn only about cases where there really is a problem. There are other cases which are just a readability issue, where a separate check for "redundant static" would fit better. Remove also the automatic fix-it sinc

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

2022-12-25 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 485236. carlosgalvezp added a comment. Remove remaining function declaration in the check class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/ https://reviews.llvm.org/D140290 Files: clang-

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

2022-12-27 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 485396. carlosgalvezp added a comment. Narrow warnings further, and provide fix-its. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/ https://reviews.llvm.org/D140290 Files: clang-tools-extra/

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

2022-12-27 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. I'm happy with the check now, feel free to review! I have run it on the LLVM repo and it seems to do what it's supposed to (including fix-its). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/ https://review

[PATCH] D140772: [clang-tidy] Fix minor bug in add_new_check.py

2022-12-30 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, I have a suggestion for further improvement. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:324-328 doc_files = [] - for subdir in list(fil

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

2022-12-30 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. @tberghammer Do you need help to land 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 list cfe

[PATCH] D140772: [clang-tidy] Fix minor bug in add_new_check.py

2022-12-30 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:324-328 doc_files = [] - for subdir in list(filter(lambda s: not s.endswith('.rst') and not s.endswith('.py'), - os.listdir(docs_dir))): + for subdir in list(f

[PATCH] D140772: [clang-tidy] Fix minor bug in add_new_check.py

2022-12-30 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:324-328 doc_files = [] - for subdir in list(filter(lambda s: not s.endswith('.rst') and not s.endswith('.py'), - os.listdir(docs_dir))): + for subdir in list(f

[PATCH] D140772: [clang-tidy] Fix minor bug in add_new_check.py

2022-12-30 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision. carlosgalvezp added a comment. Thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140772/new/ https://reviews.llvm.org/D140772 ___ cfe-commits mailing

[PATCH] D140793: [clang-tidy] Implement CppCoreGuideline CP.53

2023-01-01 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp:31 +const MatchFinder::MatchResult &Result) { + const auto *Param = Result.Nodes.getNodeAs("param"); + Check for nullp

[PATCH] D140793: [clang-tidy] Implement CppCoreGuideline CP.53

2023-01-02 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the patch! Maybe wait a couple days before landing to give some extra time for other reviewers. You might also want to mark the comments as "Done", so reviewers

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

2023-01-02 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp:21-23 +AST_MATCHER(NamedDecl, isInAnonymousNamespace) { + return Node.isInAnonymousNamespace(); +} I recently added this matcher in

[PATCH] D140894: [clang-tidy] Don't emit misc-unused-using-decl warnings for header files.

2023-01-03 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/unused-using-decls.rst:26 + A semicolon-separated list of filename extensions of header files (the filename + extensions should not include "." prefix). Default is ";h;hh;hpp;hxx"

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

2023-01-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/StrlenStringCStrCheck.h:23 + : ClangTidyCheck(Name, Context), +EnableForDataMethod(Options.get("EnableForDataMethod", false)) {} + The docs say this is "true" b

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

2023-01-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/strlen-string-cstr.cpp:1 +// RUN: %check_clang_tidy -check-suffix=0 %s readability-strlen-string-cstr %t -- -config="{CheckOptions: [{key: readability-strlen-string-cstr.En

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

2023-01-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp created this revision. Herald added subscribers: arphaman, 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. We have a number o

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

2023-01-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D141000#4026600 , @Eugene.Zelenko wrote: > Looks OK for me, but probably will be good idea to remove check-specific > options in same commit. Thanks for the review! Sure, I can do that, I just thought we should try to

[PATCH] D140793: [clang-tidy] Implement CppCoreGuideline CP.53

2023-01-05 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb06b248ad9dc: [clang-tidy] Implement CppCoreGuideline CP.53 (authored by ccotter, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D140772: [clang-tidy] Fix minor bug in add_new_check.py

2023-01-05 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D140772#4028570 , @ccotter wrote: > @carlosgalvezp - could you also help commit this if it looks good? Sure thing, thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D140772: [clang-tidy] Fix minor bug in add_new_check.py

2023-01-05 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG210b731c01b0: [clang-tidy] Fix minor bug in add_new_check.py (authored by ccotter, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D140307: [clang-tidy] Match derived types in in modernize-loop-convert

2023-01-05 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:258 + hasMethod(cxxMethodDecl(hasName("begin"), isConst())), + hasMethod(cxxMethodDecl(hasName("end"), +

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

2023-01-05 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Thanks for the feedback! > 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 in

[PATCH] D140307: [clang-tidy] Match derived types in in modernize-loop-convert

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, let's give @njames93 a few days in case he has some comments. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:258 +

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. On a second thought, using `getLocalOrGlobal` would still require all checks to have 2 private member variables (the raw string option and the parsed header option), together with the logic in the constructor to get the option an parse that. I would like to get ri

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. I found now what this global `User` option is about: /// Specifies the name or e-mail of the user running clang-tidy. /// /// This option is used, for example, to place the correct user name in TODO() /// comments in the relevant check. std::optional User;

[PATCH] D141118: [clang-tidy][NFC] Remove unused User argument in misc-misleading-bidirectional check

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
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. R

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Friendly ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/ https://reviews.llvm.org/D140290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. > Therefore does it make sense to also do the same with the IncludeStyle as > lots of checks add new includes. Sure, that could also be done, though I think it'd be preferable to do it in a separate patch. Personally I'm a bit reluctant to having formatting option

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp created this revision. Herald added subscribers: arphaman, 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. - Specify that the

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486913. carlosgalvezp added a comment. Remove extra newline 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/tool/ClangTidy

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486915. carlosgalvezp added a comment. Remove wrong dash in key/value options. 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-

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486916. carlosgalvezp added a comment. Fix incorrect dash in key/value pairs 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-ti

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486921. carlosgalvezp added a comment. Remove colon after the config options, for consistency with the command-line options. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 486922. carlosgalvezp added a comment. Fix alignment 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/tool/ClangTidyMain.cp

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp marked 2 inline comments as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/index.rst:142 e.g. --config-file=/some/path/myTidyConfigFile -This option

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9 +auto explicit_this_capture = [=, this]() { }; +// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture t

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

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:1 +// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-this-with-capture-default %t + -

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

2023-01-07 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9 +auto explicit_this_capture = [=, this]() { }; +// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture t

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

2023-01-07 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp:35 +Capture.getLocation(), SourceMgr, Context.getLangOpts(), tok::amp); +llvm::errs() << "FOR REF capture loc= " +

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM! Please give a few days for other reviewers to comment. If you are not 100% happy with the check name I'd suggest to change it before merging, it will be much harder to chan

[PATCH] D141118: [clang-tidy][NFC] Remove unused User argument in misc-misleading-bidirectional check

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc0d0b1237a9f: [clang-tidy][NFC] Remove unused User argument in misc-misleading-bidirectional… (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 487191. carlosgalvezp added a comment. - Add also ImplementationFileExtensions. - Add deprecation notice to affected checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.llvm.

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 487192. carlosgalvezp retitled this revision from "[clang-tidy] Introduce HeaderFileExtensions option" to "[clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options". carlosgalvezp edited the summary of this revision. carlosga

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision. carlosgalvezp added a comment. LGTM! Minor nit. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst:4 +cppcoreguidelines-avoid-capture-default-when-capturing-this +==

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. I've heard Clang 16 will be branched out on 24th January, it would be good to get this merged by then (if people are happy with it) so the full removal can happen in clang-tidy 18 :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Since we are at it - are we happy with the name "ImplementationFileExtensions"? I think "SourceFileExtensions" is more common in the literature, and it would look more visually pleasing in the config file since it has the same length as "HeaderFileExtensions".

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 487202. carlosgalvezp added a comment. Add check to ensure HeaderFileExtensions and ImplementationFileExtensions do not overlap. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.l

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D141000#4034305 , @Eugene.Zelenko wrote: > It'll be good idea to check that `HeaderFileExtensions` and > `ImplementationFileExtensions` do not overlap. Good idea, done! Repository: rG LLVM Github Monorepo CHANGES

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D141000#4034307 , @carlosgalvezp wrote: > Since we are at it - are we happy with the name > "ImplementationFileExtensions"? I think "SourceFileExtensions" is more common > in the literature, and it would look more visu

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

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. On the other hand, the LLVM coding standards talk about "implementation file". Ok, will leave as is. Then I'd say it's ready for review! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.ll

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

2023-01-09 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 487363. carlosgalvezp marked an inline comment as done. carlosgalvezp added a comment. Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.org/D141144 Files: c

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

2023-01-09 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 487370. carlosgalvezp added a comment. Trim first character instead, to keep the code visually pleasing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.org/D141144 Files:

[PATCH] D140307: [clang-tidy] Match derived types in in modernize-loop-convert

2023-01-09 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Hi! I'm finding merge conflicts when rebasing the patch on latest trunk, would you mind uploading a rebased version? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140307/new/ https://reviews.llvm.org/D140307

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

2023-01-10 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. @njames93 Do you agree with the overall idea of documenting config file options like this? I need this in place for the other patch that cleans the duplication for `HeaderFileExtensions`, `ImplementationFileExtensions`, `IncludeStyle`, etc. Repository: rG LLVM

[PATCH] D140307: [clang-tidy] Match derived types in in modernize-loop-convert

2023-01-10 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe43295209bb8: [clang-tidy] Match derived types in in modernize-loop-convert (authored by ccotter, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

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

2023-01-10 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 487844. carlosgalvezp marked 3 inline comments as done. carlosgalvezp added a comment. Revert changes to CheckOptions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.org/D14

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

2023-01-10 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:34 +static StringRef TrimFirstChar(StringRef x) { return x.substr(1); } + njames93 wrote: > This seems a bit of a needless change, if you want to remove the ne

[PATCH] D141391: [NFC] [clang-tools-extra] Alphabetize clang-tidy release notes

2023-01-10 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D141391#4040044 , @Eugene.Zelenko wrote: > In D141391#4040002 , @ccotter wrote: > >> I can't land these myself, so if you are able to, would you mind doing so? >> Thanks! > > @c

[PATCH] D141391: [NFC] [clang-tools-extra] Alphabetize clang-tidy release notes

2023-01-10 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9fff1ddb560c: [NFC] [clang-tools-extra] Alphabetize clang-tidy release notes (authored by ccotter, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

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

2023-01-10 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:31 + const auto *MatchedExpr = Result.Nodes.getNodeAs("x"); + diag(MatchedExpr->getBeginLoc(), "enum is implictly converted to an integral") + << MatchedExpr->getSour

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

2023-01-10 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Thanks for the fix! Some minor comments/questions. Comment at: clang-tools-extra/clang-tidy/rename_check.py:75 print("Renaming '%s' -> '%s'..." % (fileName, newFileName)) - os.rename(fileName, newFileName) + subprocess.check_call(["git", "mv"

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

2023-01-11 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/rename_check.py:97 def getListOfFiles(clang_tidy_path): - files = glob.glob(os.path.join(clang_tidy_path, '*')) - for dirname in files: -if os.path.isdir(dirname): - files += glob.glob(os.pa

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

2023-01-11 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the fix! Let's give a few days for other reviewers to get a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

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

2022-11-03 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp created this revision. Herald added a subscriber: xazax.hun. 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 Github Monorepo https://reviews.llvm.or

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

2022-11-03 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 472946. carlosgalvezp added a comment. Fix release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files: clang-tools-extra/clang-tidy/modernize/CMakeL

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

2022-11-03 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 472948. carlosgalvezp added a comment. Add reference to undeprecation of static. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files: clang-tools-extra/clan

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

2022-11-03 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 472995. carlosgalvezp added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files: clang-tools-extra/clang-tidy/modernize/

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

2022-11-03 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D137340#3906250 , @njames93 wrote: > 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

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

2022-11-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 473156. carlosgalvezp added a comment. Move to misc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt

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

2022-11-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 473159. carlosgalvezp retitled this revision from "[clang-tidy] Add modernize-use-anonymous-namespace check" to "[clang-tidy] Add misc-use-anonymous-namespace check". carlosgalvezp added a comment. Update commit message Repository: rG LLVM Github M

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

2022-11-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 473211. carlosgalvezp added a comment. Remove some code duplication Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files: clang-tools-extra/clang-tidy/misc/C

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

2022-11-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 473214. carlosgalvezp added a comment. Fix typo in binding names Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files: clang-tools-extra/clang-tidy/misc/CMak

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

2022-11-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 473494. carlosgalvezp added a comment. Simplify code for printing the type of the declaration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files: clang-to

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

2022-11-10 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. @njames93 Friendly ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

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

2022-11-17 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. @njames93 Ping again. Alternatively, could someone else review? @aaron.ballman maybe? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 __

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

2023-01-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. LGTM, really useful check! Would be nice to get some more eyes on the implementation, it's fairly advanced for my basic AST knowledge. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137302/new/ https://reviews.llvm.or

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

2023-01-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 491728. carlosgalvezp added a comment. Add HeaderFileExtensions and ImplementationFileExtensions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.org/D141144 Files: clang-

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

2023-01-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 491729. carlosgalvezp added a comment. Fix formatting 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/tool/ClangTidyMain.c

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

2023-01-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 491731. carlosgalvezp edited the summary of this revision. carlosgalvezp added a comment. Remove newline Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.org/D141144 Files:

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

2023-01-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Friendly ping @njames93 , see my previous comment about `TrimFirstChar`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141144/new/ https://reviews.llvm.org/D141144 ___ cfe-

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

2023-01-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:13 + +#include + Not sure if this is allowed in the repo or if we should use something equivalent from ADT.

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

2023-01-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:1 +// RUN: %check_clang_tidy -std=c++14-or-later %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- -- -fno-delayed-templ

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

2023-01-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:1 +// RUN: %check_clang_tidy -std=c++14-or-later %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- -- -fno-delayed-templ

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

2023-01-26 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:20 +void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this); +} Cl

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

2023-01-26 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.cpp:18-19 + +// CHECK-MESSAGES: warning: 'addll' must be tagged with the LIBC_INLINE macro +// CHECK-MESSAGES: warning: 'addul' must be tagged with the LIBC_

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

2023-01-26 Thread Carlos Galvez via Phabricator via cfe-commits
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. Re-introduce it, now with a

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

2023-01-26 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 492526. carlosgalvezp retitled this revision from "[clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options" to "[WIP][clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options". carlosgalvezp added

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

2023-01-26 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 492555. carlosgalvezp added a comment. Add functions to ClangTidyCheck so checks can get access to them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142655/new/ https://reviews.llvm.org/D142655 Files:

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

2023-01-26 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:233-234 + utils::FileExtensionsSet HeaderFileExtensions; + utils::FileExtensionsSet ImplementationFileExtensions; + @njames93 Would appreciate feedback

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

2023-01-26 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:149 + Options.HeaderFileExtensions = {"", "h", "hh", "hpp", "hxx"}; + Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"}; Options.HeaderFilterRegex = "";

[PATCH] D142673: [clang-tidy] Refactor HeaderGuardCheck to add HeaderGuardStyle

2023-01-27 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Note: I do not have possibility to add code comments - is there some too strict permissions set for this patch? It has not happened to other patches. Review comment: I think the design choice in clang-tidy is that checks from different modules should not depend o

[PATCH] D142673: [clang-tidy] Refactor HeaderGuardCheck to add HeaderGuardStyle

2023-01-27 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Also, in general the design choice is that checks should be independent and authors should not need to care about whether they conflict with other checks. In this particular case, if both checks are providing different results, users should choose one and disable

[PATCH] D142565: [clang-tidy] Fix warning in portability-simd-intrinsics

2023-01-27 Thread Carlos Galvez via Phabricator via cfe-commits
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/D142565/new/ https://reviews.llvm.org/D142565

<    1   2   3   4   5   6   7   8   9   >