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.

Sign up for GitHub

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

Jump to bottom

Fix parsing of template specializations and templated friend functions #733

Merged
merged 15 commits into from
Jan 9, 2024
Merged

Fix parsing of template specializations and templated friend functions #733

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
82f2dd2
Untemplate class name before adding them to infoMap
HGuillemet Jan 2, 2024
13eda5d
Fix template instance friend functions
HGuillemet Jan 2, 2024
09e2ba2
Check argument count when instantiating templates
HGuillemet Jan 4, 2024
69dc9e0
Prevent automatic instantiation of full template specializations
HGuillemet Jan 4, 2024
6707164
Use Info.define to force template instanciation when javaText is not …
HGuillemet Jan 5, 2024
ca923d7
Prevent double annotations of templated functions
HGuillemet Jan 5, 2024
bd0ed7a
Update CHANGELOG.md and fix nits
saudet Jan 6, 2024
d0bcdcc
Fix doubled template args in function infoMap query
HGuillemet Jan 7, 2024
cf0b6ff
Fix tracking of virtual inheritance for downcasting
HGuillemet Jan 7, 2024
5d6b8c5
Merge remote-tracking branch 'HGuillemet/fix_info_template' into fix_…
HGuillemet Jan 7, 2024
8008345
Remove class Inheritance
HGuillemet Jan 8, 2024
84cfdc8
Fix javadoc
saudet Jan 8, 2024
2e1f5c9
Support `<>` in templateArguments
HGuillemet Jan 8, 2024
108b9f3
Prevent Context.qualify to add template arguments if some are already…
HGuillemet Jan 8, 2024
01ee71e
Merge remote-tracking branch 'HGuillemet/fix_info_template' into fix_…
HGuillemet Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/java/org/bytedeco/javacpp/tools/DeclarationList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ public boolean add(Declaration decl, String fullName) {
if (templateMap != null && templateMap.empty() && !decl.custom && (decl.type != null || decl.declarator != null)) {
// method templates cannot be declared in Java, but make sure to make their
// info available on request (when Info.javaNames or Info.define is set) to be able to create instances
// decl.custom is true when info has a javaText and define is false. This allows to apply a javaText
// to a template without instantiating it. define forces instantiation.
if (infoIterator == null) {
Type type = templateMap.type = decl.type;
Declarator dcl = templateMap.declarator = decl.declarator;
Expand Down
133 changes: 85 additions & 48 deletions src/main/java/org/bytedeco/javacpp/tools/Parser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,40 @@ public Parser(Logger logger, Properties properties, String encoding, String line
String lineSeparator = null;
/** Java names of classes needing upcast from their subclasses. */
Set<String> 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<String, Set<Type>> downcasts = new HashMap<>();

static private class Inheritance {
Type base;
boolean virtual;
HGuillemet marked this conversation as resolved.
Show resolved Hide resolved

public Inheritance(Type b, boolean v) {
base = b;
virtual = v;
}

@Override
public boolean equals(Object o) {
Inheritance i = (Inheritance) o;
return i != null && i.base.equals(base) && i.virtual == virtual;
}

@Override
public int hashCode() {
return base.hashCode() + (virtual ? 1 : 0);
}
}

/** Classes that have a base class appearing as a key in this map need a downcast constructor.
* The associated value gives the class to downcast from and whether the inheritance is virtual. */
Map<String, Set<Inheritance>> downcasts = new HashMap<>();
/** Java names of classes recognized as polymorphic. */
Set<String> polymorphicClasses = new HashSet<>();

private void addDowncast(String base, Type from) {
Set<Type> types = downcasts.get(base);
if (types == null) {
downcasts.put(base, types = new HashSet<>(1));
private void addDowncast(String base, Type from, boolean virtual) {
Set<Inheritance> inh = downcasts.get(base);
if (inh == null) {
downcasts.put(base, inh = new HashSet<>(1));
}
types.add(from);
inh.add(new Inheritance(from, virtual));
}

static String removeAnnotations(String s) {
Expand Down Expand Up @@ -1381,18 +1404,14 @@ Declarator declarator(Context context, String defaultName, int infoNumber, boole
}
} else if (token.match('<')) {
// template arguments
dcl.cppName += token;
int count2 = 0;
for (token = tokens.next(); !token.match(Token.EOF); token = tokens.next()) {
dcl.cppName += token;
if (count2 == 0 && token.match('>')) {
break;
} else if (token.match('<')) {
count2++;
} else if (token.match('>')) {
count2--;
}
}
Type[] types = templateArguments(context);
dcl.cppName += "<";
for (int i = 0; i<types.length; i++) {
if (i > 0) dcl.cppName += ",";
dcl.cppName += types[i].cppName;
}
if (dcl.cppName.endsWith(">")) dcl.cppName += " ";
dcl.cppName += ">";
} else if (token.match(Token.IDENTIFIER) &&
(dcl.cppName.length() == 0 || dcl.cppName.endsWith("::"))) {
dcl.cppName += token;
Expand Down Expand Up @@ -1640,7 +1659,7 @@ Declarator declarator(Context context, String defaultName, int infoNumber, boole
// pick the Java name from the InfoMap if appropriate
String originalName = fieldPointer ? groupInfo.pointerTypes[0] : dcl.javaName;
if (attr == null && defaultName == null && info != null && info.javaNames != null && info.javaNames.length > 0
&& (dcl.operator || Templates.notExists(info.cppNames[0]) || (context.templateMap != null && context.templateMap.type == null))) {
&& (dcl.operator || Templates.notExists(info.cppNames[0]) || dcl.cppName.equals(info.cppNames[0]))) {
dcl.javaName = info.javaNames[0];
}

Expand Down Expand Up @@ -1781,7 +1800,7 @@ Declarator declarator(Context context, String defaultName, int infoNumber, boole
if (info != null && info.javaText != null) {
definition.signature = definition.text = info.javaText;
definition.declarator = null;
definition.custom = true;
definition.custom = !info.define;
}
if (info == null || !info.skip) {
dcl.definition = definition;
Expand Down Expand Up @@ -1813,6 +1832,12 @@ Declarator declarator(Context context, String defaultName, int infoNumber, boole
localName = dcl.cppName.substring(context.namespace.length() + 2);
}
String simpleName = Templates.strip(localName);
if (dcl.type.friend) {
/* Friend functions are called with ADL, but ADL is not performed when a function is called
with explicit template arguments. So we remove template arguments from @Name and bet that
the compiler will be able to locate the proper instance. */
localName = simpleName;
}
if (!localName.equals(dcl.javaName) && (!simpleName.contains("::") || context.javaName == null)) {
type.annotations += "@Name(\"" + localName + "\") ";
}
Expand Down Expand Up @@ -2375,7 +2400,10 @@ boolean function(Context context, DeclarationList declList) throws ParserExcepti
}
Info info = null, fullInfo = null;
String templateArgs = declList.templateMap != null ? declList.templateMap.toString() : "";
String fullname = dcl.cppName + templateArgs;
String fullname = dcl.cppName;
if (!fullname.endsWith(">")) {
fullname += templateArgs;
}
String param1 = "", param2 = "";
if (dcl.parameters != null) {
param1 = "(";
Expand Down Expand Up @@ -2415,8 +2443,12 @@ boolean function(Context context, DeclarationList declList) throws ParserExcepti
fullname += param1;
info = fullInfo = infoMap.getFirst(fullname, false);
if (info == null) {
info = infoMap.getFirst(dcl.cppName + templateArgs + param2, false);
if (info == null && !templateArgs.isEmpty()) {
String cppName = dcl.cppName;
if (!cppName.endsWith(">")) {
cppName += templateArgs;
}
info = infoMap.getFirst(cppName + param2, false);
if (info == null && !cppName.equals(dcl.cppName)) {
info = infoMap.getFirst(dcl.cppName + param1, false);
if (info == null) {
info = infoMap.getFirst(dcl.cppName + param2, false);
Expand All @@ -2440,7 +2472,7 @@ boolean function(Context context, DeclarationList declList) throws ParserExcepti
// For constructor, we'd better not make this lookup, because of confusion
// with the class info. Kept for now for backwards compatibility.
if (info == null) {
info = infoMap.getFirst(dcl.cppName + templateArgs);
info = infoMap.getFirst(dcl.cppName + (dcl.cppName.endsWith(">") ? "" : templateArgs));
}
if (!type.constructor && !type.destructor && !type.operator && (context.templateMap == null || context.templateMap.full())) {
infoMap.put(info != null ? new Info(info).cppNames(fullname).javaNames(null) : new Info(fullname));
Expand Down Expand Up @@ -2699,10 +2731,12 @@ boolean function(Context context, DeclarationList declList) throws ParserExcepti
if (context.namespace != null && context.javaName == null) {
decl.text += "@Namespace(\"" + context.namespace + "\") ";
}
// append annotations specified for a full function declaration only to avoid overlap with type.annotations
// append annotations specified for a full function declaration only, to avoid overlap with type.annotations
if (fullInfo != null && fullInfo.annotations != null) {
for (String s : fullInfo.annotations) {
type.annotations += s + " ";
if (!type.annotations.contains(s)) { // We can still have overlap since fullinfo are created dynamically from partial info
type.annotations += s + " ";
}
}
}
if (type.constructor && params != null) {
Expand Down Expand Up @@ -2747,7 +2781,7 @@ boolean function(Context context, DeclarationList declList) throws ParserExcepti
if (info != null && info.javaText != null) {
if (first) {
decl.signature = decl.text = info.javaText;
decl.custom = true;
decl.custom = !info.define;
} else {
break;
}
Expand Down Expand Up @@ -2990,7 +3024,7 @@ boolean variable(Context context, DeclarationList declList) throws ParserExcepti
if (info != null && info.javaText != null) {
decl.signature = decl.text = info.javaText;
decl.declarator = null;
decl.custom = true;
decl.custom = !info.define;
}
int count = 0;
for (Token token = tokens.get(); !token.match(Token.EOF); token = tokens.next()) {
Expand Down Expand Up @@ -3230,7 +3264,7 @@ boolean macro(Context context, DeclarationList declList) throws ParserException
}
if (info != null && info.javaText != null) {
decl.signature = decl.text = info.javaText;
decl.custom = true;
decl.custom = !info.define;
break;
}
}
Expand Down Expand Up @@ -3390,7 +3424,7 @@ boolean typedef(Context context, DeclarationList declList) throws ParserExceptio

if (info != null && info.javaText != null) {
decl.signature = decl.text = info.javaText;
decl.custom = true;
decl.custom = !info.define;
}
String comment = commentAfter();
decl.text = comment + decl.text;
Expand Down Expand Up @@ -3429,7 +3463,7 @@ boolean using(Context context, DeclarationList declList) throws ParserException
// inherit constructors
decl.signature = decl.text = info.javaText;
decl.declarator = dcl;
decl.custom = true;
decl.custom = !info.define;
}
String comment = commentAfter();
decl.text = comment + decl.text;
Expand All @@ -3439,9 +3473,9 @@ boolean using(Context context, DeclarationList declList) throws ParserException
return true;
}

String downcast(Type derived, Type base) {
String downcast(Type derived, Type base, boolean virtual) {
final String downcastType;
if (base.virtual) {
if (virtual) {
if (polymorphicClasses.contains(base.javaName)) {
downcastType = "dynamic";
} else {
Expand Down Expand Up @@ -3657,15 +3691,18 @@ boolean group(Context context, DeclarationList declList) throws ParserException
if (type.javaName.length() > 0 && context.javaName != null) {
type.javaName = context.javaName + "." + type.javaName;
}
infoMap.put(info = new Info(type.cppName).pointerTypes(type.javaName));
// Adding this info allows proper ns resolution for next declarations, but
// we don't want it to trigger template instantiation, so we untemplate the
// name first.
infoMap.put(info = new Info(Templates.strip(type.cppName)).pointerTypes(type.javaName));
}

/* Propagate the need for downcasting from base classes */
for (Type t : baseClasses) {
Set<Type> froms = downcasts.get(t.cppName);
Set<Inheritance> froms = downcasts.get(t.cppName);
if (froms != null) {
for (Type from : froms) {
addDowncast(type.cppName, from);
for (Inheritance from : froms) {
addDowncast(type.cppName, from.base, t.virtual || from.virtual);
}
}
}
Expand Down Expand Up @@ -3699,7 +3736,7 @@ boolean group(Context context, DeclarationList declList) throws ParserException
Info baseInfo = infoMap.getFirst(t.cppName);
if (!t.javaName.equals("Pointer") && (baseInfo == null || !baseInfo.skip)) {
casts += upcast(type, t, false);
addDowncast(t.cppName, t);
addDowncast(t.cppName, t, false);
}
}
}
Expand All @@ -3710,11 +3747,11 @@ 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);
addDowncast(base.cppName, base, false);
} 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);
addDowncast(base.cppName, base, false);
}

decl.signature = type.javaName;
Expand Down Expand Up @@ -3940,17 +3977,17 @@ boolean group(Context context, DeclarationList declList) throws ParserException
}
}

Set<Type> froms = downcasts.get(base.cppName);
for (Type t : froms != null ? froms : new HashSet<Type>()) {
Set<Inheritance> froms = downcasts.get(base.cppName);
for (Inheritance i : froms != null ? froms : new HashSet<Inheritance>()) {
boolean found = false;
for (Declaration d : declList2) {
if ((shortName + "_" + t.javaName).equals(d.signature)) {
if ((shortName + "_" + i.base.javaName).equals(d.signature)) {
found = true;
break;
}
}
if (!found) {
constructors += downcast(type, t);
constructors += downcast(type, i.base, i.virtual || base.virtual);
}
}

Expand All @@ -3976,7 +4013,7 @@ boolean group(Context context, DeclarationList declList) throws ParserException
}
int end = text.lastIndexOf('}');
decl.text += text.substring(start, end).replace(base2.javaName, type.javaName);
decl.custom = true;
decl.custom = !info.define;
}
}
if (polymorphic) {
Expand Down Expand Up @@ -4014,7 +4051,7 @@ boolean group(Context context, DeclarationList declList) throws ParserException
decl.type = type;
if (info != null && info.javaText != null) {
decl.signature = decl.text = info.javaText;
decl.custom = true;
decl.custom = !info.define;
} else if (info != null && info.flatten) {
info.javaText = decl.text;
}
Expand Down Expand Up @@ -4431,7 +4468,7 @@ void declarations(Context context, DeclarationList declList) throws ParserExcept
continue;
}
Type type = new Parser(this, info.cppNames[0]).type(context);
if (type.arguments == null) {
if (type.arguments == null || type.arguments.length > map.size()) {
continue;
}
int count = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/bytedeco/javacpp/tools/TemplateMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ boolean empty() {
return false;
}
}
return !isEmpty();
return true;
saudet marked this conversation as resolved.
Show resolved Hide resolved
}

boolean full() {
Expand Down