bytedeco / javacpp

The missing bridge between Java and native C++
Other
4.5k stars 586 forks source link

Bindings using UniquePtrAdapter fail to compile with GCC 7.4 #312

Closed GeekOffTheStreet closed 5 years ago

GeekOffTheStreet commented 5 years ago

When using the std:: namespace for unique_ptr, bindings for unique_ptr return values fail to compile on Ubuntu Bionic Beaver using gcc 7.4.

Representative error is below:

jniConfig.cpp: In member function 'virtual std::unique_ptr<config::IConfig> JavaCPP__0003a_0003aconfig_0003a_0003aIConfigService::createConfig(const char*)':
java/build//jniConfig.cpp:1049:95: error: conversion from 'UniquePtrAdapter<config::IConfig>' to 'std::unique_ptr<config::IConfig>' is ambiguous
     return UniquePtrAdapter< config::IConfig >((config::IConfig*)rptr, rsize, rowner);
                                                                                               ^
/java/build//jniConfig.cpp:535:5: note: candidate: UniquePtrAdapter<T>::operator UniquePtrAdapter<T>::U&&() [with T = config::IConfig; UniquePtrAdapter<T>::U = std::unique_ptr<config::IConfig>]
     operator U&&() { return UNIQUE_PTR_NAMESPACE::move(uniquePtr); }
     ^~~~~~~~
/java/build//jniConfig.cpp:534:5: note: candidate: UniquePtrAdapter<T>::operator UniquePtrAdapter<T>::U&() [with T = config::IConfig; UniquePtrAdapter<T>::U = std::unique_ptr<config::IConfig>]
     operator U&() { return uniquePtr; }
     ^~~~~~~~
Exception in thread "main" java.lang.RuntimeException: Process exited with an error: 1
    at org.bytedeco.javacpp.tools.Builder.generateAndCompile(Builder.java:644)
    at org.bytedeco.javacpp.tools.Builder.build(Builder.java:1095)
    at org.bytedeco.javacpp.tools.Builder.main(Builder.java:1315)

Updating Generator.java to remove operator U&() { return uniquePtr; } resolves the compilation issue.

saudet commented 5 years ago

That just means we need additional casting for this to work, for example: @Cast({"config::IConfig*", "std::unique_ptr<config::IConfig>"}) @UniquePtr seems to be what your function needs.

It's also possible to create a new annotation like @MoveUniquePtr: https://github.com/bytedeco/javacpp-presets/blob/master/tensorflow/src/main/java/org/bytedeco/tensorflow/presets/tensorflow.java#L964

GeekOffTheStreet commented 5 years ago

It appears the cast already gets structured as you suggested:

@Virtual(true) public native @Cast({"config::IConfig*", "std::unique_ptr<config::IConfig>"}) @UniquePtr IConfigInternal createConfig(String name);

GeekOffTheStreet commented 5 years ago

If I try using @MoveUniquePtr, via:

infoMap.put(new Info("std::unique_ptr<config::IConfig>").valueTypes("@MoveUniquePtr IConfigInternal").pointerTypes("@UniquePtr IConfigInternal"));

I get the following error:

/java/build/jniConfig.cpp:612:57: error: conflicting return type specified for 'virtual std::unique_ptr<config::IConfig>&& JavaCPP__0003a_0003aconfig_0003a_0003aIConfigService::createConfig(const char*)'
     virtual std::unique_ptr< ::config::IConfig >&& createConfig(const char* arg0);
                                                         ^~~~~~~~~~~~
In file included /java/build/jniConfig.cpp:99:0:
/config/IConfigService.h:42:38: error:   overriding 'virtual std::unique_ptr<config::IConfig> config::IConfigService::createConfig(const char*)'
     virtual std::unique_ptr<IConfig> createConfig(const char* name) = 0;
                                      ^~~~~~~~~~~~

Looking at the annotation definition:

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

