diff --git a/CHANGELOG.md b/CHANGELOG.md index e468d24ae..40a5076bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +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)) diff --git a/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java b/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java index 6369e45e3..258cc5311 100644 --- a/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java +++ b/src/main/java/org/bytedeco/javacpp/annotation/Adapter.java @@ -10,20 +10,83 @@ 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: - * + * 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} 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}: + *
+ * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + *
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: + * * 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}. * 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/main/java/org/bytedeco/javacpp/tools/Parser.java b/src/main/java/org/bytedeco/javacpp/tools/Parser.java index 6a24d3011..c5bd5f1b4 100644 --- a/src/main/java/org/bytedeco/javacpp/tools/Parser.java +++ b/src/main/java/org/bytedeco/javacpp/tools/Parser.java @@ -96,7 +96,20 @@ 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. */ 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); + if (types == null) { + downcasts.put(base, types = new HashSet<>(1)); + } + types.add(from); + } static String removeAnnotations(String s) { return s.substring(s.lastIndexOf(' ') + 1); @@ -111,6 +124,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) { @@ -253,10 +276,10 @@ void containers(Context context, DeclarationList declList) throws ParserExceptio dcl.definition.text = "\n" + dcl.definition.text; decl.declarator = dcl; } - LinkedHashSet typeSet = new LinkedHashSet<>(); - 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) { @@ -3360,27 +3383,58 @@ 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 "; - /* 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)); - Info info = infoMap.getFirst(constructorName); + String downcast(Type derived, Type base) { + final String downcastType; + if (base.virtual) { + if (polymorphicClasses.contains(base.javaName)) { + downcastType = "dynamic"; + } else { + return ""; // No downcast possible in this case + } + } else { + downcastType = "static"; + } + /* See upcast() about annotations. */ String annotations = ""; - if (info != null && info.annotations != null) { - for (String s : info.annotations) { - annotations += s + " "; + Info constructorInfo = infoMap.getFirst(constructorName(base.cppName)); + if (constructorInfo != null && constructorInfo.annotations != null) { + for (String s : constructorInfo.annotations) { + if (!s.startsWith("@Name")) annotations += s + " "; } - res += annotations + "@Name(\"SHARED_PTR_NAMESPACE::static_pointer_cast<" + to.cppName + ", " + from.cppName + ">\") "; + } + 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);\n"; + } else { + 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; + } + + 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 += 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"; + return res; } boolean group(Context context, DeclarationList declList) throws ParserException { @@ -3423,15 +3477,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 +3499,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('(')) { @@ -3544,11 +3603,33 @@ 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 + * 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); + polymorphic |= nextPolymorphic; + if (nextPolymorphic && next.virtual && !upcasts.contains(next.javaName)) { + 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(); @@ -3556,11 +3637,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); } } } @@ -3569,7 +3652,13 @@ 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); } decl.signature = type.javaName; @@ -3670,14 +3759,11 @@ 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; 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();"); @@ -3752,19 +3838,29 @@ 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" + - " /** 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 + "() { 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" + - " 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"; + " " + 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; @@ -3774,7 +3870,8 @@ 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) { + 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" + @@ -3786,6 +3883,21 @@ boolean group(Context context, DeclarationList declList) throws ParserException " }\n"; } } + + Set froms = downcasts.get(base.cppName); + 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; } @@ -3811,6 +3923,10 @@ 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 @@ -3823,7 +3939,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()) { 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); }