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

Embind and references #3480

Closed
AlexPerrot opened this issue May 29, 2015 · 7 comments
Closed

Embind and references #3480

AlexPerrot opened this issue May 29, 2015 · 7 comments

Comments

@AlexPerrot
Copy link
Contributor

Hello,

I made a simple API that uses method chaining in C++. This is achieved by returning a reference from member function, like this :

class Chain {
    Chain& foo() {
        return *this;
    }

   Chain& bar() {
        return *this;
    }
};

So, I can write Chain().foo().bar().

Now, when binding this with embind, references are copies of the original object, so memory leaks since I should call delete after each function call, preventing chaining.
I know I have several solutions to deal with this, namely using pointers, either directly in my class or in the embind bindings, but those are not very elegant solutions.

Is there any reason for which embind makes copies when using reference instead of using the same pointer (at least for objects) ?
If it is possible to implement reference support, I am willing to participate. Where should I start ?

@kripken kripken added the embind label Jun 1, 2015
@matthiasI
Copy link

I also realized that returning a pointer is different from returning a reference. In my eyes this is a bit an unexpected behavior.
I don't know the reasons. If it is not possible to do that, I think it would be at least good to warn about, similar as it is for pointers with allow_raw_pointers.

@SergeySeroshtan
Copy link

Will be nice to implement this feature!

@AlexPerrot
Copy link
Contributor Author

AlexPerrot commented Mar 15, 2017

I have been making tests to implement this feature in the last few days.

The simplest way I have to do this client-side is :

template <typename T, T> struct proxy;

template <typename T, typename R, typename ...Args, R (T::*mf)(Args...)>
struct proxy<R (T::*)(Args...), mf>
{
	using ptr = typename std::add_pointer<typename std::remove_reference<R>::type>::type;
    static ptr call(T & obj, Args &&... args)
    {
        return &((obj.*mf)(std::forward<Args>(args)...));
    }
};

#define RETURN_POINTER(value) &proxy<decltype(value), (value)>::call

This way, you can do class_(...).function("myFunc",RETURN_POINTER(&myClass::myFunc), allow_raw_pointers());. You can also add another specialization for free functions.

I have tried several ways of adding support for reference in embind, but every time, it breaks something existing. For now, references are treated as regular objects, so they are sometimes copied and sometimes not. There also seem to be tests in test_embind that rely on references being copied, which is not what they are supposed to do in C++. The problems I encountered concern mainly (but not exlusively) val, string, smart pointers and inheritance.

If it is not possible to transparent support for references, at least having a policy similar to allow_raw_pointers() would be nice. However, this would require extensive changes to embind. Namely, being able to specify independently (not from the client side, but internally based on the policy) for each return value and argument its real type and the desired binding type.
I was able to implement something like this, but it was tedious and not very readable (yes, even less than the current embind implementation)...

Edit : I opened PR #5037 to show my progress.

@isc30
Copy link

isc30 commented Sep 1, 2018

what if the implementation is specialized for std::reference_wrapper and gets unwrapped when creating the JS object? I don't see the issue of forcing people to return std::reference_wrapper when they want the actual reference. If restricting it to non-trivial types is needed, it's as simple as adding a trait that prevents those to be returned as references.

The internal implementation can unwrap those to pointers, that get translated to js objects in the end (exactly the same thing as when you return a pointer from embind functions)

@stale
Copy link

stale bot commented Sep 18, 2019

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

@shrinktofit
Copy link
Contributor

I have been making tests to implement this feature in the last few days.

The simplest way I have to do this client-side is :

template <typename T, T> struct proxy;

template <typename T, typename R, typename ...Args, R (T::*mf)(Args...)>
struct proxy<R (T::*)(Args...), mf>
{
	using ptr = typename std::add_pointer<typename std::remove_reference<R>::type>::type;
    static ptr call(T & obj, Args &&... args)
    {
        return &((obj.*mf)(std::forward<Args>(args)...));
    }
};

#define RETURN_POINTER(value) &proxy<decltype(value), (value)>::call

This way, you can do class_(...).function("myFunc",RETURN_POINTER(&myClass::myFunc), allow_raw_pointers());. You can also add another specialization for free functions.

I have tried several ways of adding support for reference in embind, but every time, it breaks something existing. For now, references are treated as regular objects, so they are sometimes copied and sometimes not. There also seem to be tests in test_embind that rely on references being copied, which is not what they are supposed to do in C++. The problems I encountered concern mainly (but not exlusively) val, string, smart pointers and inheritance.

If it is not possible to transparent support for references, at least having a policy similar to allow_raw_pointers() would be nice. However, this would require extensive changes to embind. Namely, being able to specify independently (not from the client side, but internally based on the policy) for each return value and argument its real type and the desired binding type. I was able to implement something like this, but it was tedious and not very readable (yes, even less than the current embind implementation)...

Edit : I opened PR #5037 to show my progress.

For those suffering implicit instantiation of undefined template error, @AlexPerrot 's code can be changed to match "const member function":

template <typename T, T> struct proxy;

template <typename T, typename R, typename ...Args, R (T::*mf)(Args...) const>
struct proxy<R (T::*)(Args...) const, mf>
{
	using ptr = typename std::add_pointer<typename std::remove_reference<R>::type>::type;
    static ptr call(T & obj, Args &&... args)
    {
        return &((obj.*mf)(std::forward<Args>(args)...));
    }
};

#define RETURN_POINTER(value) &proxy<decltype(value), (value)>::call

@brendandahl
Copy link
Collaborator

FWIW, I did some work on references in #17602. I ultimately closed that issue in favor of trying to add a more explicit ownership flags.

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

7 participants