mboehme added a comment. Sorry for the delay in replying -- I was caught up in other projects.
================ Comment at: clang-tools-extra/docs/ReleaseNotes.rst:224 + <clang-tidy/checks/bugprone/use-after-move>`: + - Fixed handling for designated initializers. + - Fix: In C++17 and later, the callee of a function is guaranteed to be ---------------- Reworded according to comments in https://reviews.llvm.org/D145906. ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1296 +// In a function call, the expression that determines the callee is sequenced +// before the arguments. ---------------- PiotrZSL wrote: > This entire file tests only C++17 and later, when std::move is C++11 feature. > Add separate test file for C++11 (or even better split this one between C++11 > and C++17) > > Scenario to test in C++11, should warn ? > ``` > a->foo(std::move(a)); > ``` > This entire file tests only C++17 and later, when std::move is C++11 feature. > Add separate test file for C++11 (or even better split this one between C++11 > and C++17) Good point. I've added a new `RUN` line that also runs this in C++11 mode. > Scenario to test in C++11, should warn ? See below -- this does produce a warning in C++11 mode (but not C++17 mode). I've also redone the tests to reuse the existing class `A` without having to define a new one. ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1304 + std::unique_ptr<A> a; + a->foo(std::move(a)); +} ---------------- PiotrZSL wrote: > What about scenario like this: > > ``` > b.foo(a->saveBIntoAAndReturnBool(std::move(b))); > ``` > > Is first "b" still guaranteed to be alive after std::move ? I'm not exactly sure what you're asking here... or how this scenario is materially different from the other scenarios we already have? > Is first "b" still guaranteed to be alive after std::move ? The `b` in `b.foo` is guaranteed to be evaluated before the call `a->saveBIntoAAndReturnBool(std::move(b))` -- but I'm not sure if this is what you're asking? Or are you asking whether the `a->saveBIntoAAndReturnBool(std::move(b))` can cause the underlying object to be destroyed before the call to `b.foo` happenss? In other words, do we potentially have a use-after-free here? I think the answer to this depends on what exactly `saveBIntoAAndReturnBool()` does (what was your intent here?). I also think it's probably beyond the scope of this check in any case, as this check is about diagnosing use-after-move, not use-after-free. ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1305 + a->foo(std::move(a)); +} +} // namespace CalleeSequencedBeforeArguments ---------------- PiotrZSL wrote: > I didn't found any test for correct warning in this case: > > ``` > std::unique_ptr<A> getArg(std::unique_ptr<A>); > getArg(std::move(a))->foo(std::move(a)); > ``` Good point -- added below (in slightly different form, but testing the same thing). Note that this is an error in both C++11 and C++17, but in C++11 the use and move are unsequenced, whereas in C++17 the use definitely comes after the move. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145581/new/ https://reviews.llvm.org/D145581 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
