soywiz-archive / jtransc

Bytecode to source converting Java & Kotlin code into JavaScript, C++, D, C#, PHP, AS3, Dart and Haxe and run it everywhere. Also use JVM code in your favourite language as a library.
https://jtransc.soywiz.com/
Apache License 2.0
632 stars 67 forks source link

frem -> fmod #180

Closed fxzjshm closed 7 years ago

fxzjshm commented 7 years ago

This may fix a compilnation problem, but I'm not sure if it is correct.

soywiz commented 7 years ago

We have to add a test that fails before the fix and that works after the fix to be sure that it works, so you can be sure that it is correct.

That's the frem opcode:

https://en.wikipedia.org/wiki/Java_bytecode_instruction_listings

frem    72  0111 0010       value1, value2 → result get the remainder from a division between two floats

First disable your fix. So you probably have to add something like System.out.println(9f % 2f);. For example here: https://github.com/jtransc/jtransc/blob/master/jtransc-main/test/javatest/lang/BasicTypesTest.java#L10 (I have to add a separate class for checking all opcodes specifically, but it doesn't exists yet.)

Since it is a C++ specific bug, you have to first run this: https://github.com/jtransc/jtransc/blob/master/jtransc-main/test/CppTest.kt#L37

What those integration tests do is to run those classes with JVM reading the output, and then with JS specific target reading the output, then it compares the output. So in order to test that the only thing you have to do is to add prints with the expected behaviour.

Check that it fails. Then enable the fix again.

You can then commit and push the test. And I will merge it. (Hopefully after fixing master so travis tell us if it is fine on all targets too; I'm working on fixing master it already)

fxzjshm commented 7 years ago

Test added. Because of my mistake, you may squash them into one commit.

soywiz commented 7 years ago

Thanks!