groovy-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul King <pa...@asert.com.au>
Subject Re: @Immutable backwards compatibility
Date Wed, 26 Sep 2018 23:24:00 GMT
The String check for "groovy.transform.Immutable" would work fine if the
2.4 compiled class was actually loaded with it;s annotations but
AnnotationCollector changes any meta-annotation to no longer be an
annotation but a class to store the serialized information for the
pre-compiled case. I think we need to perhaps adjust what we do there
slightly to leave the original annotation intact. I'll try to work on that
on the plane trip home - perhaps we should delay 2.5.3 a few days to see if
we can address this but any alternative suggestions welcome.

On Thu, Sep 27, 2018 at 8:14 AM Jochen Theodorou <blackdrag@gmx.org> wrote:

> On 26.09.2018 12:58, Paul King wrote:
> > I shouldn't try to respond to emails while rushing between conference
> > sessions. Refreshed my memory and yes, the current provisions for 2.4
> > compatibility don't really help. I'll see if Jochen has some ideas on
> > how we could improve that.
>
> I guess we have to compare
>
>
> https://github.com/apache/groovy/blob/e16728b91601573ee18475ec5dee5355817f670c/src/main/org/codehaus/groovy/transform/ImmutableASTTransformation.java#L738
>
> and
>
>
> https://github.com/apache/groovy/blob/9914ae4a0b4273a5a4cb8822699a9b2dbb3c3215/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java#L352
>
> which tells me the knownImmutableClasses part is gone from @Immutable
> and the 2.5.x version has no way of getting this information anymore,
> since this is supposed to be given directly to the method.
>
> Bad, but let's continue
>
> https://github.com/ajoberstar/grgit/issues/237 talks about for example
> the Commit.groovy, that can be found here:
>
> https://github.com/ajoberstar/grgit/blob/master/grgit-core/src/main/groovy/org/ajoberstar/grgit/Commit.groovy
>
> it does have a knownImmutableClasses=[ZonedDateTime], but looking at
>
> https://github.com/apache/groovy/blob/GROOVY_2_5_X/src/main/java/org/apache/groovy/ast/tools/ImmutablePropertyUtils.java#L101
> it is in the list of builtinImmutables.
>
> And the error message is of course not about that, it is about Person:
>
> https://github.com/ajoberstar/grgit/blob/master/grgit-core/src/main/groovy/org/ajoberstar/grgit/Person.groovy
>
> And that is where I am actually stuck in understanding the issue...
>
> > if (field == null || field instanceof Enum ||
> builtinOrMarkedImmutableClass(field.getClass())) {
> >             return field;
> > }
>
> this code for the 2.4 check should have returned, because
> builtinOrMarkedImmutableClass checks if the class of the value for the
> field has an annotation named groovy.transform.Immutable, which is the
> case for Person (see
>
> https://github.com/apache/groovy/blob/9914ae4a0b4273a5a4cb8822699a9b2dbb3c3215/src/main/java/org/apache/groovy/ast/tools/ImmutablePropertyUtils.java#L194
> )
>
> It is similar for Branch.groovy...
>
> So what exactly is not working anymore? I am confused (and it is well
> too late here for looking into such an issue). Cedric, Paul, can you
> explain what exactly goes wrong?
>
> bye Jochen
>

Mime
View raw message