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

Subject: Re: [Boost-users] [Any] inconsistent return types of any_cast overloads
From: Antony Polukhin (antoshkka_at_[hidden])
Date: 2013-12-02 16:22:11

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

-- 
Best regards,
Antony Polukhin