test writes to direct byte buffers

[adinn]

As promised here is a unit test to check writes to direct byte buffers, It passes on x86 and AArch64 with the latest head.

n.b. I reverted the AArch64 address lowering code to revert the fix that updated usages one by one and, as expected, the unaligned int writes failed because the second byte of the int was being written at offset 1 rather than offset 2. So, this test would have caught the error in the original commit.

[dougxc]

Looks good to me - thanks for doing this.

[christhalinger]

Why are there no long tests?

[dougxc]

Long tests?

[christhalinger]

Too brief :) What I was trying to say, the new test tests for all basic datatypes except long:

 +        ret.byteValue = buffer.get();
 +        ret.byteValue += buffer.get();
 +        ret.shortValue = buffer.getShort();
 +        ret.intValue = buffer.getInt();
 +        ret.doubleValue = buffer.getDouble();
 +        ret.floatValue = buffer.getFloat();

Is there a reason?

[adinn]

Hi Chris!

Well, the immediate reason is that this test does all the same checks as the non-direct byte buffer test. However, you have a point that writing longs ought to be tested.

I was going to suggest as an excuse for this omission that these tests exercise cases where a fragment of the standard transfer unit is transferred i.e. that they test disassembly and reassembly of elements that make up such units, both at aligned and unaligned offsets. However, i) that doesn't apply when transferring doubles and ii) I am not sure there is any reason to omit testing that a full unit is transferred correctly both at aligned and unaligned offsets.

So, yes, there is no good reason and, indeed, both tests are not actually as comprehensive as they need to be(also, in related news, the internet is broken! ;-) Perhaps this could be fixed with a follow-up commit?

[dougxc]

Unless there's an urgency for this PR, I'd prefer the extra tests to be added here to reduce the integration overhead on my side... pretty please... ;-)

[adinn]

ok, I updated both byte buffer tests to check reads and writes of longs :-)

[christhalinger]

Cool, thanks. Can you fix the internet too?