groovy-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jochen Theodorou (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (GROOVY-8946) @CompileStatic ignores declared type and forces a cast when inside a closure
Date Sat, 05 Jan 2019 12:05:00 GMT

    [ https://issues.apache.org/jira/browse/GROOVY-8946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16734866#comment-16734866
] 

Jochen Theodorou edited comment on GROOVY-8946 at 1/5/19 12:04 PM:
-------------------------------------------------------------------

[~kikoalemao] the code passes compilation, thus it should not have failed at runtime in this
way.

How does Reference work? Whenever you write a variable in a closure a Reference is used. The
type for the variable in the reference is retained at compile time. That means code like this:
{code:java}
String x
1.times { x = "s" }
{code}
will compile to code that will cast the right side of the x="s" assignment to String before
storing the value in the Reference. That is why you have the cast to JSONElement via castToType
in there. This mechanic is not specific to static compilation. You would find for non-static
compilation as well, only that the cast is omitted if the compiler recognizes it is not required...
for example if the variable is of type Object.

 

Now with Closure and flow typing things are a bit more complicated. We do not know when and
if the Closure is executed. Because of that the type must actually be an intersection type
of the new values inferred type and the already inferred type before. Strictly spoken this
is not needed if the variable is not used after the Closure anymore, but that could lead to
problems of understanding. Which means in
{code:java}
T t = ...
1.times { t = s }
{code}
 we will have in line 2 a new type for t that is the intersection of T and S (be S the inferred
type of s). Let's call this X. Since the compiler emits type in bytecode for the declaration
it should actually compile to:
{code:java}
X t = (X) ...
1.times { t = (X) s }{code}
 since Object.getAt is used in your example for json[token], My expectation is that X is
Object alone (no interfaces Serializable or JSONElement on it as well).

[~daniel_sun] do you think my analysis is correct so far? 

Obviously this is not the case.


was (Author: blackdrag):
[~kikoalemao] the code passes compilation, thus it should not have failed at runtime in this
way.

How does Reference work? Whenever you write a variable in a closure a Reference is used. The
type for the variable in the reference is retained at compile time. That means code like this:
{code:java}
String x
1.times { x = "s" }
{code}
will compile to code that will cast the right side of the x="s" assignment to String before
storing the value in the Reference. That is why you have the cast to JSONElement via castToType
in there. This mechanic is not specific to static compilation. You would find for non-static
compilation as well, only that the cast is omitted if the compiler recognizes it is not required...
for example if the variable is of type Object.

 

Now with Closure and flow typing things are a bit more complicated. We do not know when and
if the Closure is executed. Because of that the type must actually be an intersection type
of the new values inferred type and the already inferred type before. Strictly spoken this
is not needed if the variable is not used after the Closure anymore, but that could lead to
problems of understanding. Which means in
{code:java}
T t = ...
1.times { t = s }
{code}
 we will have in line 2 a new type for t that is the intersection of T and S (be S the inferred
type of s). Let's call this X. Since the compiler emits type in bytecode for the declaration
it should actually compile to:
{code:java}
X t = (X) ...
1.times { t = (X) s }{code}
 since Object.getAt is used in your example for json[token], My expectation is that X is
Object alone (no interfaces Serializable or JSONElement on it as well).

 

Obviously this is not the case.

> @CompileStatic ignores declared type and forces a cast when inside a closure
> ----------------------------------------------------------------------------
>
>                 Key: GROOVY-8946
>                 URL: https://issues.apache.org/jira/browse/GROOVY-8946
>             Project: Groovy
>          Issue Type: Bug
>          Components: Static compilation
>    Affects Versions: 2.4.16, 3.0.0-alpha-3, 2.5.5
>         Environment: Linux wmtz00088 4.4.0-141-generic #167-Ubuntu SMP Wed Dec 5 10:40:15
UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
>            Reporter: Frederico Costa Galvão
>            Priority: Major
>
> Using a typed method (which happens to be JSON.parse from grails.converters) but handling
stuff as `Object` still makes the compiler generate a cast to the infered value type, ignoring
the declared type.
> I wanted to reproduce this as close as possible, so my snippet grabs stuff from grails.
However, this should be easily reproducible with a simple typed method instead of using the
grails stuff.
> For readability's sake, the original code (greatly simplified bellow) tries to navigate
through a json object with a deep path like 'a.b.c', in which the values at each step could
be multidimensional or not (and implicit `collect`s happens when it is).
> {code}
> @GrabResolver(name = 'r1', root = 'https://repo.grails.org/grails/core')
> @Grapes([
>     @Grab("org.grails.plugins:converters:3.3.+"),
>     @Grab(group='org.grails', module='grails-encoder', version='3.3.+'),
>     @Grab(group='org.grails', module='grails-web', version='3.3.+'),
>     @Grab(group='commons-io', module='commons-io', version='2.3'),
> ])
> @GrabExclude('org.codehaus.groovy:groovy-xml')
> import grails.converters.JSON
> import groovy.transform.CompileStatic
> @CompileStatic
> class What {
>     static void _do() {
>         Object json = JSON.parse('[{"a":1},{"a":2}]')
>         Object json2 = json['a']
>         final boolean res = 'a'.tokenize('.').every { final String token ->
>             json = json[token]//MARK, ERROR
>             return json
>         }
>         assert json == [1, 2]
>     }
> }
> What._do()
> {code}
> When I try to run that, I get:
> {noformat}
> org.codehaus.groovy.runtime.typehandling.GroovyCastException: Cannot cast object '[1,
2]' with class 'java.util.ArrayList' to class 'org.grails.web.json.JSONElement' due to: groovy.lang.GroovyRuntimeException:
Could not find matching constructor for: org.grails.web.json.JSONElement(Integer, Integer)
> {noformat}
> And the bytecode for the relevant code is (Luyten is the decompiler of choice, but I've
seen the same with groovyConsole's inner AST browser):
>  - outside `json` reference
> {code:java}
> final Reference json = new Reference((Object)JSON.parse("[{\"a\":1},{\"a\":2}]"));
> {code}
>  - closure inner `json` reference
> {code:java}
> private /* synthetic */ Reference json;
> {code}
>  - marked line
> {code:java}
> json.set((Object)ScriptBytecodeAdapter.castToType(DefaultGroovyMethods.getAt(json.get(),
token), (Class)JSONElement.class));
> {code}
> I have tried that with:
>  groovy: 2.4.[11..16], 2.5.5, 3.0.0-alpha-3
> Things that make it run/compile properly:
>  - casting `JSON.parse(...)` to `(Object)`
>  - removing `@CompileStatic`
>  - using `JsonSlurper` (which already returns `Object`)
> I understand that type inference from flow is useful and important, but shouldn't it
respect the declared type whenever it differs from def? It's not inside a type guard, nor
did I use a `JSONElement` specific member to rightfully trigger it. I may be wrong and this
could be by design, but I couldn't find something to justify this out-of-sight cast.
> Also, the other/last argument I have in favor of it being a bug is that the same operation,
if done outside the closure, compiles just fine:
> {code:java}
> final Object json2 = DefaultGroovyMethods.getAt((Object)json.get(), "a");
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message