I see your point about how users who care should always be passing this check alongside "performance-move-const-arg"; but IMVHO it still makes sense for clang-tidy to warn unconditionally about code of the form
x = std::move(y); use(y); regardless of the incidental type of `y`. Sure, it's *technically* not wrong if `y` is const... or if `y` is a primitive type such as `Widget*`... or if `y` is a well-defined library type such as `std::shared_ptr<Widget>`... or if `y` is a library type that works in practice, such as `std::list<int>` <https://stackoverflow.com/questions/60175782/is-stdlistint-listreturn-stdmovelistlist-guaranteed-to-leave/60186200#comment106478458_60186200>... but regardless of its *technical* merits, the code is still *logically semantically* wrong, and I think that's what the clang-tidy check should be trying to diagnose. my $.02, Arthur On Sun, Feb 16, 2020 at 10:44 AM Zinovy Nis via Phabricator via cfe-commits <cfe-commits@lists.llvm.org> wrote: > zinovy.nis updated this revision to Diff 244878. > > CHANGES SINCE LAST ACTION > https://reviews.llvm.org/D74692/new/ > > https://reviews.llvm.org/D74692 > > Files: > clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp > clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp > > > Index: > clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp > =================================================================== > --- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp > +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp > @@ -129,6 +129,14 @@ > // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here > } > > +// Simple case for const value: no actual move and no warning. > +void simpleConst(const A a) { > + A other_a = std::move(a); > + a.foo(); > + // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved > + // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here > +} > + > // A warning should only be emitted for one use-after-move. > void onlyFlagOneUseAfterMove() { > A a; > @@ -308,14 +316,15 @@ > } > > void lambdas() { > - // Use-after-moves inside a lambda should be detected. > + // Use-after-moves inside a lambda will not be detected as [a] > + // is passed as a const by default. > { > A a; > auto lambda = [a] { > std::move(a); > a.foo(); > - // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved > - // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here > + // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'a' used after it was > moved > + // CHECK-NOTES-NOT: [[@LINE-3]]:7: note: move occurred here > }; > } > // This is just as true if the variable was declared inside the lambda. > @@ -720,15 +729,15 @@ > // const pointer argument. > const A a; > std::move(a); > - passByConstPointer(&a); > - // CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved > - // CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here > + passByConstPointer(&a); // expect no warning: the const value was not > moved. > + // CHECK-NOTES-NOT: [[@LINE-1]]:25: warning: 'a' used after it was > moved > + // CHECK-NOTES-NOT: [[@LINE-3]]:5: note: move occurred here > } > const A a; > std::move(a); > - passByConstReference(a); > - // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved > - // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here > + passByConstReference(a); // expect no warning: the const value was not > moved. > + // CHECK-NOTES-NOT: [[@LINE-1]]:24: warning: 'a' used after it was moved > + // CHECK-NOTES-NOT: [[@LINE-3]]:3: note: move occurred here > } > > // Clearing a standard container using clear() is treated as a > Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp > =================================================================== > --- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp > +++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp > @@ -404,7 +404,8 @@ > hasArgument(0, declRefExpr().bind("arg")), > anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")), > hasAncestor(functionDecl().bind("containing-func"))), > - unless(inDecltypeOrTemplateArg())) > + unless(anyOf(inDecltypeOrTemplateArg(), > + hasType(qualType(isConstQualified()))))) > .bind("call-move"); > > Finder->addMatcher( > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits