[Boost-bugs] [Boost C++ Libraries] #9462: Fix ValueType&& any_cast(any&&)

Subject: [Boost-bugs] [Boost C++ Libraries] #9462: Fix ValueType&& any_cast(any&&)
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2013-12-02 21:21:04

#9462: Fix ValueType&& any_cast(any&&)
--------------------------+------------------------
 Reporter:  apolukhin     |      Owner:  apolukhin
     Type:  Bugs          |     Status:  new
Milestone:  Boost 1.56.0  |  Component:  any
  Version:  Boost 1.54.0  |   Severity:  Regression
 Keywords:                |
--------------------------+------------------------
 Thanks to the Arne Vogel for investigating and reporting this issue:

 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.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/9462>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.