MarcoFalke marked 3 inline comments as done.
MarcoFalke added a comment.
Thanks, addressed some more review comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1431
std::string val_;
};
----------------
PiotrZSL wrote:
> MarcoFalke wrote:
> > 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?
> For me:
> Derived - should throw a warning - same type used before C(b) (but
> questionable)
> Derived 2 -> also should throw warning (but questionable)
> Derived 3 -> also should throw warning
>
> Note that Derived & Derived 2 could be considered different issue, like use
> after free (because we take reference to argument), it's safe to assume that
> it would be called anyway, because later that argument reference could become
> invalid. But it could be also fine to not throw issue. For is better to throw
> & nolint than ignore and crash.
> But thats your call.
Ok, maybe it can be done in a follow-up? This should be a separate and
pre-existing issue on `main` already (see also my other added test case about
lambdas).
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
