Missing ) when using @Adaptor on method returning String.

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Create class method returning string using an @Adaptor annotation.

@Platform(include={"JavaCppExample.h", "MyAdaptor.hpp"})
class Foo extends Pointer
{
      native String @Adaptor("MyAdaptor") foo();
}

2. Run javacpp on class.
3. Compile generated cpp.

What is the expected output? What do you see instead?

Generated cpp compiles with error like:
     [exec] /Users/richard/Asdeq/AndroidPOC/Clients/Android/jni/platform/../../gen/c/javaCpp.cpp: In function '_jstring* Java_com_asdeq_clients_android_javacppexample_MySample_ReturnStringByOptional(JNIEnv*, jobject)':
     [exec] /Users/richard/Asdeq/AndroidPOC/Clients/Android/jni/platform/../../gen/c/javaCpp.cpp:2952:78: error: expected ')' before ';' token

Code generated is like:

    const char* rpointer;
    try {
        MyAdaptor radapter(pointer->foo();   <---- Missing bracket here.
        rpointer = radapter;
        jint rcapacity = (jint)radapter.capacity;
        jlong deallocator = ptr_to_jlong(&(BoostOptionalStringAdaptor::deallocate));

Cause:

doReturnAfter only adds the closing brace for adaptors if the return type is 
assignable to Pointer. 
doReturnBefore allows adaptors to be applied to any non-primitive type 
(including string).

WIll attach a patch momentarily so that doReturnAfter matches doReturnBefore.

However, food for thought, shouldn't adaptors be allowed for any Java type so 
that C++ methods that return  unusual types can have adaptors applied that 
convert to primitives. 

FYI I'm wrapping a class that returns boost::optional<std::string>. I've used 
an adaptor to adapt from boost::optional<string> to String as follows:

#include <boost/optional.hpp>
#include <string>

class BoostOptionalStringAdaptor
{
    public:
        BoostOptionalStringAdaptor(boost::optional<std::string> value)
        {
            this->value = value;
            this->capacity = 1;
        }

        operator const char*()
        {
            // JavaCpp will pass the returned char* to the jni fuction NewStringUTF
            // This will copy the string before the adaptor is destroyed so returning the c_str pointer is ok.
            return value ? value.get().c_str() : NULL;
        }

        static void deallocate(const char* address)
        {
            // Nothing to deallocate
        }

        int capacity;

    private:
        boost::optional<std::string> value;
};

What version of the product are you using? On what operating system?
javacpp-0.1 on OSX.

Please provide any additional information below.

Will attach patch momentarily.

Original issue reported on code.google.com by richardc...@gmail.com on 5 Jun 2012 at 8:21

[GoogleCodeExporter]

Patch attached

Original comment by richardc...@gmail.com on 5 Jun 2012 at 8:40

Attachments:

• issue20.patch

[GoogleCodeExporter]

Oops, that's regression from fixing issue #16. Yes, we should be able to use 
the `@Adapter` annotation for all classes... except primitive types obviously, 
had to think about that a bit, but I think the code in the repository is 
correct now. Let me know, thanks!

Original comment by samuel.a...@gmail.com on 9 Jun 2012 at 5:01

• Changed state: Started

[GoogleCodeExporter]

BTW, what kind of unusual C/C++ primitive type are you looking into mapping to 
a Java primitive type that does not work with @Cast? There's C99 `complex` 
types for example, but I have a hard time understanding why anyone would want 
to map that to any of the Java primitive types... ?

Original comment by samuel.a...@gmail.com on 9 Jun 2012 at 5:16

[GoogleCodeExporter]

Thanks, thought it might have been.

Have you considered using a set of automated tests i.e. set of java classes 
mapped to C++ classes with some steps in the build script for compiling java, 
running generator, and compiling C++, and possibly some junit tests that 
execute generated code. Would most likely stop that kind of regression.

Original comment by richardc...@gmail.com on 9 Jun 2012 at 5:28

[GoogleCodeExporter]

Sure, but someone has to spend some time doing them :)

Original comment by samuel.a...@gmail.com on 9 Jun 2012 at 5:37

[GoogleCodeExporter]

I had a couple of thoughts here after I figured out how to create and use an 
adaptor.

1) From memory (not looking at the code at the moment) generally the test on 
whether to use an adaptor only is in else if clauses after primitives. I guess 
what I'm thinking is that if the user explicitly added  an adaptor annotation 
then it should be applied as they must have had a reason.

