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

Problem with std::unique_ptr as callback parameter #613

Closed
ValentinaBaranova opened this issue Oct 7, 2022 · 24 comments
Closed

Problem with std::unique_ptr as callback parameter #613

ValentinaBaranova opened this issue Oct 7, 2022 · 24 comments

Comments

@ValentinaBaranova
Copy link

I have problem with creating FunctionPointer for callback which receives std::unique_ptr as argument.
This is my test function:

class Stream {
public:
    using StreamId = std::int32_t;
public:
    virtual ~Stream() = default;
    [[nodiscard]] virtual StreamId getStreamId() const noexcept = 0;
};

void testOnStreamOpenCb(std::function<void(std::unique_ptr<Stream> stream)>) {
    std::cout << "testOnStreamOpenCb";
}

This is my FunctionPointer:

    public static class OnStreamOpenCbCallback extends FunctionPointer {
        // Loader.load() and allocate() are required only when explicitly creating an instance
        static {
            Loader.load();
        }

        protected OnStreamOpenCbCallback() {
            allocate();
        }

        private native void allocate();

//        public native void call(@ByVal @UniquePtr @Cast("Stream*") Pointer stream);
//        public native void call(@ByVal @Cast("std::unique_ptr<Stream>*") Pointer stream);
        public native void call(@MoveUniquePtr @Cast("Stream*") Pointer stream);
    }

    @Documented
    @Retention(RetentionPolicy.RUNTIME)
    @Target({ElementType.METHOD, ElementType.PARAMETER})
    @Cast({"std::unique_ptr", "&&"}) @Adapter("UniquePtrAdapter")
    public @interface MoveUniquePtr {
        String value() default "";
    }

I saw uning MoveUniquePtr in tensorflow preset, but there callback returns unique_ptr, not use it as argument.
Here is my map:

infoMap.put(new Info("std::function<void(std::unique_ptr<Stream>stream)>").pointerTypes("OnStreamOpenCbCallback").define());

If I remove cpp function testOnStreamOpenCb, compilation fails because of testOnStreamOpenCb.
Could you please advice me something?

@saudet
Copy link
Member

saudet commented Oct 7, 2022

It might work if we force a cast on std::unique<...> instead of Stream with something like
@Cast({"", "std::unique_ptr<Stream>"}) @UniquePtr Stream

@ValentinaBaranova
Copy link
Author

I've tried

public native void call(@Cast({"", "std::unique_ptr<Stream>"}) @UniquePtr Stream stream);

but got and errors

error: call to implicitly-deleted copy constructor of 'std::unique_ptr<Stream>'
    JavaCPP_com_arrival_mobility_fleetsim_telemetryd_presets_Telemetryd_00024OnStreamOpenCbCallback_instances[0](arg0);


error: call to implicitly-deleted copy constructor of 'std::unique_ptr<Stream>'
    JavaCPP_com_arrival_mobility_fleetsim_telemetryd_presets_Telemetryd_00024OnStreamOpenCbCallback_instances[1](arg0);

@saudet
Copy link
Member

saudet commented Oct 8, 2022

Ok, maybe something like @Cast({"", "std::unique_ptr<Stream>&&"}) @UniquePtr Stream?

@ValentinaBaranova
Copy link
Author

I've changed code a bit to this:

class MyStream {
public:
    int id;
};

void testUniquePtrMyStreamCallback(std::function<void(std::unique_ptr<MyStream> stream)> callback) {
    std::cout << "testUniquePtrMyStreamCallback";
}

map config:

infoMap.put(new Info("std::function<void(std::unique_ptr<MyStream>stream)>").pointerTypes("UniquePtrMyStreamCallback").define());

