Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Type Resolution #91

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ dalvik
.classpath
.project
.settings
.DS_Store
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jdk:
- oraclejdk8
#- oraclejdk7
#- openjdk7
- openjdk6
#- openjdk6

compiler:
- gcc
Expand Down
69 changes: 68 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<artifactId>bridj</artifactId>
<name>BridJ (NativeLibs4Java C/C++ Interop Layer)</name>
<url>http://code.google.com/p/bridj/</url>
<version>0.7.1-SNAPSHOT</version>
<version>0.8.0-SNAPSHOT</version>
<packaging>bundle</packaging>
<repositories>
<repository>
Expand All @@ -25,6 +25,11 @@
<properties>
<versionSpecificSubPackage>v0_7_0</versionSpecificSubPackage>
<maven.compiler.optimize>true</maven.compiler.optimize>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
<maven.compiler.testSource>1.8</maven.compiler.testSource>
<maven.compiler.testTarget>1.8</maven.compiler.testTarget>
<retrolambdaTarget>1.6</retrolambdaTarget>
</properties>
<scm>
<connection>scm:git:[email protected]:nativelibs4java/BridJ.git</connection>
Expand All @@ -48,6 +53,11 @@
<artifactId>asm</artifactId>
<version>5.0.3</version>
</dependency>
<dependency>
<groupId>com.fasterxml</groupId>
<artifactId>classmate</artifactId>
<version>1.3.3</version>
</dependency>
<dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.core</artifactId>
Expand Down Expand Up @@ -109,6 +119,53 @@
</javahClassNames>
</configuration>
</plugin-->
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.6.0</version>
<configuration>
<source>${maven.compiler.source}</source>
<target>${maven.compiler.target}</target>
<testSource>${maven.compiler.testSource}</testSource>
<testTarget>${maven.compiler.testTarget}</testTarget>
</configuration>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent pom should not be setting these to 1.5. Instead, it should define these properties to 1.5, allowing child poms to override them, without having to redefine the plugin.

<executions>
<execution>
<id>default-compile</id>
<phase>compile</phase>
<goals>
<goal>compile</goal>
</goals>
<configuration>
<source>${maven.compiler.source}</source>
<target>${maven.compiler.target}</target>
</configuration>
</execution>
<execution>
<id>default-testCompile</id>
<phase>test-compile</phase>
<goals>
<goal>testCompile</goal>
</goals>
<configuration>
<testSource>${maven.compiler.testSource}</testSource>
<testTarget>${maven.compiler.testTarget}</testTarget>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>net.orfjackal.retrolambda</groupId>
<artifactId>retrolambda-maven-plugin</artifactId>
<version>2.4.0</version>
<executions>
<execution>
<goals>
<goal>process-main</goal>
<goal>process-test</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.felix</groupId>
<artifactId>maven-bundle-plugin</artifactId>
Expand Down Expand Up @@ -314,6 +371,7 @@
<goal>jar</goal>
</goals>
<configuration>
<source>${maven.compiler.source}</source>
<groups>
<group>
<title>Core Packages</title>
Expand Down Expand Up @@ -526,5 +584,14 @@ BridJ is opensource software. Please refer to LICENSE.BridJ.txt to know under wh
</plugins>
</build>
</profile>
<profile>
<id>disable-java8-doclint</id>
<activation>
<jdk>[1.8,)</jdk>
</activation>
<properties>
<additionalparam>-Xdoclint:none</additionalparam>
</properties>
</profile>
</profiles>
</project>
149 changes: 79 additions & 70 deletions src/main/java/org/bridj/StructFieldDeclaration.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@
*/
package org.bridj;

import static org.bridj.util.AnnotationUtils.isAnnotationPresent;

import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -44,11 +39,18 @@
import org.bridj.ann.Bits;
import org.bridj.ann.Field;
import org.bridj.ann.Union;
import org.bridj.util.Methods;
import org.bridj.util.Types;

import com.fasterxml.classmate.ResolvedTypeWithMembers;
import com.fasterxml.classmate.members.ResolvedField;
import com.fasterxml.classmate.members.ResolvedMember;
import com.fasterxml.classmate.members.ResolvedMethod;

