base_from_member with reference types

[K-ballo <kaballo86@…>]

base_from_member constructors take their arguments by value, which causes undefined behavior if the member type is a reference, as a dangling reference is created (unless, I presume, its initialized with boost::ref/cref).

The attached patch adds a partial specialization of base_from_member for references. It provides a single constructor that takes its argument by reference.

[K-ballo <kaballo86@…>]

Fix

[viboes]

Could you show a use case for a base_from_member with a reference and the generated error?

[kaballo86@…]

reference base_from_member issue sample

[kaballo86@…]

I have added an use case with a reference base_from_member resulting in undefined behavior. It's based on my original use case, where I had a view over a pair of iterators and I wanted to implement a view over a container reusing the previous view. When base_from_member is instantiated with a reference, the reference points to the copy of the argument used in its construction, so it effectively becomes a dangling reference.

[viboes]

Initially I didn't see the need to use base_from_member with a reference, as the referenced object is already initialized. IIUC you want that your class container_something should be able to be instantiated either with a Container and then copy it, or with a Container& and then store the reference. I find this usage a little bit bizarre. Could you tell me in which cases do you use Container and when you use Container&?

[kaballo86@…]

I could tell you more about the specific use cases, but I think you are missing the point. When instantiating a base_from_member with a reference type the member will contain a dangling reference. There is no error, no warning, no assertion; only a crash at runtime if you are lucky enough.

A reference base_from_member cannot be properly initialized (I even failed to get it working by using boost::ref/cref). If my fix is rejected, then at least there should be an assert or a big bold note in base_from_member documentation saying that base_from_member cannot be used with reference types. That is unless you could tell me how would I properly initialize a base_from_member instantiated with a reference type.

[viboes]

Replying to kaballo86@…:

I could tell you more about the specific use cases, but I think you are missing the point. When instantiating a base_from_member with a reference type the member will contain a dangling reference. There is no error, no warning, no assertion; only a crash at runtime if you are lucky enough.

No I didn't missed the point.

A reference base_from_member cannot be properly initialized (I even failed to get it working by using boost::ref/cref). If my fix is rejected, then at least there should be an assert or a big bold note in base_from_member documentation saying that base_from_member cannot be used with reference types. That is unless you could tell me how would I properly initialize a base_from_member instantiated with a reference type.

I understand the problem and how your patch solves the issue.

Note that I'm not against your patch, I was just curious to know when you use one or the other.

[anonymous]

Ok, I was just making sure you didn't miss the actual issue with base_from_member.

The reason the member may be either a reference or a copy is simple. The class is a view, which means most of the time it will contain a reference to the actual type is adapting. However, a view could also be adapting another view, and in those cases it should contain a copy to the view rather than forcing the user to define additional named variables for the intermediate views. Then one can chain views the obvious way.

[viboes]

Replying to anonymous:

Ok, I was just making sure you didn't miss the actual issue with base_from_member.

The reason the member may be either a reference or a copy is simple. The class is a view, which means most of the time it will contain a reference to the actual type is adapting. However, a view could also be adapting another view, and in those cases it should contain a copy to the view rather than forcing the user to define additional named variables for the intermediate views. Then one can chain views the obvious way.

Why it needs to copy the view? A concrete example will help.

[kaballo86@…]

Replying to viboes:

Why it needs to copy the view? A concrete example will help.

Because that's the way it's usually done: views are copied. Views don't own their elements, in a way they are already references. Also, this allows chaining views, for instance one_view( another_view( some_container ) ) where the intermediate temporary view cannot be taken by reference.

This is totally unrelated to the issue with base_from_member, and as such I think it is off-topic. If you want to continue this discussion contact me to my email address, which you should be able to see here.

[viboes]

Please, could you provide a patch for the documentation?

[K-ballo <kaballo86@…>]

I can certainly try. Should that be documentation stating the issue as it stands today, or a reference for the specialization provided by my patch?

[viboes]

Replying to K-ballo <kaballo86@…>:

I can certainly try. Should that be documentation stating the issue as it stands today, or a reference for the specialization provided by my patch?

The specialization provided by the patch.

[Andrey Semashev]

Merged in ​https://github.com/boostorg/utility/pull/12.