[ 
https://issues.apache.org/jira/browse/GROOVY-12166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ben Sherman updated GROOVY-12166:
---------------------------------
    Attachment: Bug.groovy

> STC: `return field instanceof Subtype` narrows the field's type class-wide, 
> causing `ClassCastException` -- Groovy 5 regression
> -------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12166
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12166
>             Project: Groovy
>          Issue Type: Bug
>          Components: Static compilation
>    Affects Versions: 5.0.7
>         Environment: Ubuntu, JDK 17
>            Reporter: Ben Sherman
>            Priority: Minor
>         Attachments: Bug.groovy
>
>
> (Found while upgrading Nextflow to Groovy 5)
> h2. Summary
> Under Groovy 5, when a `@CompileStatic` class contains a method whose body is
> `return someField instanceof SomeSubtype`, the static type checker narrows the
> *{*}declared type of the field itself{*}* to `SomeSubtype` for the rest of 
> the class.
> The static compiler then emits a spurious `checkcast SomeSubtype` before 
> {*}other{*},
> unrelated calls to that field elsewhere in the class. At runtime those methods
> throw `ClassCastException` whenever the field holds a different subtype of its
> declared type.
> The field's declared type is a common base class; the `instanceof` is only a
> runtime test and must not change the field's static type outside its own
> expression. This worked correctly in Groovy 4.
> h2. Affected / unaffected versions
> |Groovy version|Result|
> |—|—|
> |4.0.29|OK — prints `base`|
> |4.0.31|OK — prints `base`|
> |5.0.7|*{*}BUG{*}* — `ClassCastException`|
> This is a *{*}regression{*}* introduced somewhere between 4.0.x and 5.0.7.
> h2. Minimal reproducible example
> `Bug.groovy`:
> ```groovy
> import groovy.transform.CompileStatic
> abstract class Base {
> String describe()
> { 'base' }
> abstract void unrelated()
> }
> class Alpha extends Base \{ void unrelated() {} }
> class Beta extends Base \{ void unrelated() {} }
> @CompileStatic
> class Holder {
> private Base field
> Holder(Base f)
> { this.field = f }
> // (1) `instanceof` applied directly to the FIELD, with an explicit `return`.
> // This is what poisons the field's inferred type.
> boolean isAlpha()
> { return field instanceof Alpha }
> // (2) An UNRELATED method that merely calls a Base method on the field.
> // Under Groovy 5 the static compiler wrongly narrows `field` to `Alpha`
> // here and emits `checkcast Alpha` before the call.
> String use()
> { field.describe() }
> }
> // `field` holds a Beta and is never an Alpha
> def h = new Holder(new Beta())
> assert h.isAlpha() == false
> println h.use() // Groovy 4 -> "base" | Groovy 5 -> ClassCastException
> ```
> h3. How to run
> ```bash
> # run with a given Groovy version
> groovy Bug.groovy
> ```
> h3. Expected output
> ```
> base
> ```
> h3. Actual output (Groovy 5.0.7)
> ```
> Exception in thread "main" java.lang.ClassCastException: class Beta cannot be 
> cast to class Alpha (Beta and Alpha are in unnamed module of loader 'app')
> at Holder.use(Bug.groovy)
> at Bug.run(Bug.groovy)
> ```
> h2. Bytecode evidence
> Disassembly of `Holder.use()` (`javap -p -c Holder`), which contains 
> *{*}no{*}*
> `instanceof` and only calls `Base.describe()`:
> *{*}Groovy 4.0.29 / 4.0.31 (correct):{*}*
> ```
> 1: getfield // Field field:LBase;
> 4: invokevirtual // Method Base.describe:()Ljava/lang/String;
> ```
> *{*}Groovy 5.0.7 (incorrect — bogus cast injected):{*}*
> ```
> 1: getfield // Field field:LBase;
> 4: checkcast // class Alpha <-- injected; field is declared `Base`
> 7: invokevirtual // Method Alpha.describe:()Ljava/lang/String;
> ```
> Note that `Alpha` does not even override `describe()` — the method is 
> inherited
> from `Base` — so narrowing the receiver to `Alpha` is both unnecessary and 
> wrong.
> h2. Trigger conditions
> From bisecting the example, all of the following are required to reproduce:
> 1. The enclosing class is `@CompileStatic`.
> 2. A field (declared with a base/abstract type) is tested with `instanceof` a
> subtype *{*}directly on the field reference{*}* (not via a local copy).
> 3. That `instanceof` expression is returned with an *{*}explicit `return`{*}*
> keyword: `return field instanceof Sub`. Using Groovy's implicit return
> (`field instanceof Sub` as the last expression) does *{*}not{*}* trigger it.
> 4. Another method in the same class calls a method on the same field.
> Only the `return` on the `instanceof` method matters; the second method may 
> use
> either explicit or implicit return.
> h2. Workaround
> Copy the field into a local variable before the `instanceof` check so the
> narrowing applies to the local, not the field:
> ```groovy
> boolean isAlpha() {
> final f = field
> return f instanceof Alpha
> }
> ```
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to