Port more Scala2 tests + some mixin improvements - #25573
Conversation
| val ownerBType = ts.toTypeKind(method.owner.info) | ||
| if (isInterface) { | ||
| superCallTargets.add(ownerBType.asClassBType) | ||
| } |
There was a problem hiding this comment.
There's no point in recording a supercall target if we're going to emit invokestatic. Without this change a fair number of the mixin tests ported in this PR fail due to spurious super-traits.
| cls.info.nonPrivateMember(sym.name).hasAltWith(_.symbol == sym) | ||
| } | ||
|
|
||
| /** Does `method` need a forwarder in class `cls`? |
There was a problem hiding this comment.
This code came from c103926 on March 15, 2016.
Less than a month later, Scala2 got improvements: scala/scala@33e7106
This is porting those improvements.
| * Native, making any test using `ReflectUtil` JVM-only. | ||
| */ | ||
| object ReflectUtil { | ||
| def getFieldAccessible[T: ClassTag](n: String): Field = |
There was a problem hiding this comment.
The Scala2 version of this test util method also made fields non-final. Thankfully we don't need to do that here, because modern JDKs don't want us to do that anymore.
| f[LongMap.Bin[?]]("right").set(m, null) | ||
| ReflectUtil.getFieldAccessible[LongMap.Bin[?]]("left").set(m, null) | ||
| ReflectUtil.getFieldAccessible[LongMap.Bin[?]]("right").set(m, null) | ||
| assertFalse(m.isEmpty) // no NPE, does not access left or right |
There was a problem hiding this comment.
For the file deleted below: the first test already existed in scala3, and the second explicitly tests behavior known to be broken that already has notes in the JVM backend source code, with a rather convoluted test due to test framework limitations, so I didn't bother trying to port it.
11d4d65 to
e0f777d
Compare
e0f777d to
5348f1f
Compare
natsukagami
left a comment
natsukagami
left a comment
There was a problem hiding this comment.
Might not be the best person to review this but the changes in dotc/transformchecks out with the Scala 2 changes, backend/jvm makes sense.
I have no idea about the test changes in backend/jvm though. Did the Dotty one break somehow?
And also did MixinBytecodeTests come from Scala 2?
|
Thanks!
Yes, because now that we don't record static targets as supercall targets (see first comment in this PR), what this test was testing didn't work anymore, so I switched to the interfaces being defined in Java so the calls are recorded as supercall targets.
Yes, but they were library tests there, I moved them to the compiler because that made no sense to me. |
4 participants
Part of #25013
How much have you relied on LLM-based tools in this contribution?
not
How was the solution tested?
you have one guess