guard_impl objects should store references

[braden]

In multi_index::detail::scope_guard, it seems to me that the guard_impl objects should store references to their parameters rather than copies. Consider the following code:

  T * obj = 0;
  scope_guard guard = make_guard(delete_T, obj);

  obj = create_T();

  if (obj) {
    // do stuff
  } else {
    guard.dismiss();
  }

Since make_guard passes obj by copy (and the guard_impl holds a copy), the value passed to delete_T is always 0.

This behavior can be defeated by explicitly giving make_guard a reference parameter:

  make_guard<void (*)(T*), T*&>(delete_T, obj)

However, that obviously gets quite verbose; and I don't see a downside to storing the parameters as references consistently.

[joaquintides]

Logged In: YES 
user_id=911241
Originator: NO

Hello Braden,

Having guard_impl accept its args by reference has some
drawbacks of its own; accepting T& would preclude us to
pass temporaries:

scope_guard guard = make_guard(do_something_with_an_int,10);
// error: cannot match to T& to int

Overloading guard_impl to accept both T& and const T& fixes
this, but a subtle problem is introduced then:

scope_guard guard = make_guard(do_something_with_an_int,10);

A reference to the temporary 10 is stored, but this
temporary is no longer valid by the time the scope guard
commits! So it was decided that taking args by value is
safer. When you need to really pass a reference, you can do
it by using Boost.Ref (http://boost.org/doc/html/ref.html):

#include <boost/ref.hpp>

T * obj = 0;
scope_guard guard = make_guard(delete_T, boost::ref(obj));

obj = create_T();
...

Hope this helps. One final warning: the implementation of
scope_guard inside Boost.MultiIndex is stored in the detail
directory, which means it's not provided for public
consumption and you cannot really rely on it being available
in a future release of Boost. I don't plan to remove it, but
to be really safe you should use your homegrown version (it
suffices to use a modified version of the implementaton in
B.MI appropriately placed in a namespace of your own) until
an official scope guard lib makes it into Boost.

Hope this helps,

Joaquín M López Muñoz
Telefónica, Investigación y Desarrollo

[nobody]

Logged In: NO 

You're right, of course. For some reason I thought boost::ref wouldn't work here; but indeed it does.

With regard to using stuff in a "detail" namespace, it's my understanding that boost doesn't make any guarantees about the consistency of its API between releases. Unless/until that changes, I think the practical value of the hint "detail" provides is only to segregate the intentional interface from the incidental one.