PiotrZSL added inline comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1431
std::string val_;
};
----------------
MarcoFalke wrote:
> 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).
Yes it could be.
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
