Skip to content

Commit

Permalink
* Fix Parser handling of template specialization and their `frie…
Browse files Browse the repository at this point in the history 
…nd` declarations (pull #733)
  • Loading branch information
@HGuillemet
HGuillemet authored Jan 9, 2024
1 parent a15c408 commit f540614
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 70 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@

* Fix `Parser` handling of `template` specialization and their `friend` declarations ([pull #733](https://github.com/bytedeco/javacpp/pull/733))
* Improve `Parser` capabilities for `operator` and function templates ([pull #732](https://github.com/bytedeco/javacpp/pull/732))
* Fix `Parser` failing on nested initializer lists and on attributes found inside `enum` declarations
* Fix `Parser` for basic containers like `std::optional<std::pair<int,int> >` ([issue #718](https://github.com/bytedeco/javacpp/issues/718))
Expand Down
32 changes: 17 additions & 15 deletions src/main/java/org/bytedeco/javacpp/tools/Context.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,24 +99,26 @@ String[] qualify(String cppName, String parameters) {
name = name.substring(0, name.length() - parameters.length());
}
TemplateMap map = templateMap;
while (map != null) {
String name2 = map.getName();
if (parameters != null && name2 != null && name2.endsWith(parameters)) {
name2 = name2.substring(0, name2.length() - parameters.length());
}
if (name.equals(name2)) {
String args = "<", separator = "";
for (Type t : map.values()) {
// assume that missing arguments have default values
if (t != null) {
args += separator + t.cppName;
separator = ",";
if (!name.endsWith(">")) {
while (map != null) {
String name2 = map.getName();
if (parameters != null && name2 != null && name2.endsWith(parameters)) {
name2 = name2.substring(0, name2.length() - parameters.length());
}
if (name.equals(name2)) {
String args = "<", separator = "";
for (Type t : map.values()) {
// assume that missing arguments have default values
if (t != null) {
args += separator + t.cppName;
separator = ",";
}
}
names.add(name + args + (args.endsWith(">") ? " >" : ">") + (parameters != null ? parameters : ""));
break;
}
names.add(name + args + (args.endsWith(">") ? " >" : ">") + (parameters != null ? parameters : ""));
break;
map = map.parent;
}
map = map.parent;
}
names.add(name);

Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/bytedeco/javacpp/tools/DeclarationList.java
View file
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
132 changes: 79 additions & 53 deletions src/main/java/org/bytedeco/javacpp/tools/Parser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,20 @@ 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<>();

/** 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, Map<Type, Boolean>> 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) {
Map<Type, Boolean> inheritance = downcasts.get(base);
if (inheritance == null) {
downcasts.put(base, inheritance = new HashMap<>(1));
}
types.add(from);
inheritance.put(from, virtual);
}

static String removeAnnotations(String s) {
Expand Down Expand Up @@ -685,6 +688,9 @@ Type[] templateArguments(Context context) throws ParserException {
}
List<Type> arguments = new ArrayList<>();
for (Token token = tokens.next(); !token.match(Token.EOF); token = tokens.next()) {
if (token.match('>')) {
break;
}
Type type = type(context);
arguments.add(type);
token = tokens.get();
Expand Down Expand Up @@ -764,7 +770,7 @@ Type[] templateArguments(Context context) throws ParserException {

/**
* Read and return the operator following an operator keyword:
* any of new, delete, + - * / % ^ & | ~ ! = < > += -= *= /= %= ^= &= |= << >> >>= <<= == != <= >= <=>(since C++20) && || ++ -- , ->* -> ( ) [ ]
* any of {@code new, delete, + - * / % ^ & | ~ ! = < > += -= *= /= %= ^= &= |= << >> >>= <<= == != <= >= <=>(since C++20) && || ++ -- , ->* -> ( ) [ ]}
* taking care of template arguments, if any.
*/
private String operator(Context context) throws ParserException {
Expand Down Expand Up @@ -1381,18 +1387,18 @@ 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 +1646,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 +1787,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 +1819,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 +2387,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 +2430,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 +2459,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 +2718,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 +2768,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 +3011,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 +3251,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 +3411,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 +3450,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 +3460,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 +3678,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);
Map<Type, Boolean> froms = downcasts.get(t.cppName);
if (froms != null) {
for (Type from : froms) {
addDowncast(type.cppName, from);
for (Map.Entry<Type, Boolean> from: froms.entrySet()) {
addDowncast(type.cppName, from.getKey(), t.virtual || from.getValue());
}
}
}
Expand Down Expand Up @@ -3699,7 +3723,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 +3734,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 +3964,19 @@ boolean group(Context context, DeclarationList declList) throws ParserException
}
}

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

Expand All @@ -3976,7 +4002,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 +4040,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 +4457,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
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ boolean empty() {
return false;
}
}
return !isEmpty();
return true;
}

boolean full() {
Expand Down
Loading

0 comments on commit f540614

Please sign in to comment.