billyquith/ponder

call by reference, using a pointer or a non-const reference, does not work

shierei opened this issue · 18 comments

Declaring a metaclass function with a pointer or a non-const reference to any basic type as an argument does not compile. The pointer to a basic type can be made compilable if you register this basic type. But if you register this basic type, and property or function returning value of this type becomes ponder::ValueKind::User. This prevents you from converting the property value or the function return value back to the basic type. The conversion would throw an exception. It basically makes this basic value type used as a property value or a function returning value not visible to the client.

This is an awkward one to fix. However, I would like to fix it. It is tricky because Values handle basic types, but they do not support references to basic types. Hence you have to reference them as user types, which do support references (both ref and values). It seems a bit overkill to support basic types like this, but perhaps that should be a default solution until I can think of a better one.

Note to self: Value holder policies. external/internal.

I have a fix to support pointer typed argument nicely. That is all you need if you want a metaclass's function to return something through a function argument with a pointer to a basic type.

Sure, yes, I’d be interested in seeing it.

I like to show you in a tested code but it looks like currently the ponder branch is not buildable.

What seems to be broken? All of the tests appear to be passes in develop and master branches on all platforms.

I was not able to build on a RedHat platform. On Mac, it built however. Below is the build error.

[  1%] Building CXX object CMakeFiles/ponder.dir/src/class.cpp.o
In file included from ./ponder/include/ponder/detail/idtraits.hpp:57:0,
                 from ./ponder/include/ponder/config.hpp:79,
                 from ./ponder/include/ponder/classget.hpp:36,
                 from ./ponder/include/ponder/class.hpp:34,
                 from ./ponder/src/class.cpp:30:
./ponder/include/ponder/detail/string_view.hpp: In instantiation of  constexpr int ponder::detail::basic_string_view<CharT, Traits>::compare(ponder::detail::basic_string_view<CharT, Traits>) const [with CharT = char; Traits = std::char_traits<char>] :
./ponder/include/ponder/detail/string_view.hpp:301:85:   required from  constexpr bool ponder::detail::basic_string_view<CharT, Traits>::operator<(const ponder::detail::basic_string_view<CharT, Traits>&) const [with CharT = char; Traits = std::char_traits<char>] 
./ponder/include/ponder/detail/dictionary.hpp:46:49:   required from  bool ponder::detail::DictKeyCmp<T>::operator()(T, T) const [with T = ponder::detail::basic_string_view<char>] 
./ponder/include/ponder/detail/dictionary.hpp:102:59:   required from  ponder::detail::Dictionary<KEY, KEY_REF, VALUE, CMP>::const_iterator ponder::detail::Dictionary<KEY, KEY_REF, VALUE, CMP>::findKey(KEY_REF) const [with KEY = std::basic_string<char>; KEY_REF = ponder::detail::basic_string_view<char>; VALUE = std::shared_ptr<ponder::Function>; CMP = ponder::detail::DictKeyCmp<ponder::detail::basic_string_view<char> >; ponder::detail::Dictionary<KEY, KEY_REF, VALUE, CMP>::const_iterator = __gnu_cxx::__normal_iterator<const ponder::detail::Dictionary<std::basic_string<char>, ponder::detail::basic_string_view<char>, std::shared_ptr<ponder::Function> >::pair_t*, std::vector<ponder::detail::Dictionary<std::basic_string<char>, ponder::detail::basic_string_view<char>, std::shared_ptr<ponder::Function> >::pair_t, std::allocator<ponder::detail::Dictionary<std::basic_string<char>, ponder::detail::basic_string_view<char>, std::shared_ptr<ponder::Function> >::pair_t> > >] 
./ponder/include/ponder/detail/dictionary.hpp:119:40:   required from  bool ponder::detail::Dictionary<KEY, KEY_REF, VALUE, CMP>::tryFind(KEY_REF, ponder::detail::Dictionary<KEY, KEY_REF, VALUE, CMP>::const_iterator&) const [with KEY = std::basic_string<char>; KEY_REF = ponder::detail::basic_string_view<char>; VALUE = std::shared_ptr<ponder::Function>; CMP = ponder::detail::DictKeyCmp<ponder::detail::basic_string_view<char> >; ponder::detail::Dictionary<KEY, KEY_REF, VALUE, CMP>::const_iterator = __gnu_cxx::__normal_iterator<const ponder::detail::Dictionary<std::basic_string<char>, ponder::detail::basic_string_view<char>, std::shared_ptr<ponder::Function> >::pair_t*, std::vector<ponder::detail::Dictionary<std::basic_string<char>, ponder::detail::basic_string_view<char>, std::shared_ptr<ponder::Function> >::pair_t, std::allocator<ponder::detail::Dictionary<std::basic_string<char>, ponder::detail::basic_string_view<char>, std::shared_ptr<ponder::Function> >::pair_t> > >] 
./ponder/include/ponder/class.inl:78:37:   required from here
./ponder/include/ponder/detail/string_view.hpp:201:5: error: body of constexpr function  constexpr int ponder::detail::basic_string_view<CharT, Traits>::compare(ponder::detail::basic_string_view<CharT, Traits>) const [with CharT = char; Traits = std::char_traits<char>]  not a return-statement
     }
     ^
make[2]: *** [CMakeFiles/ponder.dir/src/class.cpp.o] Error 1
make[1]: *** [CMakeFiles/ponder.dir/all] Error 2
make: *** [all] Error 2

Please note compiler version notes. C++14 required.

Cmake automatically configured it to use std=gnu++1y on the RedHat platform.

This must be new since I was able to build just a few days ago.

Yes. It’s mentioned in the change log and release notes.

This must be new since I was able to build just a few days ago.

I put a more explicit note about C++14 support in the notes. Sorry if it wasn't clear.

I upgraded my gcc to version 7.2 and it built fine for me. Please see my pull request for this discussion thread. Thanks.

Sorry, busy week at work. I have looked at the PR. Hopefully your patch works for now.

I had a think on the way to work. My current thinking is to add calling policies and/or argument policies. This would allow customisation of the argument passing. CAMP/Ponder currently converts arguments to Values and then unpacks them at the other end. This is slow and inefficient, so we could be more direct (avoiding a variant, and just checking the argument is correct or convertible).

I made some progress on this. There is a "feature/arg" branch with a working pass-by-ref for pointers. I'm not going to push it out because it doesn't handle references, just pointers.

I'm thinking of adding a "non-owned reference" type, which Value would use as a variant type. The basic types convert into their nearest equivalent type, but the reference would have to be identical. This is necessary because non-const references can't be returned otherwise (unless we convert them to UserObject reference holders, which is expensive and requires registering the basic types).

Ideally I'd like a "direct call" mechanism as well as a "Value call" mechanism. Direct call could be used for known compile-time function signatures, and "Value call" could be used where we perhaps have runtime data-driven reflection and we don't know the type of anything until we use it. Direct calling means we could bypass some of the inefficiencies of the temporary Value call. The Lua calling mechanism works like this.

Sorry if you aren't interested in all these ramblings. Just having a brain dump!

I experimented with adding a ValueRef type that is a cheaper version of UserObject byRef. This would mean you don't have to declare basic types to use them by reference.

That sounds promising. Will call by pointer work also?

Finally checked this into the develop branch. I'm a bit rusty no what I've done!