From f9ce49c977c1f57f0d33e403df485a5d1f82c37b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Mon, 31 Jul 2023 22:42:24 +0200 Subject: [PATCH 01/16] Add of for downcasting when c-style cast cannot work --- .../java/org/bytedeco/javacpp/tools/Info.java | 7 +++ .../org/bytedeco/javacpp/tools/Parser.java | 60 +++++++++++++++---- 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Info.java b/src/main/java/org/bytedeco/javacpp/tools/Info.java index 57341f6f6..a1bf11b94 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Info.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Info.java @@ -61,6 +61,7 @@ public Info(Info i) { skip = i.skip; skipDefaults = i.skipDefaults; purify = i.purify; + polymorphic = i.polymorphic; upcast = i.upcast; virtualize = i.virtualize; base = i.base; @@ -120,6 +121,10 @@ public Info(Info i) { /** Whether a static_cast is needed to upcast a pointer to this cppName. * This is necessary for polymorphic classes that are virtually inherited from. */ boolean upcast = false; + /** Inform the parser that a class is polymorphic, if it cannot figure it out by itself. + * Knowing that a class is polymorphic is needed to generate proper downcast + * methods in case of virtual and multiple inheritance. */ + boolean polymorphic = false; /** Annotates virtual functions with @{@link Virtual} and adds appropriate constructors. */ boolean virtualize = false; /** Allows to override the base class of {@link #pointerTypes}. Defaults to {@link Pointer}. */ @@ -162,6 +167,8 @@ public Info(Info i) { public Info purify(boolean purify) { this.purify = purify; return this; } public Info upcast() { this.upcast = true; return this; } public Info upcast(boolean upcast) { this.upcast = upcast; return this; } + public Info polymorphic() { this.polymorphic = true; return this; } + public Info polymorphic(boolean p) { this.polymorphic = p; return this; } public Info virtualize() { this.virtualize = true; return this; } public Info virtualize(boolean virtualize) { this.virtualize = virtualize; return this; } public Info base(String base) { this.base = base; return this; } diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 60a55894d..67e48a85d 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -97,6 +97,7 @@ public Parser(Logger logger, Properties properties, String encoding, String line TokenIndexer tokens = null; String lineSeparator = null; Set upcasts = new HashSet<>(); + HashSet polymorphicClasses = new HashSet<>(); // Contains Java names static String removeAnnotations(String s) { return s.substring(s.lastIndexOf(' ') + 1); @@ -3360,27 +3361,43 @@ boolean using(Context context, DeclarationList declList) throws ParserException return true; } - String upcast(Type from, Type to, boolean override) { - String ecmn = upcastMethodName(to.javaName); - String res = " " + (override ? "@Override " : "") + "public " + to.javaName + " " + ecmn + "() { return " + ecmn + "(this); }\n" - + " @Namespace public static native "; + String upcast(Type derived, Type base, boolean override) { + String ecmn = upcastMethodName(base.javaName); + String res = " " + (override ? "@Override " : "") + "public " + base.javaName + " " + ecmn + "() { return " + ecmn + "(this); }\n" + + " @Namespace public static native "; /* We generate the adapter-aware version of the method when the target class has any annotation, assuming that this annotation is @SharedPtr or some other adapter storing a share_ptr in owner. This is the only use case for now. We can generalize to any adapter if the need arises and call some cast function on the adapter instead of static_pointer_cast. */ - List split = Templates.splitNamespace(to.cppName); - String constructorName = to.cppName + "::" + Templates.strip(split.get(split.size() - 1)); + List split = Templates.splitNamespace(base.cppName); + String constructorName = base.cppName + "::" + Templates.strip(split.get(split.size() - 1)); Info info = infoMap.getFirst(constructorName); String annotations = ""; if (info != null && info.annotations != null) { for (String s : info.annotations) { annotations += s + " "; } - res += annotations + "@Name(\"SHARED_PTR_NAMESPACE::static_pointer_cast<" + to.cppName + ", " + from.cppName + ">\") "; + res += annotations + "@Name(\"SHARED_PTR_NAMESPACE::static_pointer_cast<" + base.cppName + ", " + derived.cppName + ">\") "; } else { - res += "@Name(\"static_cast<" + to.cppName + "*>\") "; + res += "@Name(\"static_cast<" + base.cppName + "*>\") "; } - return res + to.javaName + " " + ecmn + "(" + annotations + from.javaName + " pointer);\n"; + res += base.javaName + " " + ecmn + "(" + annotations + derived.javaName + " pointer);\n"; + final String downcastType; + if (base.virtual) { + if (polymorphicClasses.contains(base.javaName)) { + downcastType = "dynamic"; + } else { + return res; // No downcast possible in this case + } + } else { + downcastType = "static"; + } + if (annotations.equals("")) { + res += " @Namespace public static native @Name(\"" + downcastType + "_cast<" + derived.cppName + "*>\") " + derived.javaName + " of(" + base.javaName + " pointer);"; + } else { + res += " @Namespace public static native " + annotations + "@Name(\"SHARED_PTR_NAMESPACE::" + downcastType + "_pointer_cast<" + derived.cppName + ", " + base.cppName + ">\") " + derived.javaName + " of(" + annotations + base.javaName + " pointer);"; + } + return res; } boolean group(Context context, DeclarationList declList) throws ParserException { @@ -3423,15 +3440,18 @@ boolean group(Context context, DeclarationList declList) throws ParserException boolean anonymous = !typedef && type.cppName.length() == 0, derivedClass = false, skipBase = false; if (type.cppName.length() > 0 && tokens.get().match(':')) { derivedClass = true; + boolean virtualInheritance = false; + boolean accessible = !ctx.inaccessible; for (Token token = tokens.next(); !token.match(Token.EOF); token = tokens.next()) { - boolean accessible = !ctx.inaccessible; if (token.match(Token.VIRTUAL)) { + virtualInheritance = true; continue; } else if (token.match(Token.PRIVATE, Token.PROTECTED, Token.PUBLIC)) { accessible = token.match(Token.PUBLIC); - tokens.next(); + continue; } Type t = type(context); + t.virtual = virtualInheritance; Info info = infoMap.getFirst(t.cppName); if (info != null && info.skip) { skipBase = true; @@ -3442,6 +3462,8 @@ boolean group(Context context, DeclarationList declList) throws ParserException if (tokens.get().expect(',', '{').match('{')) { break; } + virtualInheritance = false; + accessible = !ctx.inaccessible; } } if (typedef && tokens.get().match('(')) { @@ -3545,10 +3567,21 @@ boolean group(Context context, DeclarationList declList) throws ParserException infoMap.put(info = new Info(type.cppName).pointerTypes(type.javaName)); } Type base = new Type("Pointer"); + boolean polymorphic = info != null && info.polymorphic; Iterator it = baseClasses.iterator(); while (it.hasNext()) { Type next = it.next(); Info nextInfo = infoMap.getFirst(next.cppName); + boolean nextPolymorphic = polymorphicClasses.contains(next.javaName) || + (nextInfo != null && nextInfo.polymorphic); + polymorphic |= nextPolymorphic; + if (nextPolymorphic && next.virtual && !upcasts.contains(next.javaName)) { + // We can detect this only if the superclass is parsed before or + // info.polymorphic is set. + logger.warn(type.cppName + " virtually inherits from polymorphic class " + next.cppName + + ". Consider adding an upcast Info on " + next.cppName + "."); + } + if (nextInfo == null || !nextInfo.flatten) { base = next; it.remove(); @@ -3570,6 +3603,8 @@ boolean group(Context context, DeclarationList declList) throws ParserException if (upcasts.contains(base.javaName)) { casts += upcast(type, base, true); + } else if (polymorphicClasses.contains(base.javaName) && base.virtual) { + casts += upcast(type, base, false); } decl.signature = type.javaName; @@ -3670,6 +3705,7 @@ boolean group(Context context, DeclarationList declList) throws ParserException pointerConstructor = false, abstractClass = info != null && info.purify && !ctx.virtualize, allPureConst = true, haveVariables = false; for (Declaration d : declList2) { + polymorphic |= d.declarator != null && d.declarator.type != null && d.declarator.type.virtual; if (d.declarator != null && d.declarator.type != null && d.declarator.type.using && decl.text != null) { // inheriting constructors defaultConstructor |= d.text.contains("private native void allocate();"); @@ -3796,6 +3832,8 @@ boolean group(Context context, DeclarationList declList) throws ParserException decl.custom = true; } } + if (polymorphic) polymorphicClasses.add(type.javaName); + for (Declaration d : declList2) { if ((!d.inaccessible || d.declarator != null && d.declarator.type.friend) && (d.declarator == null || d.declarator.type == null From 57f3cfca2859533d5486b11029e60286a70f74ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Thu, 17 Aug 2023 18:22:30 +0200 Subject: [PATCH 02/16] Drop polymorphic info --- src/main/java/org/bytedeco/javacpp/tools/Info.java | 7 ------- src/main/java/org/bytedeco/javacpp/tools/Parser.java | 10 +++++----- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Info.java b/src/main/java/org/bytedeco/javacpp/tools/Info.java index a1bf11b94..57341f6f6 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Info.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Info.java @@ -61,7 +61,6 @@ public Info(Info i) { skip = i.skip; skipDefaults = i.skipDefaults; purify = i.purify; - polymorphic = i.polymorphic; upcast = i.upcast; virtualize = i.virtualize; base = i.base; @@ -121,10 +120,6 @@ public Info(Info i) { /** Whether a static_cast is needed to upcast a pointer to this cppName. * This is necessary for polymorphic classes that are virtually inherited from. */ boolean upcast = false; - /** Inform the parser that a class is polymorphic, if it cannot figure it out by itself. - * Knowing that a class is polymorphic is needed to generate proper downcast - * methods in case of virtual and multiple inheritance. */ - boolean polymorphic = false; /** Annotates virtual functions with @{@link Virtual} and adds appropriate constructors. */ boolean virtualize = false; /** Allows to override the base class of {@link #pointerTypes}. Defaults to {@link Pointer}. */ @@ -167,8 +162,6 @@ public Info(Info i) { public Info purify(boolean purify) { this.purify = purify; return this; } public Info upcast() { this.upcast = true; return this; } public Info upcast(boolean upcast) { this.upcast = upcast; return this; } - public Info polymorphic() { this.polymorphic = true; return this; } - public Info polymorphic(boolean p) { this.polymorphic = p; return this; } public Info virtualize() { this.virtualize = true; return this; } public Info virtualize(boolean virtualize) { this.virtualize = virtualize; return this; } public Info base(String base) { this.base = base; return this; } diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 67e48a85d..357ebe1a2 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -3567,17 +3567,17 @@ boolean group(Context context, DeclarationList declList) throws ParserException infoMap.put(info = new Info(type.cppName).pointerTypes(type.javaName)); } Type base = new Type("Pointer"); - boolean polymorphic = info != null && info.polymorphic; + /* The detection of polymorphism can fail if the base class has not been parsed yet, or is not + * parsed at all. We might need to add a "polymorphic" info flag and initialize this variable + * to true when info.polymorphic is set, and check nextInfo.polymorphic in the loop. */ + boolean polymorphic = false; Iterator it = baseClasses.iterator(); while (it.hasNext()) { Type next = it.next(); Info nextInfo = infoMap.getFirst(next.cppName); - boolean nextPolymorphic = polymorphicClasses.contains(next.javaName) || - (nextInfo != null && nextInfo.polymorphic); + boolean nextPolymorphic = polymorphicClasses.contains(next.javaName); polymorphic |= nextPolymorphic; if (nextPolymorphic && next.virtual && !upcasts.contains(next.javaName)) { - // We can detect this only if the superclass is parsed before or - // info.polymorphic is set. logger.warn(type.cppName + " virtually inherits from polymorphic class " + next.cppName + ". Consider adding an upcast Info on " + next.cppName + "."); } From 6e3e0462de31722917f70888c91c6b9040c8f6be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Mon, 28 Aug 2023 09:17:16 +0200 Subject: [PATCH 03/16] Add support for `@Name` on allocators --- .../bytedeco/javacpp/annotation/Adapter.java | 92 +++++++++++++++---- .../org/bytedeco/javacpp/tools/Generator.java | 23 ++--- .../org/bytedeco/javacpp/AdapterTest.java | 3 +- 3 files changed, 90 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java b/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java index 6369e45e3..22e444c47 100644 --- a/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java +++ b/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java @@ -10,22 +10,82 @@ import org.bytedeco.javacpp.tools.Generator; /** - * Specifies a C++ class to act as an adapter to convert the types of arguments. - * Three such C++ classes made available by {@link Generator} are {@code StringAdapter}, - * {@code VectorAdapter}, and {@code SharedPtrAdapter} to bridge a few differences between - * {@code std::string} and {@link String}; between {@code std::vector}, Java arrays of - * primitive types, {@link Buffer}, and {@link Pointer}; and between {@code xyz::shared_ptr} - * and {@link Pointer}. Adapter classes must define the following public members: - *
    - *
  • A constructor accepting 3 arguments (or more if {@link #argc()} > 1): a pointer, a size, and the owner pointer - *
  • Another constructor that accepts a reference to the object of the other class - *
  • A {@code static void deallocate(owner)} function - *
  • Overloaded cast operators to both types, for references and pointers - *
  • A {@code void assign(pointer, size, owner)} function - *
  • A {@code size} member variable for arrays accessed via pointer - *
- * To reduce further the amount of coding, this annotation can also be used on - * other annotations, such as with {@link StdString}, {@link StdVector}, and {@link SharedPtr}. + * Specifies a C++ class to act as an adapter between a target type and one or more adaptee type(s). + * Instances of the adapter class are short-living and last only for the duration of a JNI call. + *

+ * Six such C++ classes are made available by {@link Generator}: + *
+ * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + *
Adapter classTarget typeAdaptee typesHelper annotation
{@code VectorAdapter}{@code std::vector}{@code P}{@link StdVector}
{@code StringAdapter}{@code std::basic_string}{@code char}
{@code signed char}
{@code unsigned char}
{@code wchar_t}
{@code unsigned short}
{@code signed int}		{@link StdString}
{@code SharedPtrAdapter}{@code SHARED_PTR_NAMESPACE::shared_ptr}{@code T}{@link SharedPtr}
{@code UniquePtrAdapter}{@code UNIQUE_PTR_NAMESPACE::unique_ptr}{@code T}{@link UniquePtr}
{@code MoveAdapter}{@code T}{@code T}{@link StdMove}
{@code OptionalAdapter}{@code OPTIONAL_NAMESPACE::optional}{@code T}{@link Optional}
+ *
+ * The helper annotations are shortcuts that infer the template type(s) of the adapter class from the Java + * class they annotate. + *

+ * When an argument of a method is annotated, an instance of the adapter class is created from + * the Java object passed as argument, and this instance is passed to the C++ function, thus triggering + * an implicit cast to the type expected by the function (usually a reference or pointer to the target type). + * If the argument is also annotated with {@link Cast}, + * the adapter instance is cast to the type(s) specified by the {@link Cast} annotation before being passed + * to the function. + *

+ * When a method is annotated, an instance of the adapter is created from the + * value (usually a pointer or reference to the target type) returned by the C++ function or by {@code new} if the method is an allocator. + * If the method is also annotated with {@link Cast}, the value returned by the C++ function is + * cast by value 3 of the {@link Cast} annotation, if any, before instantiation of the adapter. + * Then a Java object is created from the adapter to be returned by the method. + *

+ * Adapter classes must at least define the following public members: + *
    + *
  • For each adaptee type, a constructor accepting 3 arguments (or more if {@link #argc()} > 1): a pointer to a const value of the adaptee, a size, and the owner pointer + *
  • Another constructor that accepts a reference to the target type + *
  • A {@code static void deallocate(owner)} function + *
  • Overloaded cast operators to both the target type and the adaptee types, for references and pointers + *
  • {@code void assign(pointer, size, owner)} functions with the same signature than the constructors accepting 3 arguments + *
  • A {@code size} member variable for arrays accessed via pointer + *
* * @see Generator * diff --git a/src/main/java/org/bytedeco/javacpp/tools/Generator.java b/src/main/java/org/bytedeco/javacpp/tools/Generator.java index 4f4382384..f1ed7b2fe 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Generator.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Generator.java @@ -2645,11 +2645,16 @@ void call(MethodInformation methodInfo, String returnPrefix, boolean secondCall) prefix = ""; suffix = ""; } else { - out.print((noException(methodInfo.cls, methodInfo.method) ? - "new (std::nothrow) " : "new ") + valueTypeName + typeName[1]); - if (methodInfo.arrayAllocator) { - prefix = "["; - suffix = "]"; + if (methodInfo.method.isAnnotationPresent(Name.class)) { + out.print(methodInfo.memberName[0]); + // If method is an array allocator, the function must return a pointer to an array + } else { + out.print((noException(methodInfo.cls, methodInfo.method) ? + "new (std::nothrow) " : "new ") + valueTypeName + typeName[1]); + if (methodInfo.arrayAllocator) { + prefix = "["; + suffix = "]"; + } } } } else if (Modifier.isStatic(methodInfo.modifiers) || !Pointer.class.isAssignableFrom(methodInfo.cls)) { @@ -2814,12 +2819,8 @@ void returnAfter(MethodInformation methodInfo) { // special considerations for std::string without adapter out.print(");\n" + indent + "rptr = rstr.c_str()"); } - if (adapterInfo != null) { - if (methodInfo.allocator || methodInfo.arrayAllocator) { - suffix = ", 1, NULL)" + suffix; - } else if (!methodInfo.returnType.isPrimitive()) { - suffix = ")" + suffix; - } + if ((methodInfo.allocator || methodInfo.arrayAllocator || !methodInfo.returnType.isPrimitive()) && adapterInfo != null) { + suffix = ")" + suffix; } if ((Pointer.class.isAssignableFrom(methodInfo.returnType) || (methodInfo.returnType.isArray() && diff --git a/src/test/java/org/bytedeco/javacpp/AdapterTest.java b/src/test/java/org/bytedeco/javacpp/AdapterTest.java index 7391b754c..f59dd70e8 100644 --- a/src/test/java/org/bytedeco/javacpp/AdapterTest.java +++ b/src/test/java/org/bytedeco/javacpp/AdapterTest.java @@ -27,6 +27,7 @@ import org.bytedeco.javacpp.annotation.Cast; import org.bytedeco.javacpp.annotation.Const; import org.bytedeco.javacpp.annotation.Function; +import org.bytedeco.javacpp.annotation.Name; import org.bytedeco.javacpp.annotation.Optional; import org.bytedeco.javacpp.annotation.Platform; import org.bytedeco.javacpp.annotation.SharedPtr; @@ -82,7 +83,7 @@ public class AdapterTest { static class SharedData extends Pointer { SharedData(Pointer p) { super(p); } SharedData(int data) { allocate(data); } - @SharedPtr native void allocate(int data); + @SharedPtr @Name("std::make_shared") native void allocate(int data); native int data(); native SharedData data(int data); } From 13e894eb53c98bc2bff5a884979e1d578c888f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Tue, 5 Sep 2023 11:28:30 +0200 Subject: [PATCH 04/16] Disable arrays when constructor is annotated --- .../org/bytedeco/javacpp/tools/Parser.java | 49 ++++++++++--------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 33e1d2751..21ff69012 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -3786,21 +3786,22 @@ boolean group(Context context, DeclarationList declList) throws ParserException } } + final boolean addArrayConstructor; if (implicitConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize)) { constructors += " /** Default native constructor. */\n" + " public " + shortName + "() { super((Pointer)null); allocate(); }\n" + - " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + - " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n" + - " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + - " public " + shortName + "(Pointer p) { super(p); }\n" + " " + constructorAnnotations + "private native void allocate();\n" + - " private native void allocateArray(long size);\n" + - " @Override public " + shortName + " position(long position) {\n" + - " return (" + shortName + ")super.position(position);\n" + - " }\n" + - " @Override public " + shortName + " getPointer(long i) {\n" + - " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + - " }\n"; + " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + + " public " + shortName + "(Pointer p) { super(p); }\n"; + + /* Annotations currently used on constructors are @SharedPtr and @Name. @SharedPtr produces + * memory corruption if applied to arrays. And @Name needs special versions of the provided + * C++ function that return arrays. So for safety we disable arrays for classes with + * annotations on constructor. + * position and getPointer are incompatible with @SharedPtr, but compatible with @Name. + * Since we don't distinguish annotations here, we disable them in both cases. + */ + addArrayConstructor = constructorAnnotations.isEmpty(); } else { if ((info == null || !info.purify) && (!abstractClass || ctx.virtualize)) { constructors += inheritedConstructors; @@ -3810,18 +3811,22 @@ boolean group(Context context, DeclarationList declList) throws ParserException constructors += " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + " public " + shortName + "(Pointer p) { super(p); }\n"; } - if (defaultConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize) && !arrayConstructor) { - constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + - " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n" + - " private native void allocateArray(long size);\n" + - " @Override public " + shortName + " position(long position) {\n" + - " return (" + shortName + ")super.position(position);\n" + - " }\n" + - " @Override public " + shortName + " getPointer(long i) {\n" + - " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + - " }\n"; - } + addArrayConstructor = (defaultConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize) + && !arrayConstructor && constructorAnnotations.isEmpty()); } + + if (addArrayConstructor) { + constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + + " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n" + + " private native void allocateArray(long size);\n" + + " @Override public " + shortName + " position(long position) {\n" + + " return (" + shortName + ")super.position(position);\n" + + " }\n" + + " @Override public " + shortName + " getPointer(long i) {\n" + + " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + + " }\n"; + } + if (info == null || !info.skipDefaults) { decl.text += constructors; } From 1a52e89ea208c466392763a5559fcfb48dd6979f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Tue, 5 Sep 2023 17:36:29 +0200 Subject: [PATCH 05/16] Use constructors for downcasting. --- .../org/bytedeco/javacpp/tools/Parser.java | 124 +++++++++++++----- .../java/org/bytedeco/javacpp/tools/Type.java | 4 + 2 files changed, 93 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 21ff69012..f0e652a78 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -32,6 +32,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedList; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -96,9 +97,18 @@ public Parser(Logger logger, Properties properties, String encoding, String line InfoMap leafInfoMap = null; TokenIndexer tokens = null; String lineSeparator = null; - Set upcasts = new HashSet<>(); + Set upcasts = new HashSet<>(); // Java names of base classes needing upcast + Map> downcasts = new HashMap<>(); // Keys: classes which child classes needs downcast constructor. Values: classes to downcast from. HashSet polymorphicClasses = new HashSet<>(); // Contains Java names + private void addDowncast(String base, Type from) { + Set types = downcasts.get(base); + if (types == null) { + downcasts.put(base, types = new HashSet<>(1)); + } + types.add(from); + } + static String removeAnnotations(String s) { return s.substring(s.lastIndexOf(' ') + 1); } @@ -112,6 +122,16 @@ static String upcastMethodName(String javaName) { return "as" + Character.toUpperCase(shortName.charAt(0)) + shortName.substring(1); } + /** Returns the name of the constructor of class cppName, to be used as keys in infoMap */ + static String constructorName(String cppName) { + String constructorName = Templates.strip(cppName); + int namespace = constructorName.lastIndexOf("::"); + if (namespace >= 0) { + constructorName = constructorName.substring(namespace + 2); + } + return cppName + "::" + constructorName; + } + String translate(String text) { Info info = infoMap.getFirst(text); if (info != null && info.javaNames != null && info.javaNames.length > 0) { @@ -254,7 +274,7 @@ void containers(Context context, DeclarationList declList) throws ParserExceptio dcl.definition.text = "\n" + dcl.definition.text; decl.declarator = dcl; } - LinkedHashSet typeSet = new LinkedHashSet<>(); + LinkedList typeSet = new LinkedList<>(); typeSet.addAll(Arrays.asList(firstType, secondType, indexType, valueType)); typeSet.addAll(Arrays.asList(containerType.arguments)); for (Type type : typeSet) { @@ -3361,45 +3381,60 @@ boolean using(Context context, DeclarationList declList) throws ParserException return true; } - String upcast(Type derived, Type base, boolean override) { - String ecmn = upcastMethodName(base.javaName); - String res = " " + (override ? "@Override " : "") + "public " + base.javaName + " " + ecmn + "() { return " + ecmn + "(this); }\n" - + " @Namespace public static native "; - /* We generate the adapter-aware version of the method when the target class has any annotation, - assuming that this annotation is @SharedPtr or some other adapter storing a share_ptr in owner. - This is the only use case for now. We can generalize to any adapter if the need arises and call - some cast function on the adapter instead of static_pointer_cast. */ - List split = Templates.splitNamespace(base.cppName); - String constructorName = base.cppName + "::" + Templates.strip(split.get(split.size() - 1)); - Info info = infoMap.getFirst(constructorName); - String annotations = ""; - if (info != null && info.annotations != null) { - for (String s : info.annotations) { - annotations += s + " "; - } - res += annotations + "@Name(\"SHARED_PTR_NAMESPACE::static_pointer_cast<" + base.cppName + ", " + derived.cppName + ">\") "; - } else { - res += "@Name(\"static_cast<" + base.cppName + "*>\") "; - } - res += base.javaName + " " + ecmn + "(" + annotations + derived.javaName + " pointer);\n"; + String downcast(Type derived, Type base) { final String downcastType; if (base.virtual) { if (polymorphicClasses.contains(base.javaName)) { downcastType = "dynamic"; } else { - return res; // No downcast possible in this case + return ""; // No downcast possible in this case } } else { downcastType = "static"; } - if (annotations.equals("")) { - res += " @Namespace public static native @Name(\"" + downcastType + "_cast<" + derived.cppName + "*>\") " + derived.javaName + " of(" + base.javaName + " pointer);"; + /* See upcast() about annotations. */ + String annotations = ""; + Info constructorInfo = infoMap.getFirst(constructorName(base.cppName)); + if (constructorInfo != null && constructorInfo.annotations != null) { + for (String s : constructorInfo.annotations) { + if (!s.startsWith("@Name")) annotations += s + " "; + } + } + String res = ""; + res += " /** Downcast constructor. */\n" + + " public " + derived.javaName + "(" + base.javaName + " pointer) { super((Pointer)null); allocate(pointer); }\n"; + if (annotations.isEmpty()) { + res += " @Namespace private native @Name(\"" + downcastType + "_cast<" + derived.cppName + "*>\") void allocate(" + base.javaName + " pointer);"; } else { - res += " @Namespace public static native " + annotations + "@Name(\"SHARED_PTR_NAMESPACE::" + downcastType + "_pointer_cast<" + derived.cppName + ", " + base.cppName + ">\") " + derived.javaName + " of(" + annotations + base.javaName + " pointer);"; + res += " @Namespace private native " + annotations + "@Name(\"SHARED_PTR_NAMESPACE::" + downcastType + "_pointer_cast<" + derived.cppName + ", " + base.cppName + ">\") void allocate(" + annotations + base.javaName + " pointer);"; } return res; } + String upcast(Type derived, Type base, boolean override) { + String ecmn = upcastMethodName(base.javaName); + String res = " " + (override ? "@Override " : "") + "public " + base.javaName + " " + ecmn + "() { return " + ecmn + "(this); }\n" + + " @Namespace public static native "; + /* We generate the adapter-aware version of the method when the base class has any annotation + that is not @Name, assuming that this annotation is @SharedPtr or some other adapter storing a + shared_ptr in owner. This is the only use case for now. We can generalize to any adapter if the + need arises and call some cast function on the adapter instead of static_pointer_cast. */ + String annotations = ""; + Info constructorInfo = infoMap.getFirst(constructorName(base.cppName)); + if (constructorInfo != null && constructorInfo.annotations != null) { + for (String s : constructorInfo.annotations) { + if (!s.startsWith("@Name")) annotations += s + " "; + } + } + if (annotations.isEmpty()) { + res += "@Name(\"static_cast<" + base.cppName + "*>\") "; + } else { + res += annotations + "@Name(\"SHARED_PTR_NAMESPACE::static_pointer_cast<" + base.cppName + ", " + derived.cppName + ">\") "; + } + res += base.javaName + " " + ecmn + "(" + annotations + derived.javaName + " pointer);\n"; + return res; + } + boolean group(Context context, DeclarationList declList) throws ParserException { int backIndex = tokens.index; String spacing = tokens.get().spacing; @@ -3566,6 +3601,17 @@ boolean group(Context context, DeclarationList declList) throws ParserException } infoMap.put(info = new Info(type.cppName).pointerTypes(type.javaName)); } + + /* Propagate the need for downcasting from base classes */ + for (Type t: baseClasses) { + Set froms = downcasts.get(t.cppName); + if (froms != null) { + for (Type from: froms) { + addDowncast(type.cppName, from); + } + } + } + Type base = new Type("Pointer"); /* The detection of polymorphism can fail if the base class has not been parsed yet, or is not * parsed at all. We might need to add a "polymorphic" info flag and initialize this variable @@ -3589,11 +3635,13 @@ boolean group(Context context, DeclarationList declList) throws ParserException } } String casts = ""; - if (baseClasses.size() > 0) { + if (!baseClasses.isEmpty()) { + // Base classes from multiple inheritance that we don't inherit from in Java for (Type t : baseClasses) { Info baseInfo = infoMap.getFirst(t.cppName); if (!t.javaName.equals("Pointer") && (baseInfo == null || !baseInfo.skip)) { casts += upcast(type, t, false); + addDowncast(t.cppName, t); } } } @@ -3602,11 +3650,21 @@ boolean group(Context context, DeclarationList declList) throws ParserException } if (upcasts.contains(base.javaName)) { + // Base classes explicitly set as needing upcast in infoMap casts += upcast(type, base, true); + addDowncast(base.cppName, base); } else if (polymorphicClasses.contains(base.javaName) && base.virtual) { + // In this case we know we need upcast casts += upcast(type, base, false); + addDowncast(base.cppName, base); } + Set froms = downcasts.get(base.cppName); + if (froms != null) + for (Type t: froms) { + casts += downcast(type, t); + } + decl.signature = type.javaName; tokens.index = startIndex; String shortName = name.substring(name.lastIndexOf('.') + 1); @@ -3705,12 +3763,8 @@ boolean group(Context context, DeclarationList declList) throws ParserException pointerConstructor = false, abstractClass = info != null && info.purify && !ctx.virtualize, allPureConst = true, haveVariables = false; - String constructorName = Templates.strip(originalName); - int constructorNamespace = constructorName.lastIndexOf("::"); - if (constructorNamespace >= 0) { - constructorName = constructorName.substring(constructorNamespace + 2); - } - Info constructorInfo = infoMap.getFirst(type.cppName + "::" + constructorName); + String constructorName = constructorName(originalName); + Info constructorInfo = infoMap.getFirst(constructorName); for (Declaration d : declList2) { polymorphic |= d.declarator != null && d.declarator.type != null && d.declarator.type.virtual; @@ -3866,7 +3920,7 @@ boolean group(Context context, DeclarationList declList) throws ParserException } if (/*(context.templateMap == null || context.templateMap.full()) &&*/ constructorInfo == null) { - infoMap.put(constructorInfo = new Info(type.cppName + "::" + constructorName)); + infoMap.put(constructorInfo = new Info(constructorName)); } if (constructorInfo.javaText == null && inheritedConstructors.length() > 0) { // save constructors to be able to inherit them with C++11 "using" statements diff --git a/src/main/java/org/bytedeco/javacpp/tools/Type.java b/src/main/java/org/bytedeco/javacpp/tools/Type.java index 0e43414aa..a6bef4d1f 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Type.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Type.java @@ -51,6 +51,10 @@ class Type { } } + @Override public int hashCode() { + return cppName.hashCode() ^ javaName.hashCode(); + } + String signature() { String sig = ""; for (char c : javaName.substring(javaName.lastIndexOf(' ') + 1).toCharArray()) { From fab19577bfaaee31df54e31d8e58e4b53164fbc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Thu, 7 Sep 2023 17:26:26 +0200 Subject: [PATCH 06/16] Declare polymorphicClasses as Set instead of HashSet. Update comments. --- .../java/org/bytedeco/javacpp/tools/Parser.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index f0e652a78..188461676 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -97,9 +97,19 @@ public Parser(Logger logger, Properties properties, String encoding, String line InfoMap leafInfoMap = null; TokenIndexer tokens = null; String lineSeparator = null; - Set upcasts = new HashSet<>(); // Java names of base classes needing upcast - Map> downcasts = new HashMap<>(); // Keys: classes which child classes needs downcast constructor. Values: classes to downcast from. - HashSet polymorphicClasses = new HashSet<>(); // Contains Java names + /** + * Java names of classes needing upcast from their subclasses. + */ + Set upcasts = new HashSet<>(); + /** + * Classes that have a base class appearing as key in this map need a downcast constructor. + * The associated value gives the class to downcast from. + */ + Map> downcasts = new HashMap<>(); + /** + * Java names of classes recognized as polymorphic. + */ + Set polymorphicClasses = new HashSet<>(); private void addDowncast(String base, Type from) { Set types = downcasts.get(base); From 17bc19984c697168254fec909b1f2788e0271751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Thu, 7 Sep 2023 17:49:38 +0200 Subject: [PATCH 07/16] Rearrange order of constructors --- .../org/bytedeco/javacpp/tools/Parser.java | 59 +++++++++---------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 188461676..ebf681728 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -3850,46 +3850,45 @@ boolean group(Context context, DeclarationList declList) throws ParserException } } - final boolean addArrayConstructor; + final boolean addArrayConstructor, addDefaultConstructor, addPointerCastConstructor; + /* Annotations currently used on constructors are @SharedPtr and @Name. @SharedPtr produces + * memory corruption if applied to arrays. And @Name needs special versions of the provided + * C++ function that return arrays. So for safety we disable arrays for classes with + * annotations on constructor. + * position and getPointer are incompatible with @SharedPtr, but compatible with @Name. + * Since we don't distinguish annotations here, we disable them in both cases. */ if (implicitConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize)) { - constructors += " /** Default native constructor. */\n" + - " public " + shortName + "() { super((Pointer)null); allocate(); }\n" + - " " + constructorAnnotations + "private native void allocate();\n" + - " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + - " public " + shortName + "(Pointer p) { super(p); }\n"; - - /* Annotations currently used on constructors are @SharedPtr and @Name. @SharedPtr produces - * memory corruption if applied to arrays. And @Name needs special versions of the provided - * C++ function that return arrays. So for safety we disable arrays for classes with - * annotations on constructor. - * position and getPointer are incompatible with @SharedPtr, but compatible with @Name. - * Since we don't distinguish annotations here, we disable them in both cases. - */ + addDefaultConstructor = addPointerCastConstructor = true; addArrayConstructor = constructorAnnotations.isEmpty(); } else { if ((info == null || !info.purify) && (!abstractClass || ctx.virtualize)) { constructors += inheritedConstructors; } - - if (!pointerConstructor) { - constructors += " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + - " public " + shortName + "(Pointer p) { super(p); }\n"; - } + addDefaultConstructor = false; + addPointerCastConstructor = !pointerConstructor; addArrayConstructor = (defaultConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize) && !arrayConstructor && constructorAnnotations.isEmpty()); } - if (addArrayConstructor) { - constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + - " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n" + - " private native void allocateArray(long size);\n" + - " @Override public " + shortName + " position(long position) {\n" + - " return (" + shortName + ")super.position(position);\n" + - " }\n" + - " @Override public " + shortName + " getPointer(long i) {\n" + - " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + - " }\n"; - } + if (addDefaultConstructor) + constructors += " /** Default native constructor. */\n" + + " public " + shortName + "() { super((Pointer)null); allocate(); }\n"; + if (addArrayConstructor) + constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + + " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n"; + if (addPointerCastConstructor) + constructors += " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + + " public " + shortName + "(Pointer p) { super(p); }\n"; + if (addDefaultConstructor) + constructors += " " + constructorAnnotations + "private native void allocate();\n"; + if (addArrayConstructor) + constructors += " private native void allocateArray(long size);\n" + + " @Override public " + shortName + " position(long position) {\n" + + " return (" + shortName + ")super.position(position);\n" + + " }\n" + + " @Override public " + shortName + " getPointer(long i) {\n" + + " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + + " }\n"; if (info == null || !info.skipDefaults) { decl.text += constructors; From 0ff0646c9dcba7de4303c51fe201928e61834385 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Fri, 8 Sep 2023 07:43:58 +0200 Subject: [PATCH 08/16] Reformat comments --- .../java/org/bytedeco/javacpp/tools/Parser.java | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index ebf681728..0960767c2 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -97,18 +97,11 @@ public Parser(Logger logger, Properties properties, String encoding, String line InfoMap leafInfoMap = null; TokenIndexer tokens = null; String lineSeparator = null; - /** - * Java names of classes needing upcast from their subclasses. - */ + /** Java names of classes needing upcast from their subclasses. */ Set upcasts = new HashSet<>(); - /** - * Classes that have a base class appearing as key in this map need a downcast constructor. - * The associated value gives the class to downcast from. - */ + /** Classes that have a base class appearing as key in this map need a downcast constructor. The associated value gives the class to downcast from. */ Map> downcasts = new HashMap<>(); - /** - * Java names of classes recognized as polymorphic. - */ + /** Java names of classes recognized as polymorphic. */ Set polymorphicClasses = new HashSet<>(); private void addDowncast(String base, Type from) { From fd1a5c0b21d45b1dc5ee66f914868d8db7b7e791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Fri, 8 Sep 2023 08:47:16 +0200 Subject: [PATCH 09/16] Rearrange order of constructors --- .../org/bytedeco/javacpp/tools/Parser.java | 76 ++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 0960767c2..c34970c85 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -3843,45 +3843,53 @@ boolean group(Context context, DeclarationList declList) throws ParserException } } - final boolean addArrayConstructor, addDefaultConstructor, addPointerCastConstructor; - /* Annotations currently used on constructors are @SharedPtr and @Name. @SharedPtr produces - * memory corruption if applied to arrays. And @Name needs special versions of the provided - * C++ function that return arrays. So for safety we disable arrays for classes with - * annotations on constructor. - * position and getPointer are incompatible with @SharedPtr, but compatible with @Name. - * Since we don't distinguish annotations here, we disable them in both cases. */ if (implicitConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize)) { - addDefaultConstructor = addPointerCastConstructor = true; - addArrayConstructor = constructorAnnotations.isEmpty(); + constructors += " /** Default native constructor. */\n" + + " public " + shortName + "() { super((Pointer)null); allocate(); }\n"; + if (constructorAnnotations.isEmpty()) { + constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + + " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n"; + } + constructors += " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + + " public " + shortName + "(Pointer p) { super(p); }\n" + + " " + constructorAnnotations + "private native void allocate();\n"; + if (constructorAnnotations.isEmpty()) { + /* Annotations currently used on constructors are @SharedPtr and @Name. @SharedPtr produces + * memory corruption if applied to arrays. And @Name needs special versions of the provided + * C++ function that return arrays. So for safety we disable arrays for classes with + * annotations on constructor. + * position and getPointer are incompatible with @SharedPtr, but compatible with @Name. + * Since we don't distinguish annotations here, we disable them in both cases. */ + constructors += " private native void allocateArray(long size);\n" + + " @Override public " + shortName + " position(long position) {\n" + + " return (" + shortName + ")super.position(position);\n" + + " }\n" + + " @Override public " + shortName + " getPointer(long i) {\n" + + " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + + " }\n"; + } } else { if ((info == null || !info.purify) && (!abstractClass || ctx.virtualize)) { constructors += inheritedConstructors; } - addDefaultConstructor = false; - addPointerCastConstructor = !pointerConstructor; - addArrayConstructor = (defaultConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize) - && !arrayConstructor && constructorAnnotations.isEmpty()); - } - - if (addDefaultConstructor) - constructors += " /** Default native constructor. */\n" + - " public " + shortName + "() { super((Pointer)null); allocate(); }\n"; - if (addArrayConstructor) - constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + - " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n"; - if (addPointerCastConstructor) - constructors += " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + - " public " + shortName + "(Pointer p) { super(p); }\n"; - if (addDefaultConstructor) - constructors += " " + constructorAnnotations + "private native void allocate();\n"; - if (addArrayConstructor) - constructors += " private native void allocateArray(long size);\n" + - " @Override public " + shortName + " position(long position) {\n" + - " return (" + shortName + ")super.position(position);\n" + - " }\n" + - " @Override public " + shortName + " getPointer(long i) {\n" + - " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + - " }\n"; + + if (!pointerConstructor) { + constructors += " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + + " public " + shortName + "(Pointer p) { super(p); }\n"; + } + if (defaultConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize) && !arrayConstructor + && constructorAnnotations.isEmpty() /* See comment above */) { + constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + + " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n" + + " private native void allocateArray(long size);\n" + + " @Override public " + shortName + " position(long position) {\n" + + " return (" + shortName + ")super.position(position);\n" + + " }\n" + + " @Override public " + shortName + " getPointer(long i) {\n" + + " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + + " }\n"; + } + } if (info == null || !info.skipDefaults) { decl.text += constructors; From 9e7d29b84ed375e8bbe3f9dad378f560787c2c4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Fri, 8 Sep 2023 16:36:09 +0200 Subject: [PATCH 10/16] Indentation changes --- .../org/bytedeco/javacpp/tools/Parser.java | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index c34970c85..735dfaadc 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -3845,14 +3845,14 @@ boolean group(Context context, DeclarationList declList) throws ParserException if (implicitConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize)) { constructors += " /** Default native constructor. */\n" + - " public " + shortName + "() { super((Pointer)null); allocate(); }\n"; + " public " + shortName + "() { super((Pointer)null); allocate(); }\n"; if (constructorAnnotations.isEmpty()) { constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + - " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n"; + " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n"; } constructors += " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + - " public " + shortName + "(Pointer p) { super(p); }\n" + - " " + constructorAnnotations + "private native void allocate();\n"; + " public " + shortName + "(Pointer p) { super(p); }\n" + + " " + constructorAnnotations + "private native void allocate();\n"; if (constructorAnnotations.isEmpty()) { /* Annotations currently used on constructors are @SharedPtr and @Name. @SharedPtr produces * memory corruption if applied to arrays. And @Name needs special versions of the provided @@ -3861,12 +3861,12 @@ boolean group(Context context, DeclarationList declList) throws ParserException * position and getPointer are incompatible with @SharedPtr, but compatible with @Name. * Since we don't distinguish annotations here, we disable them in both cases. */ constructors += " private native void allocateArray(long size);\n" + - " @Override public " + shortName + " position(long position) {\n" + - " return (" + shortName + ")super.position(position);\n" + - " }\n" + - " @Override public " + shortName + " getPointer(long i) {\n" + - " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + - " }\n"; + " @Override public " + shortName + " position(long position) {\n" + + " return (" + shortName + ")super.position(position);\n" + + " }\n" + + " @Override public " + shortName + " getPointer(long i) {\n" + + " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + + " }\n"; } } else { if ((info == null || !info.purify) && (!abstractClass || ctx.virtualize)) { @@ -3875,22 +3875,21 @@ boolean group(Context context, DeclarationList declList) throws ParserException if (!pointerConstructor) { constructors += " /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */\n" + - " public " + shortName + "(Pointer p) { super(p); }\n"; + " public " + shortName + "(Pointer p) { super(p); }\n"; } if (defaultConstructor && (info == null || !info.purify) && (!abstractClass || ctx.virtualize) && !arrayConstructor && constructorAnnotations.isEmpty() /* See comment above */) { constructors += " /** Native array allocator. Access with {@link Pointer#position(long)}. */\n" + - " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n" + - " private native void allocateArray(long size);\n" + - " @Override public " + shortName + " position(long position) {\n" + - " return (" + shortName + ")super.position(position);\n" + - " }\n" + - " @Override public " + shortName + " getPointer(long i) {\n" + - " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + - " }\n"; + " public " + shortName + "(long size) { super((Pointer)null); allocateArray(size); }\n" + + " private native void allocateArray(long size);\n" + + " @Override public " + shortName + " position(long position) {\n" + + " return (" + shortName + ")super.position(position);\n" + + " }\n" + + " @Override public " + shortName + " getPointer(long i) {\n" + + " return new " + shortName + "((Pointer)this).offsetAddress(i);\n" + + " }\n"; } } - if (info == null || !info.skipDefaults) { decl.text += constructors; } From 087b12634a2a71873d9e8ff0a5cfcfa400c3ba49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Sat, 9 Sep 2023 08:36:43 +0200 Subject: [PATCH 11/16] Change javadoc --- src/main/java/org/bytedeco/javacpp/annotation/Adapter.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java b/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java index 22e444c47..e0355d405 100644 --- a/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java +++ b/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java @@ -13,7 +13,9 @@ * Specifies a C++ class to act as an adapter between a target type and one or more adaptee type(s). * Instances of the adapter class are short-living and last only for the duration of a JNI call. *

- * Six such C++ classes are made available by {@link Generator}: + * Six such C++ classes are made available by {@link Generator} to bridge a few differences, for instance, + * between {@code std::string} and {@link String}, between {@code std::vector}, Java arrays of + * primitive types, {@link Buffer}, and {@link Pointer}, or between {@code xyz::shared_ptr} and {@link Pointer}: *
* * From f74e8f776251d18edfef4dad71c5b59696838ccd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Sun, 10 Sep 2023 08:24:06 +0200 Subject: [PATCH 12/16] Replace LinkedList by ArrayList --- src/main/java/org/bytedeco/javacpp/tools/Parser.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 735dfaadc..b71f3bbf8 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -32,7 +32,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; -import java.util.LinkedList; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -277,10 +276,10 @@ void containers(Context context, DeclarationList declList) throws ParserExceptio dcl.definition.text = "\n" + dcl.definition.text; decl.declarator = dcl; } - LinkedList typeSet = new LinkedList<>(); - typeSet.addAll(Arrays.asList(firstType, secondType, indexType, valueType)); - typeSet.addAll(Arrays.asList(containerType.arguments)); - for (Type type : typeSet) { + ArrayList types = new ArrayList<>(4 + containerType.arguments.length); + types.addAll(Arrays.asList(firstType, secondType, indexType, valueType)); + types.addAll(Arrays.asList(containerType.arguments)); + for (Type type : types) { if (type == null) { continue; } else if (type.annotations == null || type.annotations.length() == 0) { From ec99fb2c39fe0865f82402272b6a987123b87f7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Tue, 12 Sep 2023 08:12:29 +0200 Subject: [PATCH 13/16] Add missing \n --- src/main/java/org/bytedeco/javacpp/tools/Parser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index b71f3bbf8..e93497173 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -3406,9 +3406,9 @@ String downcast(Type derived, Type base) { res += " /** Downcast constructor. */\n" + " public " + derived.javaName + "(" + base.javaName + " pointer) { super((Pointer)null); allocate(pointer); }\n"; if (annotations.isEmpty()) { - res += " @Namespace private native @Name(\"" + downcastType + "_cast<" + derived.cppName + "*>\") void allocate(" + base.javaName + " pointer);"; + res += " @Namespace private native @Name(\"" + downcastType + "_cast<" + derived.cppName + "*>\") void allocate(" + base.javaName + " pointer);\n"; } else { - res += " @Namespace private native " + annotations + "@Name(\"SHARED_PTR_NAMESPACE::" + downcastType + "_pointer_cast<" + derived.cppName + ", " + base.cppName + ">\") void allocate(" + annotations + base.javaName + " pointer);"; + res += " @Namespace private native " + annotations + "@Name(\"SHARED_PTR_NAMESPACE::" + downcastType + "_pointer_cast<" + derived.cppName + ", " + base.cppName + ">\") void allocate(" + annotations + base.javaName + " pointer);\n"; } return res; } From 57097f70e96d209da605b1cc4f2d0a798dce1e41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Guillemet?= Date: Tue, 12 Sep 2023 08:14:10 +0200 Subject: [PATCH 14/16] Move downcast constructor with other constructors. Add check for existing signature. --- .../java/org/bytedeco/javacpp/tools/Parser.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index e93497173..7cf549c4c 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -3661,12 +3661,6 @@ boolean group(Context context, DeclarationList declList) throws ParserException addDowncast(base.cppName, base); } - Set froms = downcasts.get(base.cppName); - if (froms != null) - for (Type t: froms) { - casts += downcast(type, t); - } - decl.signature = type.javaName; tokens.index = startIndex; String shortName = name.substring(name.lastIndexOf('.') + 1); @@ -3889,6 +3883,17 @@ boolean group(Context context, DeclarationList declList) throws ParserException " }\n"; } } + + Set froms = downcasts.get(base.cppName); + if (froms != null) + ADD_DOWNCAST: + for (Type t: froms) { + for (Declaration d : declList2) + if ((shortName + "_" + t.javaName).equals(d.signature)) + break ADD_DOWNCAST; + constructors += downcast(type, t); + } + if (info == null || !info.skipDefaults) { decl.text += constructors; } From a71b3b4d7a9ee7ce515ea4bcd4f5d426982b6645 Mon Sep 17 00:00:00 2001 From: Samuel Audet Date: Tue, 12 Sep 2023 18:07:52 +0900 Subject: [PATCH 15/16] Update CHANGELOG.md and fix nits --- CHANGELOG.md | 1 + .../bytedeco/javacpp/annotation/Adapter.java | 15 +++++----- .../org/bytedeco/javacpp/tools/Parser.java | 30 +++++++++++-------- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e468d24ae..8e3398518 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ + * Enhance `Parser` by adding downcast constructors for polymorphic classes ([pull #700](https://github.com/bytedeco/javacpp/pull/700)) * Fix `Parser` failing to place annotations on default constructors ([pull #699](https://github.com/bytedeco/javacpp/pull/699)) * Let `Parser` output `reset()` methods for basic containers like `std::optional` ([pull #696](https://github.com/bytedeco/javacpp/pull/696)) * Let `Parser` define `front()` and `back()` for one-dimensional basic containers ([pull #695](https://github.com/bytedeco/javacpp/pull/695)) diff --git a/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java b/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java index e0355d405..258cc5311 100644 --- a/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java +++ b/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java @@ -14,8 +14,8 @@ * Instances of the adapter class are short-living and last only for the duration of a JNI call. *

* Six such C++ classes are made available by {@link Generator} to bridge a few differences, for instance, - * between {@code std::string} and {@link String}, between {@code std::vector}, Java arrays of - * primitive types, {@link Buffer}, and {@link Pointer}, or between {@code xyz::shared_ptr} and {@link Pointer}: + * between {@code std::string} and {@link String}, between {@code std::vector}, Java arrays of primitive + * types, {@link Buffer}, and {@link Pointer}, or between {@code xyz::shared_ptr} and {@link Pointer}: *
*
* @@ -69,12 +69,11 @@ * When an argument of a method is annotated, an instance of the adapter class is created from * the Java object passed as argument, and this instance is passed to the C++ function, thus triggering * an implicit cast to the type expected by the function (usually a reference or pointer to the target type). - * If the argument is also annotated with {@link Cast}, - * the adapter instance is cast to the type(s) specified by the {@link Cast} annotation before being passed - * to the function. + * If the argument is also annotated with {@link Cast}, the adapter instance is cast to the type(s) specified + * by the {@link Cast} annotation before being passed to the function. *

- * When a method is annotated, an instance of the adapter is created from the - * value (usually a pointer or reference to the target type) returned by the C++ function or by {@code new} if the method is an allocator. + * When a method is annotated, an instance of the adapter is created from the value (usually a pointer or + * reference to the target type) returned by the C++ function or by {@code new} if the method is an allocator. * If the method is also annotated with {@link Cast}, the value returned by the C++ function is * cast by value 3 of the {@link Cast} annotation, if any, before instantiation of the adapter. * Then a Java object is created from the adapter to be returned by the method. @@ -88,6 +87,8 @@ *
  • {@code void assign(pointer, size, owner)} functions with the same signature than the constructors accepting 3 arguments *
  • A {@code size} member variable for arrays accessed via pointer * + * To reduce further the amount of coding, this annotation can also be used on + * other annotations, such as with {@link StdString}, {@link StdVector}, and {@link SharedPtr}. * * @see Generator * diff --git a/src/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 7cf549c4c..c5bd5f1b4 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -3428,10 +3428,10 @@ String upcast(Type derived, Type base, boolean override) { if (!s.startsWith("@Name")) annotations += s + " "; } } - if (annotations.isEmpty()) { - res += "@Name(\"static_cast<" + base.cppName + "*>\") "; - } else { + if (!annotations.isEmpty()) { res += annotations + "@Name(\"SHARED_PTR_NAMESPACE::static_pointer_cast<" + base.cppName + ", " + derived.cppName + ">\") "; + } else { + res += "@Name(\"static_cast<" + base.cppName + "*>\") "; } res += base.javaName + " " + ecmn + "(" + annotations + derived.javaName + " pointer);\n"; return res; @@ -3605,10 +3605,10 @@ boolean group(Context context, DeclarationList declList) throws ParserException } /* Propagate the need for downcasting from base classes */ - for (Type t: baseClasses) { + for (Type t : baseClasses) { Set froms = downcasts.get(t.cppName); if (froms != null) { - for (Type from: froms) { + for (Type from : froms) { addDowncast(type.cppName, from); } } @@ -3885,14 +3885,18 @@ boolean group(Context context, DeclarationList declList) throws ParserException } Set froms = downcasts.get(base.cppName); - if (froms != null) - ADD_DOWNCAST: - for (Type t: froms) { - for (Declaration d : declList2) - if ((shortName + "_" + t.javaName).equals(d.signature)) - break ADD_DOWNCAST; + for (Type t : froms != null ? froms : new HashSet()) { + boolean found = false; + for (Declaration d : declList2) { + if ((shortName + "_" + t.javaName).equals(d.signature)) { + found = true; + break; + } + } + if (!found) { constructors += downcast(type, t); } + } if (info == null || !info.skipDefaults) { decl.text += constructors; @@ -3919,7 +3923,9 @@ boolean group(Context context, DeclarationList declList) throws ParserException decl.custom = true; } } - if (polymorphic) polymorphicClasses.add(type.javaName); + if (polymorphic) { + polymorphicClasses.add(type.javaName); + } for (Declaration d : declList2) { if ((!d.inaccessible || d.declarator != null && d.declarator.type.friend) From a46a112b1d8df2411efff112a15cd7f2154f3b0d Mon Sep 17 00:00:00 2001 From: Samuel Audet Date: Wed, 13 Sep 2023 10:13:33 +0900 Subject: [PATCH 16/16] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e3398518..40a5076bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ * Enhance `Parser` by adding downcast constructors for polymorphic classes ([pull #700](https://github.com/bytedeco/javacpp/pull/700)) + * Let `Generator` pick up `@Name` annotations on `allocate()` as well ([pull #700](https://github.com/bytedeco/javacpp/pull/700)) * Fix `Parser` failing to place annotations on default constructors ([pull #699](https://github.com/bytedeco/javacpp/pull/699)) * Let `Parser` output `reset()` methods for basic containers like `std::optional` ([pull #696](https://github.com/bytedeco/javacpp/pull/696)) * Let `Parser` define `front()` and `back()` for one-dimensional basic containers ([pull #695](https://github.com/bytedeco/javacpp/pull/695))