2) I think the primitive test is on whether the Java side class is a primitive 
(not the C++ side as generator is not using C++ type information) so there may 
be cases where the user want's to map a complex C++ class to a primitive Java 
class.

4) The current treatment of std:string is a bit rough (using @ByRef to tack 
.c_str() on), a std::string adaptor would probably work nicely.

5) It may be possible to build a lot of the other pass-by behaviour by using 
adaptors and this may simplify the generator significantly as the other 
behaviour annotations would just become a mechanism for selecting from a set of 
default adaptors. There might be some overhead but this might be addressed by 
declaring built-in adaptor methods as inline. Good opportunities for template 
classes and template specialisation here.

Just food for though.

Original comment by richardc...@gmail.com on 9 Jun 2012 at 5:47

[GoogleCodeExporter]

That's true, but that's what open source is for. If nicely documented they 
would form a great set of examples for users.

A better question might be, if there were some tests would you use them?

Original comment by richardc...@gmail.com on 9 Jun 2012 at 5:52

[GoogleCodeExporter]

1) But, but, what kind of use case would that look like? I see no reason why 
anyone would want to do that. IMO, it makes more sense to simply ignore it and 
not complicate the `Generator` code further

2) The primitive check is done with `Class.isPrimitive()`, which is most 
certainly based on Java types. Users can still map something like C99's `double 
complex` to `DoubleComplexPointer`, if that's what they want to do. Personally, 
I'd rather use it as a `DoubleBuffer`, but whatever, it's possible. That's what 
I do for `BoolPointer`, `CLongPointer`, and `SizeTPointer`, which is required 
in this case because the sizes of these types can vary at runtime.

3) ?

4) Yes, I hacked in `@ByRef String` before I made `@Adapter`, but it's handy 
and it's OK I think :) I mean, it's not any worse than something like 
`@Adapter("StringAdapter") String` IMO. We could add that too if you want :)

5) Are you suggesting moving stuff out of Java and into C++? That means we 
would depend on whatever the native C++ compilers feel like supporting, and 
that's never a good idea IMO. Not sure what it would gain us either...

Sure of course I would be happy to have some tests! It's fully orthogonal to 
the main code base. I would just need to add them into their own test directory 
and let Maven or whatever run them when it wants, as long as they don't 
complain too much :) And sure enough, it's good documentation. That's needed too

Original comment by samuel.a...@gmail.com on 9 Jun 2012 at 8:46

[GoogleCodeExporter]

Verified that adaptor is working for String return value at [bb85844].

Original comment by richardc...@gmail.com on 2 Jul 2012 at 12:57

[GoogleCodeExporter]

Great, thanks! It's now part of JavaCPP 0.2.

Let me know if you feel like working on virtual functions or anything that 
seems useful. Nothing's set in concrete, but I do want to keep the design as 
consistent as possible, so please keep up informed of your progress, thanks!

Original comment by samuel.a...@gmail.com on 22 Jul 2012 at 2:19

• Changed state: Fixed

[GoogleCodeExporter]

Samuel

thanks, will try to upgrade to 0.2 this week.

Perhaps unfortunately, I'm moving on to another job at the beginning of August 
and won't be dealing with Java/C++ interoperation so I'm not likely to be in a 
position to contribute further.

Thanks for producing a great library and good luck in the future.

Original comment by richardc...@gmail.com on 22 Jul 2012 at 5:39

[GoogleCodeExporter]

BTW, I figured out how to use adapters more efficiently. We can now use 
annotations like @StdString and @StdVector, while having the template types 
inferred from the Java types! Check out this update:
http://code.google.com/p/javacpp/source/detail?r=6a57e944b4b66923bc1e91f9779bed1
55473ff17

I've also realized that C++ functors could be and should be implemented along 
with the support for FunctionPointer, because we need to define new types, not 
subclass existing ones like for virtual functions. And since we have to define 
new types for FunctionPointer anyway, we could reuse those types for functors 
as well it seems...

So, thanks again for your ideas!

Original comment by samuel.a...@gmail.com on 11 Aug 2012 at 2:30