Repository navigation
GROOVY-12574: Call an outer class method on the right enclosing instance - #3106
codeconsole wants to merge 1 commit into
Conversation
An implicit-this call from an inner class to a method of an enclosing
class needs the instance of the class that has the method. When the call
has a method target, the invocation writers picked that instance wrongly
in two ways.
In a closure or lambda, the dynamic writer kept the closure's this
object as the receiver, even when the method belongs to an enclosing
instance of that object. A closure in a field initializer of an inner
class of a @CompileStatic class gets its target from the static
compiler, and the dynamic writer also writes that initializer into each
@CompileDynamic constructor, so
@CompileStatic
class Outer {
String pub() { 'pub' }
class Inner {
String v = { -> pub() }()
@CompileDynamic Inner() {}
}
}
failed with "GroovyCastException: Cannot cast object 'Outer$Inner@...'
with class 'Outer$Inner' to class 'Outer'". Lambdas, nested closures,
private methods, interface default methods and trait methods failed the
same way, as did a @CompileStatic inner class of a dynamic class.
Outside a closure, both writers loaded DeclaringClass.this, which walks
the enclosing instances until it reaches the declaring class itself. A
method that an intermediate enclosing class inherits was therefore
called on the wrong instance or, when the outermost class does not
inherit it, on an instance of the wrong type:
class Base { String base() { 'base' } }
@CompileStatic
class Outer {
class Mid extends Base {
class Inner { String m() { base() } }
}
}
failed verification with "Type 'Outer' (current frame, stack[0]) is not
assignable to 'Base'". When Outer extends Base too, base() ran on the
Outer instance, where dynamic Groovy and Java use the Mid instance.
Property reads through an inherited getter behaved the same.
Both writers now use the innermost enclosing instance whose class
extends or implements the declaring class, as the static writer already
did inside closures.
✅ All tests passed ✅🏷️ Commit: f359f7e Learn more about TestLens at testlens.app/docs. |
daniellansun
left a comment
daniellansun
left a comment
There was a problem hiding this comment.
LGTM for GROOVY-12574.
Implicit-this calls from an inner class must land on the innermost enclosing instance whose type actually has the method. InvocationWriter.getImplicitReceiverType (skip generated functions, walk this$0, isSubtype) used from both dynamic and SC writeDirectMethodCall fixes the @CompileStatic outer + @CompileDynamic inner ctor / field-initializer closure failure mode.
InnerClassTest coverage is strong: nested Mid/Inner, private/public/default trait/interface methods, lambdas, collect, SC outer + dynamic inner, and parameterized CD/TC/CS with Mid-extends-Base vs Outer-extends-Base shadowing (base:Mid).
CI green. (Codecov comment not yet posted on this PR at review time; tests exercise the new paths.)
An implicit-this call from an inner class to a method of an enclosing class now runs on the innermost enclosing instance whose class has the method, in dynamic and statically compiled code, inside and outside closures and lambdas.
Before, a closure in a field initializer of an inner class of a
@CompileStaticclass failed in each@CompileDynamicconstructor:The static compiler gives the call a method target, and the dynamic invocation writer, which writes the initializer into the
@CompileDynamicconstructor, kept the closure's this object as the receiver. Lambdas, nested closures, private methods, interface default methods and trait methods failed the same way, as did a@CompileStaticinner class of a dynamic class.A method inherited by an intermediate enclosing class (
Outer > Mid extends Base > Innercallingbase()) failed verification under@CompileStatic, or ran on the outermost instance when that class inherits the method too. Both invocation writers loadedDeclaringClass.this; they now use the innermost enclosing instance whose class extends or implements the declaring class, as the static writer already did inside closures.Tests:
InnerClassTest.testUsageOfOuterMethod11andtestUsageOfOuterMethod12(dynamic,@TypeCheckedand@CompileStatic).testUsageOfOuterMethod11and the@CompileStaticcase oftestUsageOfOuterMethod12fail without the change; the other two cases guard the dynamic and type-checked paths. Thegroovy,gls,bugsandorg.codehaus.groovypackages pass (15,499 tests). The Checkstyle and CodeNarc gates pass.Seen in 4.0.33, 5.0.8 and 6.0.0.