mboehme added inline comments.
================ 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: > mboehme wrote: > > 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. > I see this ```b.foo(a->saveBIntoAAndReturnBool(std::move(b)));``` like this: > we call saveBIntoAAndReturnBool, that takes b by std::move, then we call foo > on already moved object. > For me this is use after move, that's why I was asking. > > And in "b.foo" there is almost nothing to evaluate, maybe address of foo, but > at the end foo will be called on already moved object. > If we would have something like "getSomeObj(b).boo(std::move(b))" then we can > think about "evaluate", but when we directly call method on moved object, > then we got use after move > > Ah, I think I understand what you're getting at now. I was assuming for some reason that `b` was also a `unique_ptr` in this example, but of course that doesn't make sense because in that case we wouldn't be able to use the dot operator on `b` (i.e. `b.foo`). Distinguishing between these two cases will require making the check more sophisticated -- the logic that the callee is sequenced before the arguments is not sufficient on its own. I'll have to take a closer look at how to do this, but it will likely involve looking at the `MemberExpr` inside the `CXXMemberCallExpr`. If `MemberExpr::getBase()` is simply a `DeclRefExpr`, we'll want to do one thing, and if `MemberExpr::getBase()` is some sort of `CallExpr`, we'll want to do something else. There will likely need to be other considerations involved as well, but I wanted to sketch out in broad lines where I think this should go. I'll likely take a few days to turn this around, but in the meantime I wanted to get this comment out to let you know that I now understand the issue. 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
