bytedeco / javacpp

The missing bridge between Java and native C++
Other
4.43k stars 576 forks source link

Template class with template functions #729

Open benshiffman opened 7 months ago

benshiffman commented 7 months ago

I have a rather complicated class template (using C++20 features like concepts, etc) that I have been largely successful in mapping with a few different explicit types. The issue I'm running into now is that I'm unable to map templated functions of this class.

I'd appreciate some guidance on infoMapping templated constructors, operator overloads, and other functions, assuming any of these are possible.

template <typename T>
class DLLExport Array
{
public:

    //constructors
    Array(const std::vector<size_t>& dimv);
    Array(const std::vector<size_t>& dimv, const T& val);
    Array(const std::vector<size_t>& dimv, const std::vector<T>& vals);

    Array(const Array<T>& vals);
    template <typename U> Array(const Array<U>& vals);

    Array(const T& val); 
    template <typename U> Array(const U& val);

    Array(const T* vals, const size_t& n);
    template <typename U> Array(const U* vals, const size_t& n);

    Array(Array<T>&& vals); 

    //assignment operator
    Array<T>& operator=(const Array<T>& rhs);
    template <typename U> Array<T>& operator=(const Array<U>& rhs); // requires (!std::same_as<T,U>);

    Array<T>& operator=(Array<T>&& rhs);

    template <typename U> Array<T>& operator=(const std::vector<U>& rhs);

    Array<T>& operator=(const T& rhs); 
    template <typename U> Array<T>& operator=(const U& rhs); // requires (!std::same_as<U,T>);

    //other functions have this generic format
    template <typename F> Array<F> fun(const Array<F>& in); 
    ...
}

My logic given past experience with infoMapping as well as reviewing examples in the presets repo tells me that these should work fine, but no extra code is generated:

infoMap
    .put(new Info("Array<double>::operator =<int>(const std::vector<int>& rhs)").javaNames("putVectorDouble"))
    .put(new Info("Array<double>::Array<int>(const Array<int>& vals)")
          .javaText("public ArrayDouble(@Const @ByRef ArrayInt vals) { super((Pointer)null); allocate(vals); }" +
                        "private native void allocate(@Const @ByRef ArrayInt vals);"))
    .put(new Info("Array<double>::fun<int>").javaNames("funInt"));
;

Do you see an area I'm screwing up here or is this simply not a capability of javacpp?

A side question: are destructors for pointer types automatically called by the java garbage collector? I understand how to explicitly use Pointer.deallocate() to free the memory before stuff goes out of scope because the gc isn't invoked for that situation.

Thank you!

saudet commented 7 months ago

You'll need to at least remove argument names like rhs and vals to get a match: https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#specifying-names-to-use-in-java

benshiffman commented 7 months ago

Unfortunately these aren't mapping either although I believe they satisfy those requirements.

                infoMap
                        .put(new Info(
                                "Array<double>::operator =<int>(const Array<int>&)").javaNames(
                                "conversionAssignmentInt"))
                        .put(new Info(
                                "Array<double>::Array<int>(const Array<int>&)")
                                .javaText(
                                        "public ArrayDouble(@Const @ByRef ArrayInt vals) { super((Pointer)null); allocate(vals); }" +
                                        "private native void allocate(@Const @ByRef ArrayInt vals);"))
                ;
saudet commented 7 months ago

Since U isn't defined anywhere, it's probably going to match with either Array or Array<U> and similarly for operator =

benshiffman commented 7 months ago

This is quite bizarre. Nontemplated classes with templated functions are fine ("ArrayDoubleNonTemplated::fun") and so are templated classes with nontemplated functions ("Array::funIntNonTemplated") but as soon as both are templated the parser seems to not make the match. Like you said there's not a definition of U which I believe would only really be possible with function template instantiation. I'd be interested in continuing to dig into this issue if you have a hypothesis of what component of the parser is ignoring these mappings.

Edit:

