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

Template class with template functions #729

Open
benshiffman opened this issue Dec 6, 2023 · 15 comments
Open

Template class with template functions #729

benshiffman opened this issue Dec 6, 2023 · 15 comments

Comments

@benshiffman
Copy link

benshiffman commented Dec 6, 2023

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
Copy link
Member

saudet commented Dec 7, 2023

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
Copy link
Author

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
Copy link
Member

saudet commented Dec 7, 2023

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

@benshiffman
Copy link
Author

benshiffman commented Dec 7, 2023

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
Copy link
Member

saudet commented Dec 8, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

@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
Copy link
Member

saudet commented Dec 19, 2023

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
Copy link
Author

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
Copy link
Author

@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!

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

3 participants