Comment below.
On Fri, Jun 5, 2020 at 2:55 AM Milles, Eric (TR Tech, Content & Ops) <
eric.mil...@thomsonreuters.com> wrote:
> 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.
>
I think the union type* is the correct thing here and we should make our
bytecode generation be more sophisticated.
* agree this is a poorly understood concept but we need both union and
intersection types but perhaps we should use different names.
Cheers, Paul.
> 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
>
>
