Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
shierei opened this issue Dec 5, 2018 · 18 comments
Open
Assignees

Comments

@shierei
Copy link
Contributor

shierei commented Dec 5, 2018

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.

@billyquith
Copy link
Owner

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.

@billyquith
Copy link
Owner

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

@shierei
Copy link
Contributor Author

shierei commented Jan 23, 2019

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.

@billyquith
Copy link
Owner

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

@shierei
Copy link
Contributor Author

shierei commented Jan 24, 2019

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

@billyquith
Copy link
Owner

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

@shierei
Copy link
Contributor Author

shierei commented Jan 24, 2019

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

@billyquith
Copy link
Owner

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

@shierei
Copy link
Contributor Author

shierei commented Jan 24, 2019

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

@shierei
Copy link
Contributor Author

shierei commented Jan 24, 2019

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

@billyquith
Copy link
Owner

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

@billyquith
Copy link
Owner

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.

@shierei
Copy link
Contributor Author

shierei commented Feb 4, 2019

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

@billyquith
Copy link
Owner

billyquith commented Feb 4, 2019

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).

@billyquith
Copy link
Owner

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!

@billyquith
Copy link
Owner

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.

@shierei
Copy link
Contributor Author

shierei commented Mar 8, 2019

That sounds promising. Will call by pointer work also?

@billyquith
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants