[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

This revision was automatically updated to reflect the committed changes. Closed by commit rG95fe54931fdd: [clang-tidy] new performance-no-automatic-move check. (authored by courbet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet marked an inline comment as done. courbet added a comment. Thanks for the review. Comment at: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp:32 + + const auto const_local_variable = + varDecl(hasLocalStorage(),

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet updated this revision to Diff 230595. courbet added a comment. Use LLVM style Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/ https://reviews.llvm.org/D70390 Files: clang-tools-extra/clang-tidy/performance/CMakeLists.txt

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM after the variable names are adjusted Comment at: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp:32 + + const auto const_local_variable = +

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet updated this revision to Diff 230428. courbet marked 2 inline comments as done. courbet added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/ https://reviews.llvm.org/D70390 Files:

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

JonasToth added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp:28 +void NoAutomaticMoveCheck::registerMatchers(MatchFinder *finder) { + const auto const_local_variable = + varDecl(hasLocalStorage(),

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet updated this revision to Diff 230202. courbet added a comment. Remove && warnings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/ https://reviews.llvm.org/D70390 Files: clang-tools-extra/clang-tidy/performance/CMakeLists.txt

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet added a comment. In D70390#1751751 , @Quuxplusone wrote: > that sounds like a big non-local change whose correctness would be very > difficult to verify. Agreed. That's why I did not make this check suggest a fixit, because it's too easy to

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet planned changes to this revision. courbet added a comment. I'm going to remove the `&&` warning from the check and keep only the `const` one, as I think these is a consensus on that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

Quuxplusone added a comment. Hi, I'm the main original author (although I would not claim "current maintainer") of the Clang warning. Clang doesn't warn about `const vector v; return v;` because in that case, adding `std::move` to the return would not help anything. I had not considered that

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

JonasToth added a comment. In D70390#1751159 , @courbet wrote: > > IMHO these two should just not overlap. It makes sense, to have > > controversial or configurable stuff in clang-tidy. It should just be > > consistent with the warnings, as those are

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet added a comment. > IMHO these two should just not overlap. It makes sense, to have controversial > or configurable stuff in clang-tidy. It should just be consistent with the > warnings, as those are "always right" and clang-tidy can be > opinionated/specialized. So to make sure I

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

JonasToth added a comment. IMHO these two should just not overlap. It makes sense, to have controversial or configurable stuff in clang-tidy. It should just be consistent with the warnings, as those are "always right" and clang-tidy can be opinionated/specialized. Repository: rG LLVM

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet added a comment. Adding @Quuxplusone (warning author) for opinions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/ https://reviews.llvm.org/D70390 ___ cfe-commits mailing list

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet reclaimed this revision. courbet added a comment. Actually, thinking more about this, adding this to the existing warning might not be a very consensual change. In the case of the warning: S f() { T&& t = ...; ... return t; } there is no argument against doing the

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet abandoned this revision. courbet marked an inline comment as done. courbet added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

lebedev.ri added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls StatusOr::StatusOr(std::vector&&) + } JonasToth wrote: > lebedev.ri

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

JonasToth added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls StatusOr::StatusOr(std::vector&&) + } lebedev.ri wrote: > courbet

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet updated this revision to Diff 229840. courbet added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/ https://reviews.llvm.org/D70390 Files: clang-tools-extra/clang-tidy/performance/CMakeLists.txt

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet marked 2 inline comments as done. courbet added a comment. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/ https://reviews.llvm.org/D70390 ___ cfe-commits mailing list

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

lebedev.ri added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls StatusOr::StatusOr(std::vector&&) + } courbet wrote: > JonasToth

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:143 + + The check flags constructs that prevent automatic move of local variables. + Please omit //The check// and synchronize with first sentence of documentation.

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet marked 3 inline comments as done. courbet added a comment. Thanks for the comments Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet updated this revision to Diff 229833. courbet marked an inline comment as done. courbet added a comment. Fix markdown in doc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70390/new/ https://reviews.llvm.org/D70390 Files:

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

JonasToth added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47 +std::vector&& obj = ...; +return std::move(obj); // calls StatusOr::StatusOr(std::vector&&) + } While checking this example it

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

courbet created this revision. courbet added a reviewer: aaron.ballman. Herald added subscribers: xazax.hun, mgorny. Herald added a project: clang. The check flags constructs that prevent automatic move of local variables. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D70390