mboehme marked 9 inline comments as done.
================
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:493
@@ +492,3 @@
+ if (!S)
+ continue;
+
----------------
For some reason, I thought I had read that they weren't compatible with CFG /
CFGBlock -- but obviously I must have been imagining things. ;)
Also changed the other occurrences.
================
Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:16
@@ +15,3 @@
+The last line will trigger a warning that ``str`` is used after it has been
+moved.
+
----------------
There doesn't seem to be a clear preference here (see alexfh's comments on
similar cases), so I'll leave this open until it's resolved one way or the
other.
================
Comment at: docs/clang-tidy/checks/misc-use-after-move.rst:182
@@ +181,3 @@
+
+ struct S {
+ std::string str;
----------------
Sorry -- forgot to check that the documentation compiles (it does now).
================
Comment at: test/clang-tidy/misc-use-after-move.cpp:159
@@ +158,3 @@
+ ptr.get();
+ }
+ // This is also true if the std::unique_ptr<> or std::shared_ptr<> is wrapped
----------------
No, it doesn't. At the moment, the check intentionally disregards all uses of
unique_ptr and shared_ptr (see also the documentation).
I agree that it definitely makes sense to check for scenarios like the one you
mention. They're a bit of a different beast though because unique_ptr and
shared_ptr have a well-defined state after they've been moved from. This means
they would require some special logic -- we'd want to disallow ptr->Foo() after
a std::move, but not ptr.get(). For this reason, I've left them out of this
initial version.
Also, I'm not sure whether this check is the best place for these unique_ptr
and shared_ptr checks to live. Because the after-move state of unique_ptr and
shared_ptr is well defined, the "use, then dereference" case is really just a
subset of what could be a more general "dereference null pointer" check.
================
Comment at: test/clang-tidy/misc-use-after-move.cpp:280
@@ +279,3 @@
+ A a;
+ std::move(a);
+ auto lambda = [&]() { a.foo(); };
----------------
> can you add tests with reference capture?
Done.
> also what about: [snip]
The check won't warn about this. More generally, it doesn't do any kind of
inter-procedural analysis.
Inter-procedural analysis would certainly help in a number of ways. For
example, it could be used to answer the following:
- If a function takes a non-const reference to an object, does it reinitialize
that object? (Currently, we optimistically assume that it always does.)
- If a function (that isn't a move constructor or move assignment operator)
takes an rvalue reference to an object, does it actually move from that object,
and does it do so unconditionally?
However, this would take significant additional implementation effort and would
also run more slowly.
https://reviews.llvm.org/D23353
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