Normal functions will parse correctly WITHOUT specifying argument types but I still run into a linker error which I assume is resolvable with function template instantiation.

infoMap.put(new Info("Array<double>::fun<int>").javaNames("fillInt"));

I think I've encountered a bug/limitation though when trying to map constructors without specifying arguments. This will cause only one constructor to be created with the javaText provided (understandable and expected) BUT will cause all areas where "ArrayInt" previously appeared in the generated ArrayDouble class to be replaced by "Array::Array".

infoMap.put(new Info("Array<double>::Array<int>").javaText(
    "public ArrayDouble(@Const @ByRef ArrayInt vals) { super((Pointer)null); allocate(vals); }" +
    "private native void allocate(@Const @ByRef ArrayInt vals);"))
saudet commented 7 months ago

Yeah, the matching algorithm it's pretty ad hoc. We can see what it's trying to match by putting a print statement or something around this line: https://github.com/bytedeco/javacpp/blob/1.5.9/src/main/java/org/bytedeco/javacpp/tools/Parser.java#L2307

HGuillemet commented 7 months ago

The parser just fails to perform the template substitution correctly sometimes. Try something like:

           new Info("Array<double>::fun<int>(const Array<F>&)").javaNames("fun")
HGuillemet commented 7 months ago

For constructor:

new Info("Array<double>::Array<int>(const Array<U>&)").javaNames("Array")

Be sure to include javaNames even if it doesn't make much sense (or maybe javaText) or the template won't be instantiated.

HGuillemet commented 7 months ago

A side question: are destructors for pointer types automatically called by the java garbage collector? I understand how to explicitly use Pointer.deallocate() to free the memory before stuff goes out of scope because the gc isn't invoked for that situation.

Yes they are called, but whenever the GC feels like. Also try to use PointerScope, that's the best way to avoid memory leaks and have predictable memory usage:

try (PointerScope ignored  = new PointerScope()) {
  /* All pointers allocated here will be deallocated when we exit the block */
}
benshiffman commented 7 months ago