And helper:

    public static class UniquePtrMyStreamCallback extends FunctionPointer {
        // Loader.load() and allocate() are required only when explicitly creating an instance
        static {
            Loader.load();
        }

        protected UniquePtrMyStreamCallback() {
            allocate();
        }

        private native void allocate();


//        public native void call(@Cast({"", "std::unique_ptr<MyStream>&&"}) Pointer stream);
//        public native void call(@Cast({"", "std::unique_ptr<MyStream>"}) Pointer stream);
//        public native void call(@Cast({"std::unique_ptr<MyStream>*"}) Pointer stream);
//        public native void call(@Cast({"", "std::unique_ptr<MyStream>&&"}) @MoveUniquePtr com.arrival.mobility.telemetryd.MyStream stream);
//        public native void call(@Cast({"", "std::unique_ptr<MyStream>"}) @MoveUniquePtr com.arrival.mobility.telemetryd.MyStream stream);
//        public native void call(@MoveUniquePtr com.arrival.mobility.telemetryd.MyStream stream);
//        public native void call(@Cast({"", "std::unique_ptr<MyStream>"}) @UniquePtr com.arrival.mobility.telemetryd.MyStream stream);
//        public native void call(@Cast({"", "std::unique_ptr<MyStream>&&"}) com.arrival.mobility.telemetryd.MyStream stream);
        public native void call(@Cast({"", "std::unique_ptr<MyStream>&&"}) @UniquePtr com.arrival.mobility.telemetryd.MyStream stream);
//        public native void call(@ByVal @UniquePtr com.arrival.mobility.telemetryd.MyStream stream);
    }

I've tried all these variants and they failed (

saudet added a commit that referenced this issue Oct 12, 2022
@saudet
Copy link
Member

saudet commented Oct 12, 2022

I've pushed a couple of fixes in commit b411ac6, and with that something like @Cast({"", "std::unique_ptr<Stream>", "std::unique_ptr<Stream>&&"}) @UniquePtr Stream should compile. Whether that works correctly or not is a different question though...

@ValentinaBaranova
Copy link
Author

