[
https://issues.apache.org/jira/browse/GROOVY-12526?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135534#comment-18135534
]
Paul King commented on GROOVY-12526:
------------------------------------
I checked the merged change against master just before it (5b077f9dcf), running
each snippet in dynamic, @TypeChecked (TC) and @CompileStatic (CS) mode. Some
code that compiled and ran on 6.0.0 now behaves differently, so I'd suggest
reverting this on GROOVY_6_0_X and reworking it on master.
h3. 1. Code that compiled before is now rejected
*Ambiguous overload on a final field assigned in two constructors*
{code:groovy}
class Foo {
final bar
Foo(String s) { bar = s }
Foo(Integer i) { bar = i }
static String m(Object o) { 'Object' }
static String m(Comparable c) { 'Comparable' }
static String m(Serializable s) { 'Serializable' }
String test() { m(bar) }
}
new Foo(1).test()
{code}
|| ||dynamic||TC||CS||
|before|Comparable|Comparable|Object|
|after|Comparable|compile error: _Reference to method is ambiguous_|compile
error: same|
The field is now typed as {{Comparable & Serializable}}, the common type of
String and Integer, which matches two overloads equally. An initializer such as
{{cond ? 'abc' : 42}} gives the same compile error under CS.
*An instanceof test the type checker now calls impossible*
{code:groovy}
class Foo {
final bar = 'abc'
def test() { if (bar instanceof Integer) { return bar + 1 }; bar.toString() }
}
new Foo().test()
{code}
|| ||dynamic||TC||CS||
|before|abc|abc|abc|
|after|abc|compile error: _Incompatible instanceof types: java.lang.String and
java.lang.Integer_|compile error: same|
h3. 2. Statically compiled code chooses a different overload at run time
{code:groovy}
class Foo {
final bar = 'abc'
static String m(Object o) { 'Object' }
static String m(String s) { 'String' }
String test() { m(bar) }
}
new Foo().test()
{code}
|| ||dynamic||TC||CS||
|before|String|String|Object|
|after|String|String|*String*|
{{static final BAR = 'abc'}} read as {{Foo.BAR}} changes the same way. This
brings CS into line with dynamic code, but it changes which overload runs in a
patch release.
h3. 3. The type is wrong when a subclass overrides the getter
{code:groovy}
class A { final x = 'abc' }
class B extends A { def getX() { 42 } }
class U {
static String m(Object o) { "Object:$o" }
static String m(String s) { "String:$s" }
static String t(A a) { m(a.x) }
static int len(A a) { a.x.length() }
}
U.t(new B()) // and U.len(new B())
{code}
|| ||dynamic||TC||CS||
|t, before|Object:42|Object:42|Object:42|
|t, after|Object:42|Object:42|*String:42* (the Integer is passed to m(String))|
|len, before|MissingMethodException|compile error: _Cannot find matching method
Object#length()_|compile error: same|
|len, after|MissingMethodException|MissingMethodException|*compiles, then
throws ClassCastException*: Integer cannot be cast to String|
{{a.x}} calls a getter that a subclass can override, but the flow type assumes
it returns the String from the initializer. Under CS this loses an error the
compiler used to report.
h3. 4. The result depends on which class comes first in the source
{code:groovy}
class Foo { final bar = 'abc' }
class U {
static String m(Object o) { "Object" }
static String m(String s) { "String" }
static String t(Foo f) { m(f.bar) }
}
U.t(new Foo())
{code}
After this change, CS gives {{String}} when {{Foo}} is declared before {{U}},
and {{Object}} when it is declared after.
{{testReaderBeforeFieldClassIsNotFlowTyped}} makes this the intended behaviour,
but it means moving a class within a file changes which overload runs.
h3. Suggestion (one path - still exploring)
* Revert on GROOVY_6_0_X: case 1 breaks code that compiled and ran, and case 3
is unsound.
* On master:
** don't flow-type a field read through a getter that a subclass could
override, such as {{a.x}} from another class;
** decide whether the declaration-order dependence is acceptable;
** add a COMPATIBILITY note for the overload and instanceof changes.
The intended improvement does work. For example, {{Comparable c = bar}}, where
{{bar}} is assigned a String in one constructor and an int in another, now
compiles and runs in TC and CS. {{testConstructorsWithUnrelatedTypesDoNotFlow}}
passes only because {{length()}} isn't available on the common type: the type
does flow, so the test name is misleading.
> I.super.m() is not rejected when I is not a direct superinterface
> -----------------------------------------------------------------
>
> Key: GROOVY-12526
> URL: https://issues.apache.org/jira/browse/GROOVY-12526
> Project: Groovy
> Issue Type: Bug
> Components: Compiler
> Affects Versions: 6.0.0
> Reporter: Scott Murphy Heiberg
> Assignee: Daniel Sun
> Priority: Major
> Fix For: 7.0.0-alpha-1, 6.0.1
>
>
> javac rejects {{I.super.m()}} unless {{I}} is a direct superinterface of the
> current class or interface ("not an enclosing class: I"). Groovy does not
> reject it, and dynamic Groovy runs something else:
> {code:groovy}
> interface J { default String m() { 'J' } }
> interface I extends J {
> default String m() { 'I' }
> default String n() { I.super.m() }
> }
> class C implements I { }
> assert new C().n() == 'I' // I's own m(), not J.m()
> {code}
> {code:groovy}
> interface Outer {
> default String m() { 'outer' }
> class Inner {
> String test() { Outer.super.m() } // Inner does not implement Outer
> }
> }
> new Outer.Inner().test() // GroovyBugError: BUG! UNCAUGHT EXCEPTION: caller
> class must be a subclass below the method: Outer.m()String/invokeSpecial,
> from class Outer$Inner
> {code}
> Under {{@TypeChecked}} and {{@CompileStatic}} both are rejected, but with "No
> such property: super for Class or static property for class: I" (and "...:
> Outer"). Seen in 5.0.8, 6.0.0 and master. In 4.0.33 the first example
> returned 'I' in every mode, and the second failed with
> {{MissingPropertyException}}.
> Expected: a compile error in every mode, as javac gives.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
