Mir

Code review comment for lp:~cemil-azizoglu/mir/improve-raii

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

348 + release_fn(std::move(other.release_fn))
367 + if (release_fn)

Unfortunately the standard says that move-constructing a std::function from another std::function leaves the latter in a valid but unspecified state. So "if (release_fn)" in the destructor is not guaranteed to be false after the move. We could use "if (wrapped && release_fn)" instead, since std::shared_ptr provides the guarantee we need.

Also note that after we remove (as mentioned by Alberto above):

356 + if (release_fn)
357 + release_fn(wrapped.get());

we could use the default implementations of move-construct and move-assign methods (we would need to explicitly define them with "= default");

review: Needs Fixing

« Back to merge proposal