Unfortunately no, it doesn't compile

                            ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:1381:34: error: no matching constructor for initialization of 'UniquePtrAdapter<MyStream>'
    UniquePtrAdapter< MyStream > adapter0(arg0);
                                 ^        ~~~~
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:617:77: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'MyStream' to 'const UniquePtrAdapter<MyStream>' for 1st argument
template<class T, class D = UNIQUE_PTR_NAMESPACE::default_delete<T> > class UniquePtrAdapter {
                                                                            ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:617:77: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'MyStream' to 'UniquePtrAdapter<MyStream>' for 1st argument
template<class T, class D = UNIQUE_PTR_NAMESPACE::default_delete<T> > class UniquePtrAdapter {
                                                                            ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:623:5: note: candidate constructor not viable: no known conversion from 'MyStream' to 'UniquePtrAdapter<MyStream>::U' (aka 'unique_ptr<MyStream, std::default_delete<MyStream>>') for 1st argument
    UniquePtrAdapter(U&& uniquePtr) : ptr(0), size(0), owner(0), uniquePtr2(UNIQUE_PTR_NAMESPACE::move(uniquePtr)), uniquePtr(uniquePtr2) { }
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:624:5: note: candidate constructor not viable: no known conversion from 'MyStream' to 'const UniquePtrAdapter<MyStream>::U' (aka 'const unique_ptr<MyStream, std::default_delete<MyStream>>') for 1st argument
    UniquePtrAdapter(const U& uniquePtr) : ptr(0), size(0), owner(0), uniquePtr2(U(NULL, D())), uniquePtr((U&)uniquePtr) { }
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:625:5: note: candidate constructor not viable: no known conversion from 'MyStream' to 'UniquePtrAdapter<MyStream>::U &' (aka 'unique_ptr<MyStream, std::default_delete<MyStream>> &') for 1st argument
    UniquePtrAdapter(      U& uniquePtr) : ptr(0), size(0), owner(0), uniquePtr2(U(NULL, D())), uniquePtr(uniquePtr) { }
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:626:5: note: candidate constructor not viable: no known conversion from 'MyStream' to 'const UniquePtrAdapter<MyStream>::U *' (aka 'const unique_ptr<MyStream, std::default_delete<MyStream>> *') for 1st argument
    UniquePtrAdapter(const U* uniquePtr) : ptr(0), size(0), owner(0), uniquePtr2(U(NULL, D())), uniquePtr(*(U*)uniquePtr) { }
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:620:5: note: candidate constructor not viable: requires 3 arguments, but 1 was provided
    UniquePtrAdapter(const T* ptr, size_t size, void* owner) : ptr((T*)ptr), size(size), owner(owner),
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:1488:21: error: no viable conversion from 'UniquePtrAdapter<MyStream>' to 'MyStream'
        (*ptr->ptr)(adapter0);
                    ^~~~~~~~
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/cppbuild/macosx-x86_64/include/test_functions.h:23:7: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'UniquePtrAdapter<MyStream>' to 'const MyStream &' for 1st argument
class MyStream {
      ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/cppbuild/macosx-x86_64/include/test_functions.h:23:7: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'UniquePtrAdapter<MyStream>' to 'MyStream &&' for 1st argument
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:634:5: note: candidate function
    operator typename UNIQUE_PTR_NAMESPACE::remove_const<T>::type*() {
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:642:5: note: candidate function
    operator U&() const { return uniquePtr; }
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:643:5: note: candidate function
    operator U&&() { return UNIQUE_PTR_NAMESPACE::move(uniquePtr); }
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:644:5: note: candidate function
    operator U*() { return &uniquePtr; }
    ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:2186:9: error: no matching function for call to 'testUniquePtrMyStreamCallback'
        testUniquePtrMyStreamCallback(*ptr0);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/cppbuild/macosx-x86_64/include/test_functions.h:28:6: note: candidate function not viable: no known conversion from 'JavaCPP_com_arrival_mobility_telemetryd_helper_TelemetrydHelper_00024UniquePtrMyStreamCallback' to 'std::function<void (std::unique_ptr<MyStream>)>' for 1st argument
void testUniquePtrMyStreamCallback(std::function<void(std::unique_ptr<MyStream> stream)> callback) {
     ^
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:631:59: error: call to implicitly-deleted copy constructor of 'UniquePtrAdapter<MyStream>::U' (aka 'unique_ptr<MyStream, std::default_delete<MyStream>>')
        this->uniquePtr = owner != NULL && owner != ptr ? *(U*)owner : U((T*)ptr);
                                                          ^~~~~~~~~~
/Users/valentina.baranova/mobility/fleetSim/telemetryd-javacpp-library/target/native/com/arrival/mobility/telemetryd/macosx-x86_64/jniTelemetryd.cpp:1427:14: note: in instantiation of member function 'UniquePtrAdapter<MyStream>::assign' requested here
    adapter0.assign(rptr0, rsize0, rowner0);
             ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:213:3: note: copy constructor is implicitly deleted because 'unique_ptr<MyStream>' has a user-declared move constructor
  unique_ptr(unique_ptr&& __u) _NOEXCEPT
  ^
1 warning and 4 errors generated.

@ValentinaBaranova
Copy link
Author

Also when I try to use javacpp 1.5.8-SNAPSHOT (from repository) I've got errors even for worked code with other callbacks. If I try to compile it locally, I have failed test ThreadTest.testJNIThreadAttachFromNativeCallbacks

@saudet
Copy link
Member

saudet commented Oct 12, 2022

Yeah, well, you'll need to give me something to work with. Everything works fine here, I can't fix what isn't broken!

@ValentinaBaranova
Copy link
Author

ValentinaBaranova commented Oct 12, 2022

test-javacpp-library.tar.gz
here is my source, could you take a look?
if I use javacpp 1.5.8-SNAPSHOT, I have problems with previously worked callbacks, if I use 1.5.7 - only with unique_ptr

@saudet
Copy link
Member

saudet commented Oct 12, 2022

mvn clean package gives me the exact same error with either JavaCPP 1.5.7 or 1.5.8-SNAPSHOT:

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /tmp/test-javacpp-library/src/gen/java/com/arrival/mobility/telemetryd/BooleanCallback.java:[22,57] cannot find symbol
  symbol:   class boolsuccess
  location: class com.arrival.mobility.telemetryd.BooleanCallback
[ERROR] /tmp/test-javacpp-library/src/gen/java/com/arrival/mobility/telemetryd/BooleanCallback_boolsuccess.java:[18,36] cannot find symbol
  symbol:   class boolsuccess
  location: class com.arrival.mobility.telemetryd.BooleanCallback_boolsuccess
[INFO] 2 errors 

@ValentinaBaranova
Copy link
Author

ValentinaBaranova commented Oct 12, 2022

please remove files from gen/java dir after switching. it doesn't clean automatically

@saudet
Copy link
Member

saudet commented Oct 12, 2022

I get a different error in that case, but still the same regardless of the version of JavaCPP:

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /tmp/test-javacpp-library/src/main/java/com/arrival/mobility/telemetryd/helper/TelemetrydHelper.java:[42,147] cannot find symbol
  symbol:   class MyStream
  location: package com.arrival.mobility.telemetryd
[INFO] 1 error

@ValentinaBaranova
Copy link
Author

How I compile this preset: I go to the main directory, run mvn clean install, then run mvn clean install in platform dir. MyStream class should appear after generation. I use it in TelemetrydHelper which is in helper package so it should not be an compilation error, as you explained me here #606 (comment)

These code compiles fine for me
test-javacpp-library-2.tar.gz
To reproduce unique_ptr error please uncomment lines in test_functions.h and in TelemetrydHelper.

@saudet
Copy link
Member

saudet commented Oct 12, 2022

Ah, I see what the issue is. JavaCPP 1.5.8-SNAPSHOT adds "basic container" support for std::function, but since you're not using that in the project, you need to remove the .define() bits from the presets.

@ValentinaBaranova
Copy link
Author

It works! Thanks a lot!

@ValentinaBaranova
Copy link
Author

test-client-library-3.tar.gz
test-javacpp-library-3.tar.gz

When I try to call this callback with unique_ptr, I got SIGABRT error. Could you please take a look into main class in test-client-library? To make it work you also need 'mvn clean install' in main folder test-javacpp-library and for platform too.

@saudet
Copy link
Member

saudet commented Oct 18, 2022

This should be fixed in commit 81252dd. Please give it a try with the snapshots! Thanks

@ValentinaBaranova
Copy link
Author

Great, it works! Many thanks for quick fix!

@ValentinaBaranova
Copy link
Author

ValentinaBaranova commented Oct 25, 2022

test-client-library-4.tar.gz
test-javacpp-library-4.tar.gz

Found one more issue with unique_ptr. This unique_ptr object (MyStream object) is passed to my callback and than I need store it for later usage. But when I try to use it later, object looks corrupted. This is test class (from test-client-library-4.tar.gz):

public class Main {

    public static void main(String[] args) {
        TestUniquePtrCallback callback = new TestUniquePtrCallback();
        testUniquePtrMyStreamCallback(callback);
        System.out.println("After callback");
        System.out.println("callback.stream.id=" + callback.stream.id());

    }
}
 class TestUniquePtrCallback extends TelemetrydHelper.UniquePtrMyStreamCallback {

    MyStream stream = null;

    @Override
    public void call(MyStream stream) {
        System.out.println("TestUniquePtrCallback: stream.id=" + stream.id());
        this.stream = stream;
        System.out.println("TestUniquePtrCallback: this.stream.id=" + this.stream.id());
    }
}

This is test output:

TestUniquePtrCallback: stream.id=5
TestUniquePtrCallback: this.stream.id=5
After callback
callback.stream.id=-973405760

Could you please help with it?

@saudet
Copy link
Member

saudet commented Oct 25, 2022

To do that, in C++, we need to create a new instance of that std::unique_ptr, but I don't see how we can do that with an adapter, that's not what it was designed to do. We probably need to create a wrapper object for that std::unique_ptr instance.

@saudet
Copy link
Member

saudet commented Oct 26, 2022

That said, I think I managed to make it work in commit 45a6a4b.
Please give it a try with the snapshots: http://bytedeco.org/builds/

@ValentinaBaranova
Copy link
Author

Looks like it works with sample code, will test it later with my library. This is great, thank you very much!

@saudet
Copy link
Member

saudet commented Nov 3, 2022

Fixes for this issue have been released with JavaCPP 1.5.8. Thanks for reporting and for testing all this!

@ValentinaBaranova
Copy link
Author

Amazing! Thanks you for fixing them and for helping me and for such cool javacpp lib! I have completed writing javacpp mappings and now I'm able to use c++ functions from our company library in my kotlin code.

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