Reference semantics in VariableBlockVector

When "copying" a block from a VariableBlockVector, one has reference semantics due to Dune::Imp::BlockVectorWindow being used as return type of the operator[]:

using Vector = Dune::VariableBlockVector<...>;
Vector v(n);
// ... set up v
auto supposed_block_copy = v[0];
supposed_block_copy= foo(); // this also changes v!

I find this a little surprising, if not dangerous, because it's different from e.g. copying a block from a simple BlockVector. In my opinion, this should be made more explicit in the documentation.

Alternatively, I thought of a window implementation that can manage it's own data (e.g. via a unique_ptr) if someone tries to copy construct from it.

Such internal references a more general problem. A while back I introduced autoCopy() and AutonomousValue for such cases, though I guess we need a specialization for BlockVectorWindow to make it work.

Thanks for the quick reply. The autoCopy() mechanism looks good, though I fear it would still be easy to shoot oneself into the foot. One can not really guarantee that it's used in every algorithm in Dune.

E.g. from ildl.hh in ISTL:

auto rhs = v[ i.index() ];
ii->mv( rhs, v[ i.index() ] );

I did not test this, but I'd guess this would produce some funny results without even throwing or anything.

Yes, probably. autoCopy() is a stopgap measure -- i.e. you basically only insert it into an algorithm when you know you need it (e.g. the thing failed for you). There are rumors about an operator auto thingy, but even those rumors say the proposals are did not make it very far.

Would you be willing to provide a patch? I.e. the specialization for BlockVectorWindow, and inserting autoCopy() in whatever algorithm gave you problems?

Here someone also mentions operator auto (though without any further references). He also basically had the same idea as autoCopy(), except a very different syntax (which I don't like because it is basically impossible to find out more about by search engine if you encounter it).

Edit: Actually the article linked first contains a quick mention of operator auto

Also, Herb Sutter's GotW #94 (at the far and). Basically he argues that one can always cast explicitly when needed. Which may be true, but the problem is that you have to cast in surprising places. In that sense autoCopy() would be one of those type adjustment functions similar to as_signed().

Thanks for the references. So in summary proxy elements and auto are a little itchy in C++.

In Scott Meyer's "Effective Modern C++", Item 6, he arrives at a similar conclusion as Herb Sutter, that is to static_cast where needed.

So I end up with two options (which are not mutually exclusive):

1. implement autoCopy() for the window type and use it where needed

2. implement a constructor BlockVector(const BlockVectorWindow& window) for the BlockVector (might also be useful for 1.) and call

   auto block = static_cast<typename V::block_type>(v[i]);

   where needed.

The latter would be more in line with what the C++ gurus recommend, but it also requires every vector type used has a typedef block_type (and maybe there are types where one actually wants to copy the proxy?).

In 2.

• typename V::block_type is just a variant of AutonomousValue<decltype(v[i])>, with the disadvantage that it actually requires V to be of a type that can have member typedefs (i.e., no references, C-arrays or pointers possible) and that must conform to a stricter interface than otherwise necessary.
• the static cast is just an uglier version of autoCopy() that places the burden of determining the exact type on the programmer.

Also, in item 6 Scott Meyer does not consider generic template code. Herb Sutter did in his GotW, and thus he allowed for type adjustment functions, which is exactly what autoCopy() is.

Additionally, something needs to be done everywhere that this problem manifests. Either some form of cast with the type explicitly given (as in 2.), or a cast with the type implicitly given as in autoCopy(). Using autoCopy() is just the more generic and less painful thing to do.

Following the behaviour of std::vector<bool> I'd suggest to use V::reference_type and V::value_type. V::value_type is what the container conceptually contains, V::reference_type is what you get after dereferencing iterators and using operator[]. Notice that V::reference_type does not need to be V::value_type& but can be a proxy object that can be converted to V::value_type. This seems to be the standard pattern that we also used in the other proxy containers in dune.

It may be worth to have a look at the ranges proposal, which probably also uses proxy objects.

I agree that the autoCopy() approach is better for templated code. I will prepare a merge request for the general feature of using it for the BlockVectorWindow and then have a look at where it should be used.

mentioned in merge request !215 (merged)

closed via merge request !215 (merged)

mentioned in commit fde13e53