Use-after-move warning?

Hi all,

I recently got bitten by a bug that was caused by using a value (more specifically, returning it from a function) after moving it. Since this move happened only within one rarely executed conditional branch of a fairly large function, it wasn’t visually obvious why the behavior was different from what I expected, and it took me some time to track the issue down.

Obviously using a value after moving from it is legal C++, but in my own experience with using std::move, code like this almost always indicates a bug, or at least bad style. I’m curious whether others feel the same way. If use-after-move is only rarely useful, it would be nice to print a warning when it happens, perhaps with a way to disable the warning by using more explicit syntax (similar to how the warning for assignment in an ‘if’ conditional expression works).

If there was a patch to implement this, would there be support for including it in clang?

Thanks,
- Seth

Clang already has some infrastructure to try warn for this - the “consumed” analysis is a broad analysis designed to be used with type attributes to indicate which operations are valid in which states of an object. Some default behavior for moved-from objects might be helpful, but I’m not sure.

if the object you’re dealing with only became moved-from in one condition, but was returned unconditionally:

T t = init();
if (x)
func(std::move(t));

return t;

I doubt it’s reliable enough to warn on that generally - the user might’ve wanted to return an empty value in that situation. (think of the case where T is std::unique_ptr, with a well defined move-from state that users might rely on).

I think that the number of times that I've used that pattern is a tiny handful of the times where I've used std::move, so perhaps an attribute to flag that 'yes, I really meant to return this value even though it's now empty' is the way forward?

David

> I doubt it's reliable enough to warn on that generally - the user
might've wanted to return an empty value in that situation. (think of the
case where T is std::unique_ptr, with a well defined move-from state that
users might rely on).

I think that the number of times that I've used that pattern is a tiny
handful of the times where I've used std::move, so perhaps an attribute to
flag that 'yes, I really meant to return this value even though it's now
empty' is the way forward?

Perhaps - I'm just suggesting some of the gotchas I'd want to keep an eye
out for when evaluating the quality of such a warning. Numbers trump
everything - if someone implements it, surveys a large codebase, and finds
few enough false positives (with reasonable ways to rewrite them to not
trigger the warning) - that's success.

The consumed analysis tracks three states: “consumed”, “unconsumed”, and “unknown”. If an object is only consumed on one branch, then it would wind up in the “unknown” state afterward.

You can specify which operations are permitted in which states. For a strict analysis, you can say “warn whenever a unique_ptr is dereferenced in any state other than unconsumed.” A less strict version might say “only warn on dereference if it’s in the consumed state.” The less strict version will have a higher false negative rate, but a much lower false positive rate, especially for a widely-used type like unique_ptr.

That’s where things get tricky, because the C++ standard says that std::move must leave the object in a valid state. Some people rely on that behavior in their code, and routinely use null unique_ptrs, whereas some other people don’t. So for a widely-used type, it’s difficult to come up with a set of annotations that works for everyone. It’s much easier if you’re willing to create a strict_unique_ptr class (or something similar), that absolutely does not allow use-after-move.

BTW, the consumed analysis works, but hasn’t been widely deployed, due to the false positives issue. Expect to see some bugs.

-DeLesley

For a strict analysis, you can say "warn whenever a unique_ptr is dereferenced in any state other than unconsumed." A less strict version might say "only warn on dereference if it's in the consumed state." The less strict version will have a higher false negative rate, but a much lower false positive rate, especially for a widely-used type like unique_ptr.

These warnings seem like clear wins, and I’d definitely find them useful!

That's where things get tricky, because the C++ standard says that std::move must leave the object in a valid state. Some people rely on that behavior in their code, and routinely use null unique_ptrs, whereas some other people don't. So for a widely-used type, it's difficult to come up with a set of annotations that works for everyone. It's much easier if you're willing to create a strict_unique_ptr class (or something similar), that absolutely does not allow use-after-move.

A new class where everyone agrees on the allowed use-after-move behavior would be ideal, definitely. If we had annotations that people could use to mark their own custom classes as forbidding use-after-move, that would be enough to let people experiment with this in their own codebases, and if this approach got popular enough I imagine you’d find a class similar to strict_unique_ptr appearing in Boost or some other popular library soon enough.

So it sounds to me like so far, a refined proposal might have involve adding:

(1) Warnings about dereferencing a unique_ptr in any state other than unconsumed. (Or maybe the less strict version above, if this version has too high a false-positive rate.)
(2) A class annotation that tells the compiler to warn if objects of that class are touched at all in any state other than unconsumed.

Can we generalize (1) to an annotation that might be more broadly applicable? A straw-man annotation version of might be:

“(1) A class annotation that tells the compiler to warn if operator*, operator->, or operator[] is invoked on objects of that class in any state other than unconsumed.”

This seems applicable to e.g. shared_ptr as well.

I’d hope that the annotation version of (1) would be uncontroversial enough that it could be applied by default to smart pointer types in libc++. I’m not sure whether there are any types in the standard library that could get (2) by default without inconveniencing some users, but it doesn’t seem out of the question; std::unique_lock seems like a candidate.

Do these annotations sound like reasonable things to implement?

- Seth

The annotations are already implemented. :slight_smile: You are free to experiment with your own version of unique_ptr that does not allow dereference after consume. See warn-consumed-analysis.cpp for examples.

-DeLesley