2013/12/2 Antony Polukhin

2013/12/2 Arne Vogel

...

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(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(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(...). 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(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(makeVec()); instead of const auto vec = boost::any_cast(makeVec()); ? Regards, Kris

2013/12/4 Arne Vogel

On 03.12.2013 00:01, Krzysztof Czainski wrote: [...] what do you gain by doing:

const auto& vec = boost::any_cast(makeVec()); instead of const auto vec = boost::any_cast(makeVec()); ?

Regards, Kris

In C++11 with a moveable type, probably not much. I don't quite remember why I wrote the code like this - it could have been out of habit since in C++98 using a reference may save a copy, though I guess an optimizing compiler had already been allowed to elide it.

I'm just curious to know if anyone uses a compiler that doesn't actually elide the copy (or move) in this case ;-) In all discussions about this I've never seen someone actually name one, but still they try to avoid the copy just in case. I think taking values returned by value by const & to extend their lifetime should be deprecated in new C++ - compilers could then warn not to do it. Cheers, Kris

On 12/4/2013 7:07 AM, Krzysztof Czainski wrote:

2013/12/4 Arne Vogel

...

On 03.12.2013 00:01, Krzysztof Czainski wrote: [...] what do you gain by doing:

const auto& vec = boost::any_cast(makeVec()); instead of const auto vec = boost::any_cast(makeVec()); ?

Regards, Kris

In C++11 with a moveable type, probably not much. I don't quite remember why I wrote the code like this - it could have been out of habit since in C++98 using a reference may save a copy, though I guess an optimizing compiler had already been allowed to elide it.

I'm just curious to know if anyone uses a compiler that doesn't actually elide the copy (or move) in this case ;-) In all discussions about this I've never seen someone actually name one, but still they try to avoid the copy just in case.

I think taking values returned by value by const & to extend their lifetime should be deprecated in new C++ - compilers could then warn not to do it.

Cheers, Kris

An old article comes to mind which uses exactly this idea to implement a little API to help people write exception-safe code [1]. This is part of the Loki library [2]. I think this is still considered a legitimate technique, right? Andy 1. http://www.drdobbs.com/cpp/generic-change-the-way-you-write-excepti/18440375... 2. http://loki-lib.sourceforge.net/