-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add Java 8 classes. Again. #28
Conversation
jar-tool/hxformat.json
Outdated
@@ -0,0 +1,82 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use haxe-formatter's default style?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use a bit modified code style, but surely this setup should be removed from that repo. Just forgot about it.
jar-tool/Main.hx
Outdated
// target.visitInsn(Opcodes_Statics.DUP); | ||
target.visitMethodInsn(Opcodes_Statics.INVOKESPECIAL, "java/io/IOException", "<init>", "()V", false); | ||
target.visitInsn(Opcodes_Statics.ATHROW); | ||
target.visitMaxs(2, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It replaces existing class methods bodies with throw new IOException();
I checked old hxjava-std
with a decompiler and found there this kind of hack, so I tried to make a similar thing too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean this call in particular. Does it set the max stack? Because that should be 1 instead of 2 here, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, https://asm.ow2.io/javadoc/org/objectweb/asm/MethodVisitor.html#visitMaxs-int-int-
I commented the first two lines, so it should be 1 :)
But looks like it may be even (1, 1)? I'm new to JVM, so it's a bit confusing for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no locals so it should be (1, 0)
. The only item on the stack is the exception which gets thrown.
jar-tool/Main.hx
Outdated
@:overload | ||
override function visitMethod(access:Int, name:String, desc:String, signature:String, exceptions:NativeArray<String>):MethodVisitor { | ||
final mv:MethodVisitor = cv.visitMethod(access, name, desc, signature, exceptions); | ||
if (!isInterface && mv != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this check something along the lines of "if there is any code" instead of explicitly checking for interfaces? There are abstract methods too where we shouldn't add any code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I check it here to be sure it's a code:
https://github.com/dmitryhryppa/hxjava/blob/master/jar-tool/Main.hx#L31
But as I remember abstracts can contain the code in Java 8. Or it's not true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not what I mean. The condition should check if there is a Code attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like visitCode
only does anything if there is a Code
attribute. Though in this case the interface check should be unnecessary, but that's not very important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I did a small cleanup based on your comments.
There should be some more stuff to get rid of. If there is no code, we can also remove the StackMapTable attribute as well as the Exceptions attribute. There's probably even more, but let's start with these. |
Awesome, thank you for this! I wonder though if we should use a more recent java version? |
So are we merging this? |
IMO we should look into removing these attributes I mentioned. That should help with the file size. |
Will try to do something for Exceptions attributes. But do not know yet how to deal with StackMapTable. |
Well, I did a slight change which gives better results. But I did not find ways to clean Also, I tried to investigate previous |
println('Process $path... '); | ||
final obj:NativeArray<Int8> = Files.readAllBytes(new File(path).toPath()); | ||
final reader = new ClassReader(obj); | ||
final writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed the COMPUTE_FRAMES
bit. I suppose that means it already re-calculates stack frames, so we don't have to do anything by hand.
Hello,
hxjava-std
lib has been reduced from 64mb to 14.5mb.Also, I added sources of the
jar-tool
for stripping the code from .jar's.For future use.
It's a bit dirty, but works.
closes #27