[PATCH] D21223: [clang-tidy] misc-move-const-arg: Detect if result of std::move() is being passed as a const ref argument

[Martin Böhme via cfe-commits]

mboehme added inline comments.

================
Comment at: test/clang-tidy/misc-move-const-arg.cpp:75-76
@@ +74,4 @@
+
+class NonMoveable {
+ public:
+  NonMoveable();
----------------
aaron.ballman wrote:
> > Can you expand on this?
> >
> > The standard says: "If the definition of a class X does not explicitly declare a move constructor, one will be implicitly declared as defaulted if and only if - X does not have a user-declared copy constructor [...]" (12.8/9).
> >
> > Because I'm declaring a copy constructor, I would thus have expected not to get an implicitly-declared move constructor. Where am I going wrong here?
> 
> Move operations are optimized versions of a copy operation, so a failed move operation can fall back to perform a copy operation instead and achieve the same operational semantics but with slightly worse performance. Because you have a copy constructor, you do not get an implicitly-declared move constructor (so that's correct), but that doesn't mean the type cannot be used in a context where the user expects a move (i.e., calling `std::move()` on it) -- the operation just falls back on the copy constructor.
> 
> Basically, there's an operational difference between not having the implicitly-declared move constructor and having a deleted move constructor. In the former, fallback to copy happens and in the latter you get a diagnostic. So when I hear "non-moveable type", my brain assumes you mean "get a diagnostic when you try to move."
Ah, now I see what you mean.

I've renamed "NonMoveable" to "NoMoveSemantics" (and "Moveable" to "MoveSemantics" for consistency). What do you think?

(I don't like NonMoveConstructible because it's prone to the same ambiguity -- or even more so, because std::is_move_constructible<> uses exactly the same term and the type is move constructible in the sense of that trait.)


http://reviews.llvm.org/D21223