If I change @Cast({"std::unique_ptr", "&&"}) to: @Cast({"std::unique_ptr"}), I'm back to the original error. If I change it to: @Cast({"std::unique_ptr", "&"}) I get:

jniConfig.cpp:612:56: error: conflicting return type specified for 'virtual std::unique_ptr<config::IConfig>& JavaCPP__0003a_0003aconfig_0003a_0003aIConfigService::createConfig(const char*)'
     virtual std::unique_ptr< ::config::IConfig >& createConfig(const char* arg0);
                                                        ^~~~~~~~~~~~
In file included from jniConfig.cpp:99:0:
/config/IConfigService.h:42:38: error:   overriding 'virtual std::unique_ptr<config::IConfig> config::IConfigService::createConfig(const char*)'
     virtual std::unique_ptr<IConfig> createConfig(const char* name) = 0;
                                      ^~~~~~~~~~~~
/jniConfig.cpp: In function '_jobject* Java_config_Config_00024IConfigService_createConfig(JNIEnv*, jobject, jstring)':
/jniConfig.cpp:1064:286: error: use of deleted function 'std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp, _Dp>&) [with _Tp = config::IConfig; _Dp = std::default_delete<config::IConfig>]'
 dapter< ::config::IConfig > radapter((dynamic_cast<JavaCPP__0003a_0003aconfig_0003a_0003aIConfigService*>(ptr) != NULL ? ((JavaCPP__0003a_0003aconfig_0003a_0003aIConfigService*)ptr)->super_createConfig(ptr0) : ptr->createConfig(ptr0)));
                                                                                                                                                                                                                                                                            ^
In file included from /usr/include/c++/7/memory:80:0,
                 from /jniConfig.cpp:76:
/usr/include/c++/7/bits/unique_ptr.h:388:7: note: declared here
       unique_ptr(const unique_ptr&) = delete;
       ^~~~~~~~~~
Exception in thread "main" java.lang.RuntimeException: Process exited with an error: 1
    at org.bytedeco.javacpp.tools.Builder.generateAndCompile(Builder.java:644)
    at org.bytedeco.javacpp.tools.Builder.build(Builder.java:1095)
    at org.bytedeco.javacpp.tools.Builder.main(Builder.java:1315)
saudet commented 5 years ago

Well, what we can use is going to depend on what the types and declarations look like. Do you have an example that fails?

GeekOffTheStreet commented 5 years ago

example2.zip

Attaching an example of the original issue that doesn't use the custom annotation. Run build.sh in root directory.

saudet commented 5 years ago

So you need to override it in Java? This gets complicated. Basically we need to "move" it out (cast it to &&) but then the function signature doesn't match and we can't override it. We'd need to be able to use a third element of @Cast or something...

GeekOffTheStreet commented 5 years ago

Yes, I need to override it. I also need to be able to interact with instances of the interface returned from C++. Why does patching UniquePtrAdapter appear to work?

saudet commented 5 years ago

Patching? How did you modify it?

GeekOffTheStreet commented 5 years ago

I commented out operator U&() { return uniquePtr; } in Generator.java.

saudet commented 5 years ago

Ah, ok, it basically becomes another adapter. We can't use it to pass std::unique_ptr by reference. You could use a copy of that in your own code though.

GeekOffTheStreet commented 5 years ago

Is passing std::unique_ptr by reference something you've encountered in many libraries? Seems like in well-designed APIs unique_ptr would only show up in function signatures for ownership transfer in which case, passing them around as reference would imply the pointers are being re-seated, which would be more clearly expressed through return-by-value.

Definitely should be supported, but I would think it would be rarer of the two usage models.

saudet commented 5 years ago

There are a couple of examples of that here: https://github.com/mgbellemare/Arcade-Learning-Environment/blob/master/src/ale_interface.hpp#L190-L193 I'm not seeing std::unique_ptr used much at all in general though.

saudet commented 5 years ago

Fix included in JavaCPP 1.5.2! Thanks for the contribution.