[PATCH] D141892: Implement modernize-use-constraints

2023-03-29 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 509544. ccotter added a comment. - format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt clang-tools

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

2023-03-30 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 509874. ccotter marked 3 inline comments as done. ccotter added a comment. - format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140760/new/ https://reviews.llvm.org/D140760 Files: clang-tools-extra/clang-t

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

2023-03-30 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:158-159 + callee(cxxMethodDecl(BeginNameMatcher))), + callExpr(argumentCountIs(1), + call

[PATCH] D144347: [clang-tidy] Add readability-forward-usage check

2023-03-31 Thread Chris Cotter via Phabricator via cfe-commits
ccotter accepted this revision. ccotter added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp:98 + callExpr( + unless(isExpansionInSystemHeader()), argumentCountIs(1

[PATCH] D144347: [clang-tidy] Add readability-forward-usage check

2023-03-31 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp:99 + unless(isExpansionInSystemHeader()), argumentCountIs(1U), + IgnoreDependentExpresions + ? expr(unless(isInstantiationDependent

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

2023-04-01 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 510262. ccotter added a comment. - Fix double curly brace matching in tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145138/new/ https://reviews.llvm.org/D145138 Files: clang-tools-extra/clang-tidy/mode

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

2023-04-01 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 510267. ccotter added a comment. - Add tests for char[] Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145138/new/ https://reviews.llvm.org/D145138 Files: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCh

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

2023-04-01 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked an inline comment as done. ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-arrays.cpp:170-172 + char init4[] = "abcdef"; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare C-style arrays, use std

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

2023-04-01 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 510270. ccotter added a comment. - format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145138/new/ https://reviews.llvm.org/D145138 Files: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp clan

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

2023-04-01 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked 5 inline comments as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp:496 + const Expr *SpelledExpr = E->IgnoreUnlessSpelledInSource(); + if (dyn_cast(SpelledExpr)) +

[PATCH] D147551: [clang-tidy] Fix init-list handling in readability-implicit-bool-conversion

2023-04-04 Thread Chris Cotter via Phabricator via cfe-commits
ccotter accepted this revision. ccotter 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/D147551/new/ https://reviews.llvm.org/D147551 _

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-04-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 511847. ccotter added a comment. - Rename Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt cla

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-04-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138 + +} // namespace negative_cases ccotter wrote: > PiotrZSL wrote: > > what about when someone uses std::move

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

2023-04-09 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. bump - any other feedback? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140760/new/ https://reviews.llvm.org/D140760 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D141892: Implement modernize-use-constraints

2023-04-10 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 512308. ccotter marked 12 inline comments as done. ccotter added a comment. feedback+rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 Files: clang-tools-extra

[PATCH] D141892: Implement modernize-use-constraints

2023-04-10 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp:157-160 +return std::make_tuple( +matchEnableIfSpecialization( +LastTemplateParam->getTypeSourceInfo()->getTypeLoc()), +LastTemplateParam); -

[PATCH] D141892: Implement modernize-use-constraints

2023-04-10 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. > Would you consider supporting enable_if via parameters I was planning to support those too, but in a subsequent commit / review since this review is rather large. Is that OK? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D141892: Implement modernize-use-constraints

2023-04-10 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 512313. ccotter added a comment. refactor Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 Files: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp cl

[PATCH] D141892: Implement modernize-use-constraints

2023-04-10 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 512314. ccotter added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 Files: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp clan

[PATCH] D141892: Implement modernize-use-constraints

2023-04-10 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 512315. ccotter added a comment. arc diff properly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt cl

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-04-10 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 512318. ccotter added a comment. - format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt cla

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-04-11 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked 2 inline comments as done. ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138 + +} // namespace negative_cases PiotrZSL wrote: > ccotter wrote: >

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-04-11 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 512455. ccotter marked an inline comment as done. ccotter added a comment. Fix tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

[PATCH] D141892: Implement modernize-use-constraints

2023-05-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. bump - @njames93 let me know if you have any further feedback. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 ___ cfe-commits

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

2023-02-12 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst:40 + + struct C { // This is not checked, because the destuctor might be defaulted in another translation unit. + ~C();

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

2023-02-12 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Looks good to me - looking forward to this check! Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:170 + +static constexpr char Bind[] = ""; + NIT: should the bound node have some meaningful non-empty name? =

[PATCH] D143877: [NFC] [clang] Forward forwarding reference

2023-02-12 Thread Chris Cotter via Phabricator via cfe-commits
ccotter created this revision. Herald added a project: All. ccotter requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Update function bodies to forward forwarding references. I spotted this while authoring a clang-tidy tool for CppCoreGuideli

[PATCH] D143877: [NFC] [clang] Forward forwarding reference

2023-02-13 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Realistically, I'm not sure if this change is all that valuable aside from abiding by the CppCoreGuidelines from the readability and consistency standpoint. The only impact this change would have that I can think of is allowing an object with an `&&` (or other combinati

[PATCH] D143877: [NFC] [clang] Forward forwarding reference

2023-02-13 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 497146. ccotter added a comment. - Add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143877/new/ https://reviews.llvm.org/D143877 Files: clang/include/clang/AST/IgnoreExpr.h clang/unittests/AST/ASTExp

[PATCH] D143971: Flag code with both string constructor arguments implicitly casted

2023-02-13 Thread Chris Cotter via Phabricator via cfe-commits
ccotter created this revision. Herald added a subscriber: carlosgalvezp. Herald added a reviewer: njames93. Herald added a project: All. ccotter requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Calls to the fill constructor where

[PATCH] D143971: [clang-tidy] Flag code with both string constructor arguments implicitly casted

2023-02-13 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/string-constructor.cpp:58 + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: string constructor arguments might be incorrect; + std::string s7(kText[1], 10); + // CHECK-MESSAGES: [[@LIN

[PATCH] D143971: [clang-tidy] Flag code with both string constructor arguments implicitly casted

2023-02-13 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 497189. ccotter added a comment. - Add more cases to swapped params Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143971/new/ https://reviews.llvm.org/D143971 Files: clang-tools-extra/clang-tidy/bugprone/Str

[PATCH] D143971: [clang-tidy] Flag code with both string constructor arguments implicitly casted

2023-02-13 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/string-constructor.cpp:40 + + std::string swapped('x', 4); + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: string constructor arguments are probably swapped; expecting string(count, c

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

2023-02-16 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. poke - anyone mind reviewing this? I used this tool to fix two small cases of missing move in the llvm project: https://reviews.llvm.org/D142824 https://reviews.llvm.org/D142825 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

2023-02-17 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Some of the examples you mentioned are indeed not compliant with guideline F.18. I have a test for at least one of the examples you gave (`moves_deref_optional` matches your first example). The guideline is fairly strict IMO, and I asked about this in https://github.co

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

2023-02-17 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. @PiotrZSL The last `forward` example you have should match my `does_move_auto_rvalue_ref_param` test, which is not flagged by the tool. Let me know if you have any other forward cases (preferred as full self contained examples) that the tool incorrectly flags, as gettin

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

2023-02-17 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 498525. ccotter added a comment. - Add StrictMode option, fix const&& bug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools-extra/clang-tidy/cppcor

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

2023-02-17 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 498526. ccotter added a comment. - Add back -fno-delayed-template-parsing to test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools-extra/clang-tid

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

2023-02-17 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked an inline comment as done. ccotter added a comment. Rebased as well on top of the latest release notes Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:45 +template +void never_moves_param_template(Ob

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

2023-02-17 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 498528. ccotter marked an inline comment as done. ccotter added a comment. rebase again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools-extra/cla

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

2023-02-17 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. @PiotrZSL I added an option to allow disabling the strict behavior. It should address many of your examples (e.g., moving subobjects) . Let me know if you have more feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

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

2023-02-19 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 498730. ccotter added a comment. - Add back -fno-delayed-template-parsing to test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools-extra/clang-tid

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

2023-02-20 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 498873. ccotter added a comment. - Remove duplicated matchers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/

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

2023-02-20 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. In template struct SomeClass { public: explicit SomeClass(T&& value) : value(std::forward(value)) {} T value; }; `T` is not a universal reference in the constructor, it's an rvalue reference to `T`. There is no deducing context, so universal references

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

2023-02-20 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. I was split on handling the case where the parameter that was never moved from was not named. For this guideline enforcement to flag all unmoved rvalue reference parameters, code that `std::moves` into an argument to a virtual method call could be especially confusing o

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

2023-02-20 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:301 + } + void never_moves(T&& t) {} + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: rvalue reference parameter 't' is never moved from

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

2023-02-20 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 499034. ccotter added a comment. - Ignore params of template type - Add option for unnamed params Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools

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

2023-02-20 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. > If you add std::move you will get compilation error, if you add std::forward, everything will work fine. Simply Value& and && will evaluate to Value&, no rvalue reference here. I agree that examining the template definition alone is not correct. In your original ex

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

2023-02-20 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked 4 inline comments as done. ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:45 +template +void never_moves_param_template(Obj&& o, T t) { + // CHECK-MESSAGES: :[[@LINE-1]

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

2023-02-20 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:45 +template +void never_moves_param_template(Obj&& o, T t) { + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: rvalue reference parameter

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 499341. ccotter marked 4 inline comments as done. ccotter added a comment. - Simplify matcher Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools-ext

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Simplified matchers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h:32-33 +private: + const unsigned StrictMode : 1; + const unsigned IgnoreUnnamedParams : 1; +}; PiotrZSL wrote: > use bool here, or it

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 499350. ccotter added a comment. - Use bool - Combine into a single matcher Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools-extra/clang-tidy/cppc

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked 2 inline comments as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:103-125 + StatementMatcher MoveCallMatcher = callExpr( + anyOf(callee(functionDecl(hasName("::std::move

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 499363. ccotter marked an inline comment as done. ccotter added a comment. - Finish combining into a single matcher - Rename option and add docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https:/

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:301 + } + void never_moves(T&& t) {} + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: rvalue reference parameter 't' is never moved from

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 499366. ccotter added a comment. - Include non-deduced template types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clang-tools-extra/clang-tidy/cppcoregui

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

2023-02-21 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Ok, all feedback should be addressed (thanks for the comment to consolidate into a single matcher) with appropriate options to relax the enforcement of the guideline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/ne

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

2023-02-22 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 499681. ccotter marked 4 inline comments as done. ccotter added a comment. - More matcher simplification Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141569/new/ https://reviews.llvm.org/D141569 Files: clan

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

2023-02-22 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked 4 inline comments as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:64-74 + hasAncestor(compoundStmt(hasParent(lambdaExpr( + has(cxxRecordD

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

2023-02-22 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked an inline comment as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:110 + const auto *MoveCall = Result.Nodes.getNodeAs("move-call"); + if (!MoveCall) { +diag(Param->getLo

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

2023-02-23 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Trivial types should not be passed by rvalue reference, but by value per the diagram under http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#fcall-parameter-passing. I feel like adding an option to opt-out of this is missing the point of this check, and rvalu

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

2023-02-23 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. > Imaginate that such trivial type could be for example 200KB in size This should be passed by const ref then correct (listed under "Expensive to move (e.g. big BigPOD[]" in the parameter passing guidelines

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-05-05 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520026. ccotter marked 6 inline comments as done. ccotter added a comment. - feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 Files: clang-tools-extra/clang

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-05-05 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:70 + Finder->addMatcher( + parmVarDecl( + parmVarDecl().bind("param"), isTemplateTypeOfFunction(), PiotrZSL wrote: > maybe think l

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-05-05 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520027. ccotter added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt clang

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-05-05 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Note, @PiotrZSL I don't have commit access, so if you're happy with the updates, could you please land this for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 __

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-05-05 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520032. ccotter added a comment. - fix merge Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

[PATCH] D141892: Implement modernize-use-constraints

2023-05-06 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520104. ccotter marked 3 inline comments as done. ccotter added a comment. Fix bug, other cleanups, rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 Files: cl

[PATCH] D141892: Implement modernize-use-constraints

2023-05-06 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked an inline comment as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp:247-248 +ParamsRange.getEnd(), SM, LangOpts, tok::r_paren, tok::r_paren); +return utils::lexer::findNextAnyTokenKind(En

[PATCH] D141892: Implement modernize-use-constraints

2023-05-06 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520125. ccotter added a comment. - Better handling for ctor inits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 Files: clang-tools-extra/clang-tidy/modernize/CMak

[PATCH] D141892: Implement modernize-use-constraints

2023-05-06 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Addressed all comments except for the `handleReturnType` one which I responded to - let me know your thoughts, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141892/new/ https://reviews.llvm.org/D141892 ___

[PATCH] D143971: [clang-tidy] Flag more buggy string constructor cases

2023-05-06 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520143. ccotter added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143971/new/ https://reviews.llvm.org/D143971 Files: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp cl

[PATCH] D143971: [clang-tidy] Flag more buggy string constructor cases

2023-05-06 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. bump please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143971/new/ https://reviews.llvm.org/D143971 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

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

2023-05-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked 10 inline comments as done. ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp:477 + + for (S::const_iterator It = cbegin(Ss), E = cend(Ss); It != E; ++It) { +printf("s4 has value %d\n", (*It

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

2023-05-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520232. ccotter added a comment. Feedback and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140760/new/ https://reviews.llvm.org/D140760 Files: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.

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

2023-05-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter marked an inline comment as done. ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp:481 + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (auto It :

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

2023-05-07 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 520237. ccotter marked an inline comment as done. ccotter added a comment. - Fix compile for non-libc++ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140760/new/ https://reviews.llvm.org/D140760 Files: clang

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

2023-05-01 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. bump? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143971: [clang-tidy] Flag more buggy string constructor cases

2023-06-10 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. bump please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143971/new/ https://reviews.llvm.org/D143971 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D143971: [clang-tidy] Flag more buggy string constructor cases

2023-06-11 Thread Chris Cotter via Phabricator via cfe-commits
ccotter updated this revision to Diff 530349. ccotter added a comment. - Exclude false positive Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143971/new/ https://reviews.llvm.org/D143971 Files: clang-tools-extra/clang-tidy/bugprone/StringConstru

[PATCH] D143971: [clang-tidy] Flag more buggy string constructor cases

2023-06-11 Thread Chris Cotter via Phabricator via cfe-commits
ccotter added a comment. Thanks, fixed the first false positive example you gave. Let me think about the second example in your most recent post: char c = '\n'; using Size = int; Size size = 10U; std::string str2(c, size); This is my newly added case `swapped4`, where one of my original

<    1   2   3