This is an interesting one, you are right about the method selection, because
static compiler doesn't know that it's a ListWithDefault, it calls DGM.getAt
instead of ListWithDefault.getAt. I think this is reasonable and static
compiler has selected the proper method.
But, I also agree it could be considered a bug in DGM.getAt, or with
ListWithDefault. If I implement my own class that does the same as withDefault,
it will have the same bug even in dynamic Groovy. The DGM.getAt method checks,
using List.size() method, if the index is valid, if so it calls List.get else
it returns null without trying get. ListWithDefault has an interesting behavior
that get at an index past the end extends the list. The contract of List.get
states that index out of range produces IndexOutOfBoundsException,
ListWithDefault doesn't, so it doesn't obey List's contract.
Here is an example where I am simulating what withDefault does, but from the
perspective of a non-Groovy developer that doesn't know about getAt (i.e.
ListWrapper maybe is implemented in Java or Scala):
class ListWrapper implements List {
@Delegate List delegate
def get(int index) {
if (index >= size())
"past the end"
}
}
x = new ListWrapper(delegate: [])
assert x[1] == null
assert x.get(1) == "past the end"
As I see it, there are two solutions to this:
1. Argue that ListWithDefault is not obeying List contract and have its size
method return Integer.MAX_VALUE
2. DGM.getAt(List, int) should be written to replace the if (index < size)
check with always calling List.get returning null if catching
IndexOutOfBoundsException (or perhaps on any exception).
The second option is definitely the safer and acts like a developer would
expect. I'm not sure try/catch has a significant performance impact in Java
when the exception is not thrown, though. It still can represent a breaking
change if any code relies on get not having been called (but before I saw code
or docs I would expect x[0] on a List type to always attempt x.get(0)).
Jason
-----Original Message-----
From: Jochen Theodorou [mailto:blackd...@gmx.org]
Sent: Wednesday, September 30, 2015 2:05 AM
To: users@groovy.incubator.apache.org
Subject: Re: A @CompileStatic bug, or is it just me: [].withDefault { 0 }
This actually highlights a general problem with static compilation and
extension methods.
Extension methods are not part of the class in the Java sense. Thus there is no
virtual method call. This means that if the compiler assumes the wrong type, it
will call the wrong method. In Java everything is fine as long as the "wrongly"
assumed type is a super type and defines the method. Example:
class Foo {
int foo(){1}
}
class Bar extends Foo {
int foo(){2}
}
def method(Foo f) {
return f.foo()
}
assert method(new Foo()) == 1
assert method(new Bar()) == 2
A static compiler will select Foo#foo() for the call, but since the method call
is done virtual, you still end up doing Bar#foo() if f is Bar. But with
extension methods:
class Foo{}
class ExtensionMethodsForFoo { // placed so compiler knows this
static int foo(Foo f){1}
}
class Bar extends Foo{
int foo(){2}
}
assert method(new Foo()) == 1
assert method(new Bar()) == 1
Again the compiler will only see Foo#foo, because he cannot know better.
But since the call cannot be virtual, there is no way of calling Bar#foo this
way. It would also be the same if Bar#foo is defined as extension method as
well.
In general: inheritance with extension methods does not work properly for
static compiled Groovy. It cannot imho.
For the case here, it might be a workaround to let the withDefault method not
return List (which will let the compiler then later select List#getAt from
DGM), but to use the specific type (ListWithDefault or something). Then the
compiler can select the proper method through flow typing. Of course, as soon
as you moves parts of the code in another method, you may end up with the same
problem. So it is not a really refactoring-safe solution. But better than
nothing I guess.
Anyway... just wanted to confirm you guys why this is no static compilation bug.
Of course you could think about doing double dispatch in the DGM method, but
either you do dynamic calls from there and loose the advantage of static
compilation, or
bye blackdrag
Am 29.09.2015 23:58, schrieb Shil Sinha:
> The bytecode for x[n] is:
>
> ILOAD 1
> INVOKESTATIC
> org/codehaus/groovy/runtime/DefaultGroovyMethods.getAt
> (Ljava/util/List;I)Ljava/lang/Object;
>
> The implementation of DGM.getAt(List, int) returns null if the given
> index is >= the size of the list, so this makes sense. Seems like a
> bug in the DGM implementation more than @CompileStatic.
>
> On Tue, Sep 29, 2015 at 5:17 PM, Søren Berg Glasius <soe...@glasius.dk
> <mailto:soe...@glasius.dk>> wrote:
>
> Should I file a bug report?
>
>
> Best regards / Med venlig hilsen,
> Søren Berg Glasius
>
> Hedevej 1, Gl. Rye, 8680 Ry, Denmark
> Mobile: +45 40 44 91 88 <tel:%2B45%2040%2044%2091%2088>, Skype:
> sbglasius
> --- Press ESC once to quit - twice to save the changes.
>
> On 29 September 2015 at 10:17, Dinko Srkoč <dinko.sr...@gmail.com
> <mailto:dinko.sr...@gmail.com>> wrote:
>
> On 29 September 2015 at 10:12, Cédric Champeau
> <cedric.champ...@gmail.com <mailto:cedric.champ...@gmail.com>>
> wrote:
> > That's because it's withDefault { 1 } ;)
>
> Argh! Still morning for me. :-(
>
> >
> > 2015-09-29 10:05 GMT+02:00 Dinko Srkoč <dinko.sr...@gmail.com
> <mailto:dinko.sr...@gmail.com>>:
> >>
> >> On 29 September 2015 at 09:58, Cédric Champeau
> >> <cedric.champ...@gmail.com
> <mailto:cedric.champ...@gmail.com>> wrote:
> >> > This looks like a bug. Would be interesting to look at the
> bytecode to
> >> > check
> >> > what method is called for x[n].
> >>
> >> Curiously, I tried to do just that in the Groovy AST Browser
> and, for
> >> the following piece of code:
> >>
> >> @groovy.transform.CompileStatic
> >> def foo() {
> >> [].withDefault(1)
> >> }
> >>
> >> got this:
> >>
> >> Unable to produce AST for this phase due to earlier
> compilation error:
> >> startup failed:
> >> script1443513793544.groovy: 3: [Static type checking] -
> Cannot find
> >> matching method java.util.List#withDefault(int). Please
> check if the
> >> declared type is right and if the method exists.
> >> @ line 3, column 5.
> >> [].withDefault(1)
> >> ^
> >>
> >> That's Groovy 2.4.4
> >>
> >> Cheers,
> >> Dinko
> >>
> >> >
> >> > 2015-09-29 9:54 GMT+02:00 Søren Berg Glasius
> <soe...@glasius.dk <mailto:soe...@glasius.dk>>:
> >> >>
> >> >> Hi Fellows,
> >> >>
> >> >> I stumbled upon this today.
> >> >>
> >> >> This code runs:
> >> >>
> >> >> class Test {
> >> >> private List<Integer> x = [].withDefault { 0 }
> >> >> Integer getValue(int n) {
> >> >> return x[n]
> >> >> }
> >> >> }
> >> >> assert new Test().getValue(5) == 0
> >> >>
> >> >> where as when I compile static:
> >> >>
> >> >> @CompileStatic
> >> >> class Test {
> >> >> private List<Integer> x = [].withDefault { 0 }
> >> >> Integer getValue(int n) {
> >> >> return x[n]
> >> >> }
> >> >> }
> >> >> assert new Test().getValue(5) == 0
> >> >>
> >> >> I get an assertion failed, because new Test().getValue(5)
> == null
> >> >>
> >> >> Is this expected behavior or a bug?
> >> >>
> >> >>
> >> >> Best regards / Med venlig hilsen,
> >> >> Søren Berg Glasius
> >> >>
> >> >> Hedevej 1, Gl. Rye, 8680 Ry, Denmark
> >> >> Mobile: +45 40 44 91 88 <tel:%2B45%2040%2044%2091%2088>,
> Skype: sbglasius
> >> >> --- Press ESC once to quit - twice to save the changes.
> >> >
> >> >
> >
> >
>
>
>
--
Jochen "blackdrag" Theodorou
blog: http://blackdragsview.blogspot.com/
----------------------------------------------------------------------
This email message and any attachments are for the sole use of the intended
recipient(s). Any unauthorized review, use, disclosure or distribution is
prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message and any attachments.
