Re: [Boost-users] [Any] inconsistent return types of any_cast overloads

Subject: Re: [Boost-users] [Any] inconsistent return types of any_cast overloads
From: Krzysztof Czainski (1czajnik_at_[hidden])
Date: 2013-12-02 18:01:19

2013/12/2 Antony Polukhin <antoshkka_at_[hidden]>

>
> 2013/12/2 Arne Vogel <avogel_at_[hidden]>
>
>> Hello Boost Users,
>>
>> Code like the following used to work with Boost 1.53 on x86-64 Linux
>> using GCC 4.7 and -std=c++11:
>>
>> boost::any makeVec()
>> {
>>    return std::vector<int>{ 1, 2, 3 };
>> }
>>
>> void foo()
>> {
>>   const auto& vec = boost::any_cast<std::vector<int>>(makeVec());
>>   for ( auto value : vec )
>>   {
>>       // ...
>>   }
>> }
>>
>> The same code is likely to cause a segmentation violation using Boost
>> 1.55. According to the online documentation *1, any_cast has two overloads
>> accepting reference arguments:
>>
>> template<typename ValueType> ValueType any_cast(any&);
>> template<typename ValueType> ValueType any_cast(const any&);
>>
>> If these were indeed the only overloads accepting reference arguments,
>> there is no reason why the example code shouldn't work. ValueType
>> any_cast(const any&) should be the preferred overload and it returns a
>> vector<int> temporary whose lifetime is then extended to the end of the
>> block by being bound to a local const reference, as required by the C++
>> standard.
>>
>> However, in any.hpp, I discovered the following overload, which is only
>> declared if the macro BOOST_NO_CXX11_RVALUE_REFERENCES is not set. I
>> suppose this overload was added since 1.53, or that, previously, it was by
>> default omitted.
>>
>> template<typename ValueType> ValueType&& any_cast(any&&);
>>
>> This has higher precedence binding to the temporary returned from
>> makeVec, and returns an rvalue reference to the vector<int> contained in
>> said temporary, which is then implicitly converted to a const lvalue
>> reference as by the usual language rules. Since the lifetime of the
>> temporary returned by makeVec is NOT extended - this only works for
>> temporaries directly bound to a local const reference, whereas the result
>> of makeVec is "hidden" inside the call to any_cast - the temporary is
>> destroyed before the start of the for loop, which renders vec invalid and
>> the code a denizen of undefined behavior land.
>>
>> So much for the analysis, now for my actual question. :-)
>>
>> Why does the new overload return by reference when the old ones return by
>> value? The rationale for the old overloads was, I believe, "you should have
>> to ask for a reference if you want one", which makes sense given precisely
>> that using references tends to be more error prone if temporaries are
>> involved. So this would be the very reason why Any does not declare:
>>
>> template<typename ValueType> const ValueType& any_cast(const any&);
>>
>> even though it could be implemented as easily as the actual overload. If
>> I really wanted to get an const reference out of any_cast, I could write
>> something like
>>
>>   boost::any_cast<const ValueType&>(var);
>>
>> I can't think of a reason to be inconsistent in this regard and return a
>> reference if a temporary is passed to any_cast, instead of requiring a user
>> who really wants any_cast to return an rvalue reference to write
>> any_cast<ValueType&&>(...). That the overload breaks existing code and (due
>> to being undocumented and not checked at compile time *2), silently so,
>> whereas using ValueType any_cast(any&&) would at least fail to do any harm,
>> is, in my opinion, another reason to be sceptical of the current solution.
>>
>> Furthermore, there is a usability reason for preferring ValueType(any
>> &&): If ValueType &&(any &&) is declared and I do NOT want a reference, I
>> need to write more verbose and inelegant code than in the opposite case:
>>
>> // Name the temporary
>> const auto &vecAny = makeVec();
>> // Now, ValueType&& any_cast(any&&) is eliminated from candidate set (no
>> conversion from an lvalue to an rvalue reference)
>> const auto& vec = boost::any_cast<std::vector<int>>(vecAny);
>>
>> Note that the const is accidental here, using a mutable lvalue would work
>> as well.
>>
>> Looking forward to your opinions,
>> Arne Vogel
>>
>> *1) http://www.boost.org/doc/libs/1_55_0/doc/html/boost/any_
>> cast_idp25247624.html
>> *2) A compiler may issue a warning. However, I found that GCC 4.8.1 does
>> not warn, even with -Wall and the definition of any_cast being known at the
>> point of use.
>>
>
> You are absolutely right: ValueType&& any_cast(any&&) was not a good idea
> and it may break code that accepts result by const reference.
> Making it ValueType any_cast(any&&) would be more correct.
>
> I'll fix that as soon as the migration to the GIT will be finished.
>
> Great thanks for finding, investigating and reporting this issue!
>
> Created ticket #9462 : https://svn.boost.org/trac/boost/ticket/9462
>

That said, what do you gain by doing:
  const auto& vec = boost::any_cast<std::vector<int>>(makeVec());
instead of
  const auto vec = boost::any_cast<std::vector<int>>(makeVec());
?

Regards,
Kris