[jira] [Commented] (GROOVY-8938) Final field caching issues in inherited field.
[
https://issues.apache.org/jira/browse/GROOVY-8938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16728794#comment-16728794
]
Anton Pryamostanov commented on GROOVY-8938:
Many thanks for the feedback, guys!
The issue can be closed, there are no immediate actions required.
Once there is time to look on any refactoring (if needed), this can be looked
again onto.
> Final field caching issues in inherited field.
> --
>
> Key: GROOVY-8938
> URL: https://issues.apache.org/jira/browse/GROOVY-8938
> Project: Groovy
> Issue Type: Bug
> Components: groovy-runtime
>Affects Versions: 2.5.4
>Reporter: Anton Pryamostanov
>Priority: Critical
>
> Note: **this seems to be a quite major issue**, kindly look at this one with
> emphasis.
> Please see the below test case:
> SubClass extends the SuperClass and overrides the field "inheritedField".
> {code}
> class OtherClass {
> String otherField
> OtherClass(String otherField) {
> this.otherField = otherField
> }
> String toString() {
> return otherField
> }
> }
> class SuperClass {
> final OtherClass inheritedField = new OtherClass("Super Class String")
> }
> class SubClass extends SuperClass {
> final OtherClass inheritedField = new OtherClass("Sub Class String")
> }
> SuperClass superClass = new SuperClass()
> SubClass subClass = new SubClass()
> assert subClass.inheritedField.toString() == "Sub Class String"
> {code}
> *Actual result*: assertion fails
> *Expected result*: assertion should pass
> *Note*: If "final" modifier is removed, assertion passes as expected.
> This seems an issue with final field caching.
> Assertion error text:
> {code}
> Assertion failed:
> assert subClass.inheritedField.toString() == "Sub Class String"
>|| | |
>|| | false
>|| 'Super Class String'
>|Super Class String
>SubClass@5b4b841f
> at ConsoleScript11.run(ConsoleScript11:18)
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GROOVY-8938) Final field caching issues in inherited field.
[
https://issues.apache.org/jira/browse/GROOVY-8938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16728781#comment-16728781
]
Anton Pryamostanov commented on GROOVY-8938:
Hi [~paulk], [~blackdrag], I sent you private message to Slack (in Groovy
space).
Can you check? Or please suggest non-public way to discuss on this issue (I
have some considerations that will become clear).
> Final field caching issues in inherited field.
> --
>
> Key: GROOVY-8938
> URL: https://issues.apache.org/jira/browse/GROOVY-8938
> Project: Groovy
> Issue Type: Bug
> Components: groovy-runtime
>Affects Versions: 2.5.4
>Reporter: Anton Pryamostanov
>Priority: Critical
>
> Note: **this seems to be a quite major issue**, kindly look at this one with
> emphasis.
> Please see the below test case:
> SubClass extends the SuperClass and overrides the field "inheritedField".
> {code}
> class OtherClass {
> String otherField
> OtherClass(String otherField) {
> this.otherField = otherField
> }
> String toString() {
> return otherField
> }
> }
> class SuperClass {
> final OtherClass inheritedField = new OtherClass("Super Class String")
> }
> class SubClass extends SuperClass {
> final OtherClass inheritedField = new OtherClass("Sub Class String")
> }
> SuperClass superClass = new SuperClass()
> SubClass subClass = new SubClass()
> assert subClass.inheritedField.toString() == "Sub Class String"
> {code}
> *Actual result*: assertion fails
> *Expected result*: assertion should pass
> *Note*: If "final" modifier is removed, assertion passes as expected.
> This seems an issue with final field caching.
> Assertion error text:
> {code}
> Assertion failed:
> assert subClass.inheritedField.toString() == "Sub Class String"
>|| | |
>|| | false
>|| 'Super Class String'
>|Super Class String
>SubClass@5b4b841f
> at ConsoleScript11.run(ConsoleScript11:18)
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GROOVY-8938) Final field caching issues in inherited field.
[
https://issues.apache.org/jira/browse/GROOVY-8938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16728687#comment-16728687
]
Paul King commented on GROOVY-8938:
---
As Jochen mentions, the current behavior is mainly for historical reasons. If
you add a modifier like 'synchronized' to the property, it probably makes sense
to also add it to the getter but it's less clear for something like final.
Similarly, there was lots of debate historically around annotations. An
annotation on a property might make sense to be copied over to the field or
just the getter or setter or to all places. The current approach has been to
use a simple copying strategy for all modifiers and annotations. Even though it
was never seen as ideal we hadn't defined a better approach to date.
> Final field caching issues in inherited field.
> --
>
> Key: GROOVY-8938
> URL: https://issues.apache.org/jira/browse/GROOVY-8938
> Project: Groovy
> Issue Type: Bug
> Components: groovy-runtime
>Affects Versions: 2.5.4
>Reporter: Anton Pryamostanov
>Priority: Critical
>
> Note: **this seems to be a quite major issue**, kindly look at this one with
> emphasis.
> Please see the below test case:
> SubClass extends the SuperClass and overrides the field "inheritedField".
> {code}
> class OtherClass {
> String otherField
> OtherClass(String otherField) {
> this.otherField = otherField
> }
> String toString() {
> return otherField
> }
> }
> class SuperClass {
> final OtherClass inheritedField = new OtherClass("Super Class String")
> }
> class SubClass extends SuperClass {
> final OtherClass inheritedField = new OtherClass("Sub Class String")
> }
> SuperClass superClass = new SuperClass()
> SubClass subClass = new SubClass()
> assert subClass.inheritedField.toString() == "Sub Class String"
> {code}
> *Actual result*: assertion fails
> *Expected result*: assertion should pass
> *Note*: If "final" modifier is removed, assertion passes as expected.
> This seems an issue with final field caching.
> Assertion error text:
> {code}
> Assertion failed:
> assert subClass.inheritedField.toString() == "Sub Class String"
>|| | |
>|| | false
>|| 'Super Class String'
>|Super Class String
>SubClass@5b4b841f
> at ConsoleScript11.run(ConsoleScript11:18)
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GROOVY-8938) Final field caching issues in inherited field.
[
https://issues.apache.org/jira/browse/GROOVY-8938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16728676#comment-16728676
]
Jochen Theodorou commented on GROOVY-8938:
--
[~anton.pryamostanov] why is it "final", because we once said all modifiers
that on the "property" should be applied to the getter/setter if possible. But
it is indeed a question of it that should be done for final. Following the
logic of a field I would say the getter should not be final. But I think that
will start breaking things in other cases like @Immutable? If we keep the final
on the method we should fail compilation in your example though... but then how
would you work around it? By declaring it as field. If you declare it as field,
then what will
assert subClass.inheritedField.toString() == "Sub Class String"
give you? Afair you would still access the super class and get still a failing
assert. And of course you could do,
assert subClass.@inheritedField.toString() == "Sub Class String"
but that would work already.
Coming back to your suggestion it looks like a good idea, but it will make
declaring immutables much more difficult. So before we do a change here I would
suggest to first ensure the story for these is sound.
> Final field caching issues in inherited field.
> --
>
> Key: GROOVY-8938
> URL: https://issues.apache.org/jira/browse/GROOVY-8938
> Project: Groovy
> Issue Type: Bug
> Components: groovy-runtime
>Affects Versions: 2.5.4
>Reporter: Anton Pryamostanov
>Priority: Critical
>
> Note: **this seems to be a quite major issue**, kindly look at this one with
> emphasis.
> Please see the below test case:
> SubClass extends the SuperClass and overrides the field "inheritedField".
> {code}
> class OtherClass {
> String otherField
> OtherClass(String otherField) {
> this.otherField = otherField
> }
> String toString() {
> return otherField
> }
> }
> class SuperClass {
> final OtherClass inheritedField = new OtherClass("Super Class String")
> }
> class SubClass extends SuperClass {
> final OtherClass inheritedField = new OtherClass("Sub Class String")
> }
> SuperClass superClass = new SuperClass()
> SubClass subClass = new SubClass()
> assert subClass.inheritedField.toString() == "Sub Class String"
> {code}
> *Actual result*: assertion fails
> *Expected result*: assertion should pass
> *Note*: If "final" modifier is removed, assertion passes as expected.
> This seems an issue with final field caching.
> Assertion error text:
> {code}
> Assertion failed:
> assert subClass.inheritedField.toString() == "Sub Class String"
>|| | |
>|| | false
>|| 'Super Class String'
>|Super Class String
>SubClass@5b4b841f
> at ConsoleScript11.run(ConsoleScript11:18)
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GROOVY-8938) Final field caching issues in inherited field.
[
https://issues.apache.org/jira/browse/GROOVY-8938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16728597#comment-16728597
]
Anton Pryamostanov commented on GROOVY-8938:
Hi [~paulk],
Thank you for the feedback!
Question: why the synthetic getter has to be final, in case field is final?
I don't see direct dependency here. Wouldn't it be rather safe redesign to just
make getters non-final in this case?
> Final field caching issues in inherited field.
> --
>
> Key: GROOVY-8938
> URL: https://issues.apache.org/jira/browse/GROOVY-8938
> Project: Groovy
> Issue Type: Bug
> Components: groovy-runtime
>Affects Versions: 2.5.4
>Reporter: Anton Pryamostanov
>Priority: Critical
>
> Note: **this seems to be a quite major issue**, kindly look at this one with
> emphasis.
> Please see the below test case:
> SubClass extends the SuperClass and overrides the field "inheritedField".
> {code}
> class OtherClass {
> String otherField
> OtherClass(String otherField) {
> this.otherField = otherField
> }
> String toString() {
> return otherField
> }
> }
> class SuperClass {
> final OtherClass inheritedField = new OtherClass("Super Class String")
> }
> class SubClass extends SuperClass {
> final OtherClass inheritedField = new OtherClass("Sub Class String")
> }
> SuperClass superClass = new SuperClass()
> SubClass subClass = new SubClass()
> assert subClass.inheritedField.toString() == "Sub Class String"
> {code}
> *Actual result*: assertion fails
> *Expected result*: assertion should pass
> *Note*: If "final" modifier is removed, assertion passes as expected.
> This seems an issue with final field caching.
> Assertion error text:
> {code}
> Assertion failed:
> assert subClass.inheritedField.toString() == "Sub Class String"
>|| | |
>|| | false
>|| 'Super Class String'
>|Super Class String
>SubClass@5b4b841f
> at ConsoleScript11.run(ConsoleScript11:18)
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GROOVY-8938) Final field caching issues in inherited field.
[
https://issues.apache.org/jira/browse/GROOVY-8938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16728545#comment-16728545
]
Paul King commented on GROOVY-8938:
---
This is a little convoluted but is kind of working as designed. The default
behavior for final Groovy properties is to create a final getter. Then, rather
than complaining that you are trying to override a final getter in the sub
class, Groovy sees that you have an existing getter and doesn't complain nor
provide a new one (since an existing one is found). This design has a few
compromises and should arguably be re-worked but it has been around a long time
and there are several ways to work with (get around?) the current conventions.
(1) Make the properties non-final.
{code}
class SuperClass {
int inheritedField = 42
}
class SubClass extends SuperClass {
int inheritedField = 21
}
SuperClass superClass = new SuperClass()
assert superClass.inheritedField == 42
SubClass subClass = new SubClass()
assert subClass.inheritedField == 21
{code}
(2) Make the fields public so they are not properties and the conventions won't
apply:
{code:java}
class SuperClass {
public final int inheritedField = 42
}
class SubClass extends SuperClass {
public final int inheritedField = 21
}
SuperClass superClass = new SuperClass()
SubClass subClass = new SubClass()
assert subClass.inheritedField == 21
{code}
(3) Make the super property not final and have an explicit getter in the
subclass:
{code:java}
class SuperClass {
int inheritedField = 42
}
class SubClass extends SuperClass {
public final int inheritedField = 21
int getInheritedField() {
inheritedField
}
}
SuperClass superClass = new SuperClass()
SubClass subClass = new SubClass()
assert subClass.inheritedField == 21
{code}
(4) Do all the getters manually:
{code:java}
class SuperClass {
private final int inheritedField = 42
int getInheritedField() {
inheritedField
}
}
class SubClass extends SuperClass {
private final int inheritedField = 21
int getInheritedField() {
inheritedField
}
}
SuperClass superClass = new SuperClass()
SubClass subClass = new SubClass()
assert subClass.inheritedField == 21
{code}
But all of these shadow the field which is often considered a slight code
smell. If that worries you, consider (5), just use an inherited protected field:
{code:java}
class SuperClass {
protected final int inheritedField
SuperClass(field) { inheritedField = field }
SuperClass() { this(42) }
}
class SubClass extends SuperClass {
SubClass() { super(21) }
}
SuperClass superClass = new SuperClass()
assert superClass.inheritedField == 42
SubClass subClass = new SubClass()
assert subClass.inheritedField == 21
{code}
> Final field caching issues in inherited field.
> --
>
> Key: GROOVY-8938
> URL: https://issues.apache.org/jira/browse/GROOVY-8938
> Project: Groovy
> Issue Type: Bug
> Components: groovy-runtime
>Affects Versions: 2.5.4
>Reporter: Anton Pryamostanov
>Priority: Critical
>
> Note: **this seems to be a quite major issue**, kindly look at this one with
> emphasis.
> Please see the below test case:
> SubClass extends the SuperClass and overrides the field "inheritedField".
> {code}
> class OtherClass {
> String otherField
> OtherClass(String otherField) {
> this.otherField = otherField
> }
> String toString() {
> return otherField
> }
> }
> class SuperClass {
> final OtherClass inheritedField = new OtherClass("Super Class String")
> }
> class SubClass extends SuperClass {
> final OtherClass inheritedField = new OtherClass("Sub Class String")
> }
> SuperClass superClass = new SuperClass()
> SubClass subClass = new SubClass()
> assert subClass.inheritedField.toString() == "Sub Class String"
> {code}
> *Actual result*: assertion fails
> *Expected result*: assertion should pass
> *Note*: If "final" modifier is removed, assertion passes as expected.
> This seems an issue with final field caching.
> Assertion error text:
> {code}
> Assertion failed:
> assert subClass.inheritedField.toString() == "Sub Class String"
>|| | |
>|| | false
>|| 'Super Class String'
>|Super Class String
>SubClass@5b4b841f
> at ConsoleScript11.run(ConsoleScript11:18)
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
