polyglot-compiler / JLang

JLang: Ahead-of-time compilation of Java programs to LLVM
http://polyglot-compiler.github.io/JLang/
Other
284 stars 29 forks source link

Support Reflection of Array Type #42

Closed guoyiteng closed 5 years ago

guoyiteng commented 5 years ago

Previously, we represented java array type as jlang.runtime.Array in our runtime environment, so the reflection of array type was not handled correctly in class.getDeclarationFields(). Now, we check the field's type signature in our runtime and assign the corresponding array type_ptr to it.

In this PR, I revert all changes about type_info_ptr in PR #36 since we do not need type class initializer. If we need those changes in the future (e.g. dynamic class loading), we can find them in PR #36.

dz333 commented 5 years ago

Are you missing a commit here or something?

getDeclaredFields always crashes now when I run it

guoyiteng commented 5 years ago

I just tried and it crashed if I did not comment out line 21 (getGenericType) in FieldReflection.java. Could you try it again after commenting out that line?

dz333 commented 5 years ago

I just tried and it crashed if I did not comment out line 21 (getGenericType) in FieldReflection.java. Could you try it again after commenting out that line?

As I said, it crashes when getDeclaredFields is called, which is called without any of the "generics" code. This also causes the HashtableTest.java test to fail. Please double check that you've pushed all your commits.

guoyiteng commented 5 years ago

I am very sure that I pushed all commits I had. I only made one commit, which is 5b69eef. All the changes I didn't push are some intellij config files, which, I believe, are unrelated to this issue.

In addition, HashtableTest.java could run successfully on my laptop. I also ran make test, and three test cases failed as before. It is possible that the current generated code somehow triggered a memory access bug in your system (I also got this kind of error before on my macOS, but not always).

dz333 commented 5 years ago

Ok cool. I can look into it then on my end.

guoyiteng commented 5 years ago

In fact, this fix did not solve the array type problem in general. It only corrects the name of array type as a Field object. We know the underlying type of the jlang.runtime.Array because we can examine its field's signature. However, we still have problems with array in the following cases.

System.out.println((new int[]{1}).toString());    // output: jlang.runtime.Array@10db8620
// or
// fld is a Field object of an int array.
System.out.println(fld.get(cls));  // output: jlang.runtime.Array@70b45a0

In these cases, if we want to implement the correct toString() method of an array, we must know the type of that array in runtime. Currently, we don't pass this information into llvm in our compiler. We may need to change our object layout for array type to fix this problem.

guoyiteng commented 5 years ago

Also, when we fill the typeclass ptr for an array field, we use its signature to look up a typeclass in our unorder_map. However, it seems that signature and class name use two different mangling rules. For example, as JNI specified, String[] has signature class [Ljava/lang/String; while its class name is [Ljava.lang.String;. Our code currently follows the JNI spec to mangles field's type, but in runtime, the global typeclass map uses class names as keys. Shall we change all slashes back to dot in our runtime to correct the name when we construct the field object? Another way to solve it is to directly use the class name as the field signature.

dz333 commented 5 years ago

Ah yes, these are good points. This (the ability to call methods on the generated array class) was something I meant to do once we had the simple runtime generation of class stubs working for arrays.

As far as string mangling goes, let's not change the current representations if possible. It would be better to make special cases for Arrays since they are actually a special case and we're likely to break other reflection features in the runtime otherwise. We can discuss these changes more in person tomorrow.