[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

ldionne added a comment. I personally don't think the note is useful. What we're trying to tell the user is "don't use X after you've moved out of X" -- whether the move actually has an effect or not is not useful information. To me, adding `; move of a 'const' argument has no effect` just

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis added a comment. One more gentle ping) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis updated this revision to Diff 255128. zinovy.nis added a comment. Rebase over the current master. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 Files: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis added a comment. Gentle ping) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

aaron.ballman added reviewers: lebedev.ri, JonasToth, ldionne. aaron.ballman added a comment. Herald added a subscriber: dexonsmith. In D74692#1923315 , @zinovy.nis wrote: > In D74692#1923191 , @Quuxplusone >

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis added a comment. In D74692#1923191 , @Quuxplusone wrote: > I still think this entire patch is misguided; there's no reason to make the > note for `const std::string s; std::move(s)` any longer than the note for > `int i; std::move(i)` or

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

Quuxplusone added a comment. I still think this entire patch is misguided; there's no reason to make the note for `const std::string s; std::move(s)` any longer than the note for `int i; std::move(i)` or `volatile std::string v; std::move(v)`. People should not be using moved-from objects,

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis updated this revision to Diff 250375. zinovy.nis added a comment. Removed top-level consts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 Files: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from the `const` nits. You should wait for @Quuxplusone before committing in case there are any additional comments. Comment at:

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis added a comment. I'm done, Aaron, Quuxplusone, do you have any comments? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis marked 2 inline comments as done. zinovy.nis added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:342 + // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured const here }; }

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis added a comment. In D74692#1911150 , @Quuxplusone wrote: > Anyway, I still don't see the point of this patch. It seems like you're just > duplicating the work of `performance-move-const-arg`. People who want to be > shown notes about

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

Quuxplusone added a comment. Anyway, I still don't see the point of this patch. It seems like you're just duplicating the work of `performance-move-const-arg`. People who want to be shown notes about moves-of-const-args should just enable that check instead. CHANGES SINCE LAST ACTION

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:342 + // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured const here }; } zinovy.nis wrote: > aaron.ballman

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:342 + // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured const here }; } aaron.ballman wrote: > One more test

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis updated this revision to Diff 248905. zinovy.nis marked 4 inline comments as done. zinovy.nis added a comment. Cosmetic and style fixes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 Files:

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:383 SourceLocation MoveLoc = MovingCall->getExprLoc(); + const bool MoveArgIsConst = MoveArg->getType().isConstQualified(); Drop the top-level

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis added a comment. Any further comments? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis updated this revision to Diff 247985. zinovy.nis added a comment. Typo fixed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 Files: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis updated this revision to Diff 247982. zinovy.nis added a comment. 1. Special handling for captured variables in lambdas, 2. messages texts were changed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74692/new/ https://reviews.llvm.org/D74692 Files:

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:399-400 +Check->diag(MoveArgLoc, +"std::move of the const expression has no effect; " +"remove std::move() or make the variable

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

zinovy.nis marked an inline comment as done. zinovy.nis added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:329 // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here + // CHECK-NOTES: [[@LINE-6]]:7: note:

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:329 // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here + // CHECK-NOTES: [[@LINE-6]]:7: note: std::move of the const expression {{.*}} };