javacpp icon indicating copy to clipboard operation
javacpp copied to clipboard

PointerPointer's ambiguous constructor

Open oneengineer opened this issue 4 years ago • 5 comments

Here is the code There are these two constructors in class PointerPointer:

    public PointerPointer(P ... array) { this(array.length); put(array); }

    public PointerPointer(Pointer p) { super(p); }

When you pass one pointer object to it, it will call the second constructor. If user didn't know the detail of constructor, it could bring difficult bugs to the program.

Here is the program I wrote, the second params is wrong because I want a pointer points to an array.

        LLVMValueRef c_neg1 = LLVMConstInt(LLVMInt32Type(), -1,1);
        PointerPointer<LLVMValueRef> params = new PointerPointer( new LLVMValueRef[] {c_neg1} );
        LLVMBuildCall(builder, fun_exit, params, 1, "");

        PointerPointer<LLVMValueRef> params = new PointerPointer<LLVMValueRef>( c_neg1 );   //wrong

I feel converting a pointer to a pointer pointer is somehow a re-interpret ? Maybe using a static function in PointerPointer and a pointer as parameter, instead of using constructor is a good choice?

oneengineer avatar Jun 28 '20 04:06 oneengineer

There are a lot of small issues like that, it's not easy to map C++ APIs to something usable in Java. Breaking backward compatibility for one small change like that isn't worth it, so this will have to wait for the next major release. If you have any concrete suggestions though, please provide more details or a pull request.

saudet avatar Jun 28 '20 05:06 saudet

BTW, an easier more efficient way to do the right thing without having to create an array is by calling the put() method like this:

PointerPointer params = new PointerPointer(1).put(c_neg1);

saudet avatar Jun 28 '20 11:06 saudet

yeah, this is simpler

oneengineer avatar Jul 01 '20 02:07 oneengineer

Something similar happens with the array constructor for IntPointer, LongPointer, etc.

/cc @treo

saudet avatar Jul 06 '20 02:07 saudet

With IntPointer or LongPointer, it is worse than that: new IntPointer(1) creates a pointer with size for one integer. new IntPointer(1, 2) is equivalent to new IntPointer(new int[]{1, 2]) and creates a pointer with the given array as its content.

An IDE with modern Linting Rules (i.e. IntelliJ IDEA) will see the second type of usage and complain about the extraneous array creation, you can call the constructor with vararg arguments after all. And it does that even if your array has only a single element. So it will tell you that new IntPointer(new int[]{7}) can be replaced with new IntPointer(7) and as I've learned that is very wrong.

So there are two problems here:

  • The IDE rules about what can be considered equivalent.
  • The fact that it isn't obviously visible that this is a problem, because the n > 2 case actually works like that.

The comment is more about documenting this behavior. I see that because it is public API it can't easily be changed.

treo avatar Jul 06 '20 12:07 treo