MarcoFalke added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:400
void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
auto CallMoveMatcher =
+ callExpr(
----------------
PiotrZSL wrote:
> This is correct but consider this:
>
> ```
> auto TryEmplaceMatcher =
> cxxMemberCallExpr(callee(cxxMethodDecl(hasName("try_emplace"))));
>
> auto CallMoveMatcher =
> callExpr(argumentCountIs(1), // because matching this will be faster than
> checking name.
> callee(functionDecl(hasName("::std::move"))),
> unless(inDecltypeOrTemplateArg()),
> expr().bind("call-move"),
> unless(hasParent(TryEmplaceMatcher)),
>
> anyOf(hasAncestor(compoundStmt(hasParent(lambdaExpr().bind("containing-lambda")))),
>
> hasAncestor(functionDecl(anyOf(cxxConstructorDecl(hasAnyConstructorInitializer(
>
> withInitializer(expr(
>
>
> anyOf(equalsBounNode("call-move"),
>
>
> hasDescendant(equalsBounNode("call-move")))
>
> ).bind("containing-ctor-init-stmt")
>
> ))
>
> ).bind("containing-ctor"),
>
> functionDecl().bind("containing-func")))))
> );
> ```
>
I get:
```
error: no matching function for call to object of type 'const
internal::ArgumentAdaptingMatcherFunc<clang::ast_matchers::internal::HasDescendantMatcher>'
anyOf(equalsBoundNode("call-move"),hasDescendant(equalsBoundNode("call-move")))
^~~~~~~~~~~~~
./llvm-project/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1491:3:
note: candidate template ignored: could not match 'Matcher' against
'PolymorphicMatcher'
operator()(const Matcher<T> &InnerMatcher) const {
^
./llvm-project/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1498:3:
note: candidate template ignored: could not match 'MapAnyOfHelper' against
'PolymorphicMatcher'
operator()(const MapAnyOfHelper<T...> &InnerMatcher) const {
^
1 error generated.
```
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1431
std::string val_;
};
----------------
PiotrZSL wrote:
> Missing tests:
> - Test with A derive from B, C, D, and argument is moved into C, but D
> initialization also uses it.
> - Test with lambda call inside init, and in lambda std::move of captured by
> reference constructor parameter, and lambda could be mutable, and lambda
> could be saved into member (or passed to base class), or called instantly
> (better both tests).
> - Test with lambda that takes argument into capture by move
>
> In short:
> ```
> struct Obj {};
> struct D
> {
> D(Obj b);
> };
>
> struct C
> {
> C(Obj b);
> };
>
> struct B
> {
> B(Obj& b);
> }
>
> struct A : B, C, D
> {
> A(Obj b)
> : B(b), C(std::move(b)), D(b)
> {
> }
> };
>
> and second:
>
> struct Base
> {
> Base(Obj b) : bb(std::move(b)) {}
> template<typename T>
> Base(T&& b) : bb(b()) {};
>
> Obj bb;
> };
>
> struct Derived : Base, C
> {
> Derived(Obj b)
> : Base([&] mutable { return std::move(b); }()), C(b)
> {
> }
> };
>
> struct Derived2 : Base, C
> {
> Derived2(Obj b)
> : Base([&] mutable { return std::move(b); }), C(b)
> {
> }
> };
>
> struct Derived3 : Base, C
> {
> Derived3(Obj b)
> : Base([c = std::move(b)] mutable { return std::move(c); }), C(b)
> {
> }
> };
> ```
I added the test `Derived2`, however, I wonder if it should throw a warning. In
the general case, I don't think we can know whether `Base(Call&&call)` will
actually call `call` and execute the `std::move`, no?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146288/new/
https://reviews.llvm.org/D146288
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
