Any::cast fails for references

[kb0n]

Situation

I have roughly the following situation:

• I store user-structs in the blackboard, not always small ones
• I want to read them regularly (e.g. for displaying its contents in a GUI)
• => I don't want to create copies each time

So what I want to do is roughly:

struct Foo {...}; // some larger struct; stored an instance in blackboard with key "someKey"

const BT::Any* value = myBlackboard->getAny("someKey");
const auto& entry = value->cast<const Foo&>();

any_cast supports casting to references, so intuitively this should be working. But I can only invoke

value->cast<Foo>(); 
value->cast<const Foo>();

Error

This does not compile. Error (filtered)

/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/expected.hpp: In instantiation of ‘class nonstd::expected_lite::expected<const Foo&, std::__cxx11::basic_string<char> >’:
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/safe_any.hpp:137:34:   required from ‘T BT::Any::cast() const [with T = const Foo&]’
/testCode.cpp:160:83:   required from here
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/expected.hpp:1224:34: error: forming pointer to reference type ‘nonstd::expected_lite::expected<const Foo&, std::__cxx11::basic_string<char> >::value_type’ {aka ‘const Foo&’}
 1224 |     constexpr value_type const * operator ->() const
      |                                  ^~~~~~~~
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/expected.hpp:1229:18: error: forming pointer to reference type ‘nonstd::expected_lite::expected<const Foo&, std::__cxx11::basic_string<char> >::value_type’ {aka ‘const Foo&’}
 1229 |     value_type * operator ->()
      |                  ^~~~~~~~
In file included from /BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/safe_any.hpp:14,
                 from /BehaviorTree.CPP/include/behaviortree_cpp_v3/basic_types.h:14,
                 from /BehaviorTree.CPP/include/behaviortree_cpp_v3/blackboard.h:12,
                 from /project/libs/p90902/components/task_management/Blackboard.h:16,
                 from /testCode.cpp:1:
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/expected.hpp: In instantiation of ‘union nonstd::expected_lite::detail::storage_t<const Foo&, std::__cxx11::basic_string<char> >’:
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/expected.hpp:1383:28:   required from ‘class nonstd::expected_lite::expected<const Foo&, std::__cxx11::basic_string<char> >’
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/safe_any.hpp:137:34:   required from ‘T BT::Any::cast() const [with T = const Foo&]’
/testCode.cpp:160:83:   required from here
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/expected.hpp:400:24: error: forming pointer to reference type ‘nonstd::expected_lite::detail::storage_t<const Foo&, std::__cxx11::basic_string<char> >::value_type’ {aka ‘const Foo&’}
  400 |     value_type const * value_ptr() const
      |                        ^~~~~~~~~
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/expected.hpp:405:18: error: forming pointer to reference type ‘nonstd::expected_lite::detail::storage_t<const Foo&, std::__cxx11::basic_string<char> >::value_type’ {aka ‘const Foo&’}
  405 |     value_type * value_ptr()
      |                  ^~~~~~~~~
/BehaviorTree.CPP/include/behaviortree_cpp_v3/utils/expected.hpp:421:16: error: non-static data member ‘nonstd::expected_lite::detail::storage_t<const Foo&, std::__cxx11::basic_string<char> >::m_value’ in a union may not have reference type ‘nonstd::expected_lite::detail::storage_t<const Foo&, std::__cxx11::basic_string<char> >::value_type’ {aka ‘const Foo&’}
  421 |     value_type m_value;

Reason

I looked into the code, also the error messages are quite obvious. Any::cast() invokes Any::convert(), which has a signature template <typename DST> nonstd::expected<DST,std::string> convert(...).

DST cannot be a reference though, does not work with nonstd::expected.

Suggested Fix

Fix is not straightforward becaus convert() should not be possible if we want to cast to a reference, since it returns a temporary nonstd::expected, and cast() returns one of its member. So we can't just e.g. modify cast() to do a auto res = convert<std::decay_t<T>>() => dangling reference.

So we could either:

• avoid that case with `if constexpr'

// safe_any.hpp
    template <typename T>
    T cast() const
    {
        if( _any.empty() )
        {
            throw std::runtime_error("Any::cast failed because it is empty");
        }
        if (_any.type() == typeid(T))
        {
            return linb::any_cast<T>(_any);
        }
+
+       // cannot use conversion for reference return type, since that uses temporary expecteds
+       if constexpr(std::is_reference_v<T>)
+       {
+           throw std::runtime_error( errorMsg<T>() );
+       }
        else
        {
            auto res = convert<T>();
            if( !res )
            {
                throw std::runtime_error( res.error() );
            }
            return res.value();
        }
    }

• or similar with non-cpp17 templates
• or change return-type of convert(EnableUnknownType<DST> = 0)

    template <typename DST>
-   nonstd::expected<DST,std::string> convert(EnableUnknownType<DST> = 0) const
+   nonstd::expected<std::decay_t<DST>,std::string> convert(EnableUnknownType<DST> = 0) const
    {
        return nonstd::make_unexpected( errorMsg<DST>() );
    }

Testing

I did a quick debugging in my IDE and can confirm that

• code compiles with the proposed fixes
• value->cast<Foo>(); returns a copy of the original
• value->cast<const Foo&>(); returns a const ref to the same data (so no copying here)

How to proceed

What's your opinion here? What would you suggest?
I think the last mentioned option is a bit hacky and doesn't cover all possible cases.
I was also wondering if it might've actually been a design-choice that Any::cast can only return copies.

[facontidavide]

I have been thinking about this and reference semantic is not really best practice in my opinion.

Blackboard are supposed to use value semantic as much as possible and, if not, shared_ptr seems like a much better alternative to references.
I prefer to forbid its use as you suggested in if constexpr

[facontidavide]

see the applied change