Add Bytecode test (ASM-based) to partest. #2014
Conversation
This commit introduces a new kind of test `Bytecode` that allows
one to inspect bytecode generated for given piece of Scala code.
The bytecode inspection is achieved by inspection of ASM trees.
See the included example for details.
NOTE: This commit does not introduce a new category of pratest tests.
Bytecode tests should be run in `jvm` category of partest tests.
Specific list of changes:
* Add BytecodeTest that contains common utilities to partest
* Add asm to classpath when compiling partest. That's not a
new dependency as it's being already done for javac task
we were running while compiling partest.
* Add an example test that shows how to count null checks in
given method.
|
Requested by @adriaanm for testing his pattern matcher improvements. No more excuses for not improving byte code size generated by pattern matcher! :-) Review by @JamesIry |
|
Also, in case you are guys wondering why I'm not just dumping bytecode and doing textual diff. The reason is that we want to test specific things about bytecode and not it overall shape. |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1715/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2450/ |
|
I bet we find some nice abstractions over raw ASM as we use this, but LGTM as a start. |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1716/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2451/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1715/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2450/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1716/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2451/ |
| */ | ||
| def isNullCheck(node: asm.tree.AbstractInsnNode): Boolean = { | ||
| val opcode = node.getOpcode | ||
| (opcode == asm.Opcodes.IFNULL) || (opcode == asm.Opcodes.IFNONNULL) |
There was a problem hiding this comment.
Micro-review-note:
I prefer to deduplicate the == with Set(asm.Opcodes.IFNULL, asm.Opcodes.IFNONNULL)(node.getOpcode)
There was a problem hiding this comment.
...which is potentially pretty expensive, so lift the Set into a val in object Test. It's only a test, but still.
There was a problem hiding this comment.
It's one of those areas where I cross my fingers and wait for escape analysis to kick in. If it inlines to Set2#contains, we're golden.
There was a problem hiding this comment.
I did it the way I did for reason @paulp pointed out.
|
A general question: if we are testing the pattern matcher, rather than the byte-code emitter, wouldn't it be better to to test Trees instead? In cases when we want to integration test the whole chain, or compiler transforms that operate only on bytecode (e.g. code gen, the experimental optimizer, the recently proposed alternative try-expression transform), bytecode testing makes perfect sense. |
|
This is a useful addition to our tool chest. Just a comment: it may end up being used to test transforms in a JVM-dependent way, when the transform itself is platform-independent. |
|
testing tree equality is a bit harder than bytecode as the trees emitted by patmat are not expressible in source level |
|
@retronym yes, it's more about the whole chain/integration testing. If you care about every byte we spend on translating pattern matches then it makes sense to test the final outcome. |
Add Bytecode test (ASM-based) to partest.
This commit introduces a new kind of test
Bytecodethat allowsone to inspect bytecode generated for given piece of Scala code.
The bytecode inspection is achieved by inspection of ASM trees.
See the included example for details.
NOTE: This commit does not introduce a new category of pratest tests.
Bytecode tests should be run in
jvmcategory of partest tests.Specific list of changes:
new dependency as it's being already done for javac task
we were running while compiling partest.
given method.