I am suggesting that when a closure is encountered, it is treated like an
optional branch of the code. So assignments within alter the inferred type the
same way an assignment within an if with no else would. I am not looking to
examine the code flow and distinguish between closure executed immediately and
closure that is executed asynchronously or later or not at all. I have this
change in hand, but did not want to alter the "second pass" checking without
discussing it first.
If you are talking about using a union type, I don't think that is the safe way
to go. Static compilation is enforcing a typecast to the inferred type and
this is a runtime error for 9344.
class A {}
class B extends A{ def b() {}}
class C extends A{}
@CompileStatic
void test() {
def x = new B()
({ x = new C() })()
def z = x // new behavior: typeof(x) and typeof(z) is A
z.b() // new behavior: STC error because "b()" is not a member of A; user
would need to use block instead of closure or add a typecast or type checking
extension
}
-----Original Message-----
From: Jochen Theodorou <blackd...@gmx.org>
Sent: Thursday, June 4, 2020 11:35 AM
To: dev@groovy.apache.org
Subject: Re: STC: Closure shared variable assignment handling
On 04.06.20 17:07, Milles, Eric (TR Tech, Content & Ops) wrote:
> There are a couple open bugs related to STC handling of closure shared
> variables. When a shared variable is assigned a value within the
> closure, the assigned type is not included in the inferred type
> outside the closure/lambda and this leads to some problems.
>
> If this behavior is changed to save the LUB(A,B) as the inferred type
> of "x", as is suggested in 9516 and seems required by 9344, the second
> pass checks for method calls will be replaced by no matching method errors.
> That is "Cannot find matching method java.lang.Object#something().
> Please check if the declared type is correct and if the method exists."
> will replace "A closure shared variable [x] has been assigned with
> various types and the method [charAt(int)] does not exist in the
> lowest upper bound...".* Is this an acceptable change in STC
> behavior?*
>
> @groovy.transform.CompileStatic
>
> void test() {
>
> def x = new A();
>
> { -> x = new B() } // may also be called immediately
>
> x.something() // current STC infers typeof(x) == A
>
> }
as this is currently failing you are suggesting to let it pass and use the type
A?
> I think it was done this way because you could define the closure but
> not call it (like above). Or call it asynchronously:
>
> @groovy.transform.CompileStatic
>
> void test() {
>
> def x = new A();
>
> Thread.start { -> x = new B() }
>
> x.something() // current STC infers typeof(x) == A
>
> }
would you not agree, just by looking at the realized flow here, that x should
not be A?
> https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissu
> es.apache.org%2Fjira%2Fbrowse%2FGROOVY-9516&data=02%7C01%7Ceric.mi
> lles%40thomsonreuters.com%7C7b12f318629043d33b9d08d808a54e51%7C62ccb86
> 46a1a4b5d8e1c397dec1a8258%7C0%7C0%7C637268853358509821&sdata=zoylX
> L4c9Iic%2FHhoVXxE9iXSV2kp4BBQW2m8wEUYeeM%3D&reserved=0
this is
> class A {}
> class B extends A{ def b() {}}
> class C extends A{}
>
> @CompileStatic
> static foo() {
> def x = new B()
> ({ x = new C() })()
> def z = x
> z.b()
> }
ah.. now I get the problem. but to say z is of type C is also wrong.
What we really need here is not really the LUB in the sense that
LUB(B,C) = A, iff B extends A && C extends A && B and C are siblings.
What we would really need is a type A plus all methods defined on B and C. I
vaguely remember that our LUB calculation does things like that..
but maybe I remember wrong. If I go by
https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.groovy-lang.org%2Flatest%2Fhtml%2Fdocumentation%2Fcore-semantics.html%23section-lub&data=02%7C01%7Ceric.milles%40thomsonreuters.com%7C7b12f318629043d33b9d08d808a54e51%7C62ccb8646a1a4b5d8e1c397dec1a8258%7C0%7C0%7C637268853358519815&sdata=NgKVDLj5YZdz43BRn9Du2%2F5yNxoj78BWVWRzAEXSdlk%3D&reserved=0
then I am wrong. Of course I am well aware, that such a type is difficult to
realize. And of course the question is how to do the actual method call in
bytecode. If I cannot cast z to B or B, then I cannot create the required
invokevirtual. This means this must use an invokedynamic, but a different one
than we normally use.
What I am trying to say is that we can handle this case as I imagine it, but
the required implementation work is not small.
> https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissu
> es.apache.org%2Fjira%2Fbrowse%2FGROOVY-9344&data=02%7C01%7Ceric.mi
> lles%40thomsonreuters.com%7C7b12f318629043d33b9d08d808a54e51%7C62ccb86
> 46a1a4b5d8e1c397dec1a8258%7C0%7C0%7C637268853358519815&sdata=qHXA1
> 4ljlSoFfGd3Y6FskDqkUkqF4fnOt1T7BTwbnQ8%3D&reserved=0
This is
> class A {}
> class B {}
>
> @groovy.transform.CompileStatic
> def cs() {
> def var
> var = new A()
> def c = {
> var = new B() // Cannot cast object 'B@4e234c52' with class 'B' to
> class 'A'
> }
> c()
> var.toString()
> }
>
> assert cs() != null
which looks to me like a different problem, var is declared with "def", so
assigning a B should be valid. Only if the static type does no comply with the
assigned type it should cause an error. There should be also no cast inserted.
> https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissu
> es.apache.org%2Fjira%2Fbrowse%2FGROOVY-5874&data=02%7C01%7Ceric.mi
> lles%40thomsonreuters.com%7C7b12f318629043d33b9d08d808a54e51%7C62ccb86
> 46a1a4b5d8e1c397dec1a8258%7C0%7C0%7C637268853358519815&sdata=oLyIU
> IbjMuxQCbLXT5RKs0vGxg%2BRH17syiM4dHyWlgg%3D&reserved=0
This is
> @CompileStatic
> static main(args)
> {
> def list = [1, 2, 3]
>
> def sum = 0
> list.each { int i -> sum += i }
> println(sum)
>
> sum = list.inject(0, { int x, int y -> x + y })
> println(sum)
> }
what I would imagine here is that in def sum, the flow type for sum is int and
for list it is List<Integer>. That means sum+=i should change the flow type
internally to Integer or keep int. The result of the inject should be Integer
and it should always be allowed to assign it to sum, even if not.
bye Jochen
