Re: [boost] [variant] wish list

From: Yuval Ronen (ronen_yuval_at_[hidden])
Date: 2006-06-09 12:06:57

Eric Friedman wrote:
>>> As for operator!= (and operator<=, etc.), while I do think it may be 
>>> useful, it raises the question of what requirements variant imposes on 
>>> its bounded types. Specifically, adding support for these operators is 
>>> not so straightforward as saying operator!= is simply !(*this == rhs). 
>>> The idea of operator== and operator< is that if the current bounded type 
>>> is the same in both variant instances, then operator overload _for that 
>>> type_ will be invoked. To support this same behavior for the other 
>>> operators would require adding to the bounded type requirements. See 
>>> <http://boost.org/doc/html/variant/reference.html#variant.concepts> for 
>>> a refresher on variant's approach to concepts. Maybe this is an 
>>> overly-pedantic response on my part, though.
>>
>> I think it *is* as straightforward as saying operator!= is simply 
>> !(*this == rhs). There is no other way that makes sense.
> 
> I of course agree with you that this definition of operator!= makes 
> sense, but I am reluctant because this is not how C++ works: the 
> language allows entirely separate definitions of operator== and 
> operator!=. It would seem unfortunate I think if variant called 
> operator==(T,T) but never operator!=(T,T)...

It's true that C++ allows for entirely separate definitions of 
operator== and operator!=, but we have to make the assumption that these 
definitions always compare for equality. In fact, you made this 
assumption yourself when implementing variant's operator==. The code 
that returns false if this->which() != rhs.which() is exactly such 
assumption - why return false and not some other thing?

That said, if you choose to implement operator!= using T's operator!=, 
then I'd be completely cool with that.

>> Operators >, <=, >=, on the other hand, are completely different. I 
>> think there is no natural ordering of variant objects, so not supporting 
>> them is logical. Operator < is an exception because it's used for 
>> ordered conrainers. IOW, I completely agree with your decision to supply 
>> operator < and not supplying operators >, <=, >=.
> 
> Now how is that operator!= as !(lhs == rhs) makes sense, but operator<= 
> as (lhs < rhs || lhs == rhs) is completely different?

Because operator== has only one purpose in C++: comparison for equality. 
Operator<, on the other hand, has two common roles.

For classes that have natural ordering, it should define this ordering, 
and then all the other operators (> <= >=) should be defined as well. 
Classes that doesn't have this natural ordering, often still define 
operator< just to be able to be used in ordered containers. In such 
cases, operator< doesn't define a natural ordering because there isn't 
any. Instead, it just defines an ordering "that works" for ordered 
containers, nothing more. It doesn't need to compatible with operator== 
and !=, and defining operators >, <=, >= is misleading and should be 
avoided.

boost::shared_ptr is a good example of such a class with no natural 
ordering, with operator== and !=, with operator<, but without the other 
operators. And shared_ptr is of TR1 strength...

The question we need to ask, is whether variant has a natural ordering, 
and I think the answer is "no". All the rest is just a consequence of it.

>>> The reason (or at least one of them) that apply_visitor accepts visitors 
>>> by reference and not by value is that I had intended to introduce a 
>>> dynamic_visitor counterpart to static_visitor in a more general 
>>> Boost.Visitor library. Passing by value would interact poorly with 
>>> polymorphic dynamic visitors due to slicing. If you are interested, I 
>>> believe Boost.Visitor is probably still sitting around in the Boost 
>>> Sandbox CVS on Sourceforge.
>>
>> Then maybe different names should be used for dynamic_visitors, for 
>> example apply_dyn_visitor(). We need to consider whether the need for 
>> dynamic_visitors is large enough in order to justify the negative impact 
>> on the static_visitor's interface. Have you found there is great need 
>> for dynamic visitors? I have to admit that I've never needed them... Not 
>> to mention that the whole standard library is based on static functors 
>> passed by value...
> 
> I think my opinion here is that a visitor is fundamentally different 
> from a functor. Functors model function types; they may be implemented 
> with a template, but logically they have a single signature. By 
> contrast, a visitor is a set of functions with different signatures used 
> to handle a set of different cases.

I think the difference you rely on is of no significance. From the 
user's point of view, the functor passed to std::for_each is to be 
applied to the values in the container, just as the visitor should be 
applied to the value in the variant. The fact that there are several 
options, and hence there need to be several functions with different 
signatures, doesn't matter at all.

IMO, the only thing the user is concerned about is passing his value to 
the function. Call it functor or visitor doesn't change anything, and 
may actually be misleading. So maybe only the dynamic_visitor is a 
visitor, and the static_visitor is just a variant_functor. That's my 
humble opinion.

> Also, looking back to your original email, you mentioned a desire to 
> pass temporary visitor objects. I believe that is supported with the 
> current implementation. The following compiles just fine under gcc 3.4:
> 
>    #include "boost/variant.hpp"
> 
>    class TestVisitor : public boost::static_visitor<>
>    {
>    public:
> 
>      template <typename T>
>      void operator()(const T&) const
>      { /*...*/ }
> 
>    };
> 
>    int main()
>    {
>      boost::variant<int, double> var;
>      apply_visitor(TestVisitor(), var);
>    }

If this code works, then it's great, sure (I find calling a member 
function a bit more elegant than calling a free function, but it's 
definitely not a deal-breaker), but it doesn't change the principal I 
tried to convince you of in the previous paragraph.

>>> Variant already supports reference types, at least on modern compilers. 
>>> However, references are not assignable, and so as noted in the concepts 
>>> documentation for variant, this implies that the resultant variant is 
>>> not assignable. To address your comment about boost::optional, then, 
>>> variant does not allow rebinding of references.
>>
>> No problems here.
>> I just want to express my opinion that the decision regarding reference 
>> support should apply both to Optional and to Variant.
> 
> So you think Optional should change or Variant should change?

If only I knew...
I have to say I don't really know the answer to this tough question. the 
only thing I do know, is that the decision should be variant/optional 
unanimous.

Thanks again for taking the time to relate to my points,
Yuval