aionnetwork / AVM

Enabling Java code to run in a blockchain environment
https://theoan.com/
MIT License
49 stars 25 forks source link

[CLOSED] boolean[] is wrapped as ByteArray and then unwrapped into byte[] while the user may still expect boolean[] #263

Closed aionbot closed 5 years ago

aionbot commented 5 years ago

Issue created by nancyaion (on Thursday Sep 27, 2018 at 20:21 GMT)

We do not have the BooleanArray wrapper. I see the reason behind it; however, a user may still expect boolean[] while one can only get byte[].

For example, in our test shadowing.testString line 44,

    Assert.assertTrue(java.util.Arrays.equals(new byte[]{1, 0, 1, 0, 1, 0, 0}, (byte[]) TestingHelper.decodeResult(result))); // passes
    //Assert.assertTrue(java.util.Arrays.equals(new boolean[]{true, false, true, false, true, false, false}, (boolean[]) TestingHelper.decodeResult(result)));  // does not pass
aionbot commented 5 years ago

Comment by jeff-aion (on Friday Sep 28, 2018 at 21:25 GMT)

What is the rationale behind not providing BooleanArray? That will cause lots of problems. For example, I just tried adding these 2 lines of code to a contract and it produced a ClassFormatError:

    public void foo(byte[] arg) {}
    public void foo(boolean[] arg) {}

I would suspect that we absolutely need BooleanArray so did we make a note of why we didn't think that we did or what the problem with it was? Was there some sort of difficulty with it?

aionbot commented 5 years ago

Comment by nancyaion (on Friday Oct 05, 2018 at 15:35 GMT)

Was thinking if we could combine ArrayWrappingMethodAdapterRef and ArrayWrappingMethodAdapter and then realized it's better to keep them as is since they utilized different ways to generate/replace the bytecode. ArrayWrappingMethodAdapterRef inherits from MethodNode which has access to the stack frames; ArrayWrappingMethodAdapter inherits from AdaptorVisitor which provides a convenient way to generate method call bytecode with the invokeVirtual method. So both are necessary.

Close this issue.