mboehme added a comment.
I've added some individual comments above, but I'd question whether it's really
necessary to make this change? Even if no move actually happens, the code is
still wrong (it shouldn't be using std::move in the first place), and it's
likely the author actually intended for a move to happen, so we should warn
that that would cause a use-after-move.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:408
+ unless(anyOf(inDecltypeOrTemplateArg(),
+ hasType(qualType(isConstQualified())))))
.bind("call-move");
----------------
Is this in the right place? It looks as if it's applying to the std::move, not
its argument?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:137
+ // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved
+ // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here
+}
----------------
Generally, I would avoid CHECK-NOTES-NOT if possible. Just leave off the
comments entirely; the test will fail if there were any unexpected diagnostics,
and the test is more resilient this way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74692/new/
https://reviews.llvm.org/D74692
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