class StructFieldDeclaration {

final StructFieldDescription desc = new StructFieldDescription();
Method setter;
ResolvedMethod setter;
long index = -1, unionWith = -1;//, byteOffset = -1;
Class<?> valueClass;
Class<?> declaringClass;
Expand All @@ -57,79 +59,87 @@ class StructFieldDeclaration {
public String toString() {
return desc.name + " (index = " + index + (unionWith < 0 ? "" : ", unionWith = " + unionWith) + ", desc = " + desc + ")";
}

protected static boolean acceptFieldGetter(Member member, boolean getter) {
if ((member instanceof Method) && ((Method) member).getParameterTypes().length != (getter ? 0 : 1)) {
return false;
}

if (((AnnotatedElement) member).getAnnotation(Field.class) == null) {
return false;
}

int modifiers = member.getModifiers();

return !Modifier.isStatic(modifiers);
}

/**
* Creates a list of structure fields
*/

protected static boolean acceptFieldGetter(ResolvedMember<?> member, boolean getter) {
if ((member instanceof ResolvedMethod) && ((ResolvedMethod) member).getRawMember().getParameterTypes().length != (getter ? 0 : 1)) {
return false;
}

if (member.get(Field.class) == null) {
return false;
}

return !member.isStatic();
}

protected static List<StructFieldDeclaration> listFields(Class<?> structClass) {
List<StructFieldDeclaration> list = new ArrayList<StructFieldDeclaration>();
for (Method method : structClass.getMethods()) {
if (acceptFieldGetter(method, true)) {
StructFieldDeclaration io = fromGetter(method);
try {
Method setter = structClass.getMethod(method.getName(), io.valueClass);
if (acceptFieldGetter(setter, false)) {
io.setter = setter;
}
} catch (Exception ex) {
//assert BridJ.info("No setter for getter " + method);
}
if (io != null) {
list.add(io);
List<StructFieldDeclaration> list = new ArrayList<StructFieldDeclaration>();
ResolvedTypeWithMembers resolvedStruct = Types.resolveTypeWithInstanceMethods(structClass);
for (ResolvedMethod method : resolvedStruct.getMemberMethods()) {
if (acceptFieldGetter(method, true)) {
StructFieldDeclaration io = fromGetter(method);
try {
// this only works when the names are equal, does not support setXXX methods.
ResolvedMethod setter = Methods.getMethod( resolvedStruct.getMemberMethods(), method.getName(), io.valueClass);
if (acceptFieldGetter(setter, false)) {
io.setter = setter;
}
} catch (Exception ex) {
//assert BridJ.info("No setter for getter " + method);
}
if (io != null) {
list.add(io);
}
}
}

int nFieldFields = 0;
for (java.lang.reflect.Field field : structClass.getFields()) {
if (acceptFieldGetter(field, true)) {
StructFieldDeclaration io = StructFieldDeclaration.fromField(field);
if (io != null) {
list.add(io);
nFieldFields++;
}
int nFieldFields = 0;
for ( ResolvedField field : resolvedStruct.getMemberFields()) {
if (acceptFieldGetter(field, true)) {
StructFieldDeclaration io = StructFieldDeclaration.fromField(field);
if (io != null) {
list.add(io);
nFieldFields++;
}
}
if (nFieldFields > 0 && BridJ.warnStructFields) {
BridJ.warning("Struct " + structClass.getName() + " has " + nFieldFields + " struct fields implemented as Java fields, which won't give the best performance and might require counter-intuitive calls to BridJ.readFromNative / .writeToNative. Please consider using JNAerator to generate your struct instead, or use BRIDJ_WARN_STRUCT_FIELDS=0 or -Dbridj.warnStructFields=false to mute this warning.");
}

return list;
}
if (nFieldFields > 0 && BridJ.warnStructFields) {
BridJ.warning("Struct " + structClass.getName() + " has " + nFieldFields + " struct fields implemented as Java fields, which won't give the best performance and might require counter-intuitive calls to BridJ.readFromNative / .writeToNative. Please consider using JNAerator to generate your struct instead, or use BRIDJ_WARN_STRUCT_FIELDS=0 or -Dbridj.warnStructFields=false to mute this warning.");
}

protected static StructFieldDeclaration fromField(java.lang.reflect.Field getter) {
StructFieldDeclaration field = fromMember((Member) getter);
return list;
}

protected static String nameForMember( ResolvedMember<?> member ) {
String name = member.getName();
if (name.matches("get[A-Z].*")) {
return Character.toLowerCase(name.charAt(3)) + name.substring(4);
} else if ( name.matches("set[A-Z].*")) {
return Character.toLowerCase(name.charAt(3)) + name.substring(4);
} else {
return name;
}
}

protected static StructFieldDeclaration fromField(ResolvedField getter) {
StructFieldDeclaration field = fromMember(getter);
field.desc.field = getter;
field.desc.valueType = getter.getGenericType();
field.valueClass = getter.getType();
field.desc.valueType = getter.getType();
field.valueClass = getter.getType().getErasedType();
return field;
}

protected static StructFieldDeclaration fromGetter(Method getter) {
StructFieldDeclaration field = fromMember((Member) getter);
field.desc.getter = getter;
field.desc.valueType = getter.getGenericReturnType();
field.valueClass = getter.getReturnType();
return field;
protected static StructFieldDeclaration fromGetter(ResolvedMethod getter) {
StructFieldDeclaration field = fromMember(getter);
field.desc.getter = getter;
field.desc.valueType = getter.getReturnType();
field.valueClass = getter.getReturnType().getErasedType();
return field;
}

private static StructFieldDeclaration fromMember(Member member) {
private static StructFieldDeclaration fromMember(ResolvedMember<?> member) {
StructFieldDeclaration field = new StructFieldDeclaration();
field.declaringClass = member.getDeclaringClass();
field.declaringClass = member.getRawMember().getDeclaringClass();

String name = member.getName();
if (name.matches("get[A-Z].*")) {
Expand All @@ -138,11 +148,10 @@ private static StructFieldDeclaration fromMember(Member member) {

field.desc.name = name;

AnnotatedElement getter = (AnnotatedElement) member;
Field fil = getter.getAnnotation(Field.class);
Bits bits = getter.getAnnotation(Bits.class);
Alignment alignment = getter.getAnnotation(Alignment.class);
Array arr = getter.getAnnotation(Array.class);
Field fil = member.get(Field.class);
Bits bits = member.get(Bits.class);
Alignment alignment = member.get(Alignment.class);
Array arr = member.get(Array.class);
if (fil != null) {
field.index = fil.value();
//field.byteOffset = fil.offset();
Expand All @@ -166,8 +175,8 @@ private static StructFieldDeclaration fromMember(Member member) {
field.desc.arrayLength = length;
field.desc.isArray = true;
}
field.desc.isCLong = isAnnotationPresent(org.bridj.ann.CLong.class, getter);
field.desc.isSizeT = isAnnotationPresent(org.bridj.ann.Ptr.class, getter);
field.desc.isCLong = member.get(org.bridj.ann.CLong.class) != null;
field.desc.isSizeT = member.get(org.bridj.ann.Ptr.class) != null;
return field;
}
}
29 changes: 24 additions & 5 deletions src/main/java/org/bridj/StructFieldDescription.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@
import org.bridj.util.DefaultParameterizedType;
import org.bridj.util.Utils;

import com.fasterxml.classmate.ResolvedType;
import com.fasterxml.classmate.members.ResolvedField;
import com.fasterxml.classmate.members.ResolvedMethod;

/**
* Internal metadata on a struct field
*/
Expand All @@ -69,9 +73,9 @@ public class StructFieldDescription {
public long bitMask = -1;
public boolean isArray, isNativeObject;
public Type nativeTypeOrPointerTargetType;
public java.lang.reflect.Field field;
public ResolvedField field;
Type valueType;
Method getter;
ResolvedMethod getter;
String name;
boolean isCLong, isSizeT;

Expand All @@ -90,7 +94,12 @@ static Type resolveType(Type tpe, Type structType) {
}

Type ret;
if (tpe instanceof ParameterizedType) {
if (tpe instanceof ResolvedType ) {
ResolvedType rt = (ResolvedType)tpe;
// TODO: what do we do here?
ret = tpe;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this actually be anything other than ResolvedType now?

else if (tpe instanceof ParameterizedType) {
ParameterizedType pt = (ParameterizedType) tpe;
Type[] actualTypeArguments = pt.getActualTypeArguments();
Type[] resolvedActualTypeArguments = new Type[actualTypeArguments.length];
Expand Down Expand Up @@ -178,8 +187,18 @@ static StructFieldDescription aggregateDeclarations(Type structType, List<Struct
field.desc.byteLength = Pointer.SIZE;
//field.callIO = CallIO.Utils.createPointerCallIO(field.valueClass, field.desc.valueType);
} else if (Pointer.class.isAssignableFrom(field.valueClass)) {
Type tpe = (field.desc.valueType instanceof ParameterizedType) ? ((ParameterizedType) field.desc.valueType).getActualTypeArguments()[0] : null;
field.desc.nativeTypeOrPointerTargetType = resolveType(tpe, structType);
Type tpe = null;
if( field.desc.valueType instanceof ResolvedType ) {
ResolvedType rt = (ResolvedType)field.desc.valueType;
if( !rt.getTypeParameters().isEmpty() ) {
tpe = rt.getTypeParameters().get(0);
field.desc.nativeTypeOrPointerTargetType = tpe;
}
}
else if(field.desc.valueType instanceof ParameterizedType) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be dead code.

tpe = ((ParameterizedType) field.desc.valueType).getActualTypeArguments()[0];
field.desc.nativeTypeOrPointerTargetType = resolveType(tpe, structType);
}
if (field.desc.isArray) {
field.desc.byteLength = BridJ.sizeOf(field.desc.nativeTypeOrPointerTargetType);
if (field.desc.alignment < 0)
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/bridj/StructUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ static String describe(StructObject struct, Type structType, StructFieldDescript
try {
Object value;
if (fd.getter != null) {
value = fd.getter.invoke(struct);
value = fd.getter.getRawMember().invoke(struct);
} else {
value = fd.field.get(struct);
value = fd.field.getRawMember().get(struct);
}

if (value instanceof String) {
Expand Down
Loading