Removing the parameter names and keeping the template placeholder allowed both the plain functions and constructors to be parsed and mapped correctly, save for the std::complex version of the Array class not receiving a generated map of the plain function (maybe something to do with the bracket nesting but I've accounted for the correct closing bracket spacing). Operator overloading with the same technique is still not generating everything.

Be sure to include javaNames even if it doesn't make much sense (or maybe javaText) or the template won't be instantiated.

I actually wasn't aware that there was supposed to be automatic instantiation since it's not happening for member functions nor constructors.

HGuillemet commented 7 months ago

save for the std::complex version of the Array class

You must first map std::complex to some Java type. There is no predefined container in the parser for Complex, so you must map it to FloatPointer, or a pair of float:

.put(new Info("std::complex<float>").cast().pointerTypes("FloatPointer", "float..."))
.put(new Info("Array<std::complex<float> >").pointerTypes("ArrayComplex"))
.put(new Info("Array<double>::fun<std::complex<float> >").javaNames("fun"))
.put(new Info("Array<double>::Array<std::complex<float> >(const Array<U>&)"))

If you don't specify the argument of the constructor, there is a confusion with the Array class.

Operator overloading with the same technique is still not generating everything.

I don't know why. Maybe some parsing error around =< if you write operator =<Array<int> >. You can try:

.put(new Info("Array<double>::operator =(const Array<U>&)").javaText("...")

and put multiple lines in the text (separated by \n).

benshiffman commented 7 months ago

Whoops, I should have specified that I created my own ComplexDoublePointer class (which is incredibly helpful and I'm considering pushing it to this repo if there's any interest). The complex double version of the array is working as expected, but ArrayComplex just doesn't get ANY versions of "fun". Other versions of the Array class are in fact receiving a parsed version of fun() for std::complex as well as all other types.

I discovered the linker issues with the template instantiation (on constructors and regular functions) are on my end and was able to fix them by DLLExporting the explicit instantiations.

My conclusion on operators is that templated operators inside templated functions are simply not something the parser can handle right now.

benshiffman commented 7 months ago

@saudet @HGuillemet I thought I'd give an update in hopes that some nominal efforts here might point people with much better knowledge of the Parser code to a potentially easy fix to this templating business.

I put together a simple test library (windows only) that has one class Template. I instantiate it for both double and std::complex<double> types. TemplatedClass.zip

TemplatedClass.h:

#pragma once

#include <type_traits>
#include <concepts>
#include <iostream>
#include <complex>

#ifdef _BUILD_DLL_
#define DLLImport __declspec( dllimport )
#define DLLExport __declspec( dllexport )
#else
#define DLLImport 
#define DLLExport 
#endif

template <typename T>
class DLLExport TemplatedClass
{
public:

    TemplatedClass() = delete;
    TemplatedClass(const std::string& str);

    std::string getString();

    template <typename U> void funTemp(const U& val);

    template <typename U> TemplatedClass<T>& operator=(const TemplatedClass<U>& rhs);

    template <typename S> friend class TemplatedClass;

private:
    std::string str_val;
};

#ifdef _BUILD_DLL_
#include "TemplatedClass.hpp"
#endif

templatedclass.java:

package org.bytedeco.templatedclass.presets;

import org.bytedeco.javacpp.*;
import org.bytedeco.javacpp.annotation.*;
import org.bytedeco.javacpp.tools.*;

@Properties(
        value = @Platform(
                compiler = {"cpp20"},
                include = {
                        "TemplatedClass.h",
                },
                link = {
                        "TemplatedClass"
                },
                define = {"_CRT_SECURE_NO_WARNINGS"}
        ),
        target = "org.bytedeco.templatedclass",
        global = "org.bytedeco.templatedclass.global.templatedclass"
)
public class templatedclass implements InfoMapper
{
    static
    {
        Loader.checkVersion("org.bytedeco", "templatedclass");
    }

    public void map(InfoMap infoMap)
    {
        // ignore the following lines in TemplatedClass.h:
        // #define DLLImport __declspec( dllimport )
        // #define DLLExport __declspec( dllexport )
        infoMap.put(new Info("DLLImport","DLLExport").cppTypes().annotations());

        // explicit class types
        infoMap.put(new Info("TemplatedClass<double>").pointerTypes("TemplatedClassDouble"));
        infoMap.put(new Info("TemplatedClass<std::complex<double> >").pointerTypes("TemplatedClassComplexDouble"));

        // funTemp function
        infoMap.put(new Info("TemplatedClass<double>::funTemp<double>(const U&)").javaNames("funTemp"));
        infoMap.put(new Info("TemplatedClass<double>::funTemp<int>(const U&)").javaNames("funTemp"));
        infoMap.put(new Info("TemplatedClass<std::complex<double> >::funTemp<double>(const U&)").javaNames("funTemp"));
        infoMap.put(new Info("TemplatedClass<std::complex<double> >::funTemp<int>(const U&)").javaNames("funTemp"));

        // conversion assignment operator
        infoMap.put(new Info("TemplatedClass<double>::operator=<std::complex<double> >(const TemplatedClass<U>&)").javaNames("putConvert"));
        infoMap.put(new Info("TemplatedClass<std::complex<double> >::operator=<double>(const TemplatedClass<U>&)").javaNames("putConvert"));
    }
}

Parser.java line 2308 (tag 1.5.9)

System.out.println("INFO: " + fullname);
INFO: TemplatedClass::TemplatedClass()
INFO: TemplatedClass::TemplatedClass(const std::string&)
INFO: TemplatedClass::getString()
INFO: TemplatedClass::funTemp(const U&)
INFO: TemplatedClass::operator =(const TemplatedClass<U>&)
INFO: TemplatedClass<double>::TemplatedClass<double>()
INFO: TemplatedClass<double>::TemplatedClass<double>(const std::string&)
INFO: TemplatedClass<double>::getString()
INFO: TemplatedClass<double>::funTemp(const U&)
INFO: TemplatedClass<double>::funTemp(const double&)
INFO: TemplatedClass<double>::funTemp(const int&)
INFO: TemplatedClass<double>::operator =(const TemplatedClass<U>&)
INFO: TemplatedClass<std::complex<double> >::TemplatedClass<std::complex<double> >()
INFO: TemplatedClass<std::complex<double> >::TemplatedClass<std::complex<double> >(const std::string&)
INFO: TemplatedClass<std::complex<double> >::getString()
INFO: TemplatedClass<std::complex<double> >::funTemp(const U&)
INFO: TemplatedClass<std::complex<double> >::operator =(const TemplatedClass<U>&)

As you can see, the parser is not replacing "U" with the relevant type in some areas. This happens for ALL versions of the conversion assignment operator= as well as the std::complex<double> version of the funTemp() function. This is reflected in the generated java code:

TemplatedClassComplexDouble.java:

// Targeted by JavaCPP version 1.5.9: DO NOT EDIT THIS FILE

package org.bytedeco.templatedclass;

import java.nio.*;
import org.bytedeco.javacpp.*;
import org.bytedeco.javacpp.annotation.*;

import static org.bytedeco.templatedclass.global.templatedclass.*;

@Name("TemplatedClass<std::complex<double> >") @NoOffset @Properties(inherit = org.bytedeco.templatedclass.presets.templatedclass.class)
public class TemplatedClassComplexDouble extends Pointer {
    static { Loader.load(); }
    /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */
    public TemplatedClassComplexDouble(Pointer p) { super(p); }

    public TemplatedClassComplexDouble(@StdString BytePointer str) { super((Pointer)null); allocate(str); }
    private native void allocate(@StdString BytePointer str);
    public TemplatedClassComplexDouble(@StdString String str) { super((Pointer)null); allocate(str); }
    private native void allocate(@StdString String str);

    public native @StdString BytePointer getString();
}

TemplatedClassDouble.java:

// Targeted by JavaCPP version 1.5.9: DO NOT EDIT THIS FILE

package org.bytedeco.templatedclass;

import java.nio.*;
import org.bytedeco.javacpp.*;
import org.bytedeco.javacpp.annotation.*;

import static org.bytedeco.templatedclass.global.templatedclass.*;

// #endif

@Name("TemplatedClass<double>") @NoOffset @Properties(inherit = org.bytedeco.templatedclass.presets.templatedclass.class)
public class TemplatedClassDouble extends Pointer {
    static { Loader.load(); }
    /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */
    public TemplatedClassDouble(Pointer p) { super(p); }

    public TemplatedClassDouble(@StdString BytePointer str) { super((Pointer)null); allocate(str); }
    private native void allocate(@StdString BytePointer str);
    public TemplatedClassDouble(@StdString String str) { super((Pointer)null); allocate(str); }
    private native void allocate(@StdString String str);

    public native @StdString BytePointer getString();

    public native void funTemp(double val);

    public native void funTemp(int val);
}

I'll do my best to keep digging for the source of the issue but the Parser is long and dense. If you have tips or know what's happening here, please let me know!

saudet commented 7 months ago

Well, ideally we should use Clang to parse everything, so unless this is preventing anyone from doing something it's probably not worth looking into. Is this issue preventing you from doing anything?

benshiffman commented 7 months ago

Being able to map function templates which are members of class templates is essential for key functionality of my library (which emulates the Matlab arrays). There is a way I can use javaText() to explicitly map operators and functions but it's cumbersome and as the library scales this becomes less and less practical.

benshiffman commented 7 months ago

@saudet I'm pretty sure I've fixed it. This was a serious test of my patience and debugging abilities but the issue is in InfoMap.normalize. When remaking "name", token.spacing is not considered for '>' tokens that follow other '>' tokens.

Please see #730!