groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mauro Molinari <mauro...@tiscali.it>
Subject Re: Is FieldsAndPropertiesStaticCompileTest#testUseGetterFieldAccess really correct?
Date Fri, 24 Nov 2017 12:09:51 GMT
I think this is a quite "grey" area of Groovy, at least it took a while 
for me to understand getter/setter vs direct field access in different 
cases when I faced it and some fixes to the Groovy plugin for Eclipse 
were also necessary to properly support code highlighting and navigation.
Now you're going further in the analysis by adding inheritance and 
member visibility to the picture. IMHO Groovy should define a coherent 
and consistent policy for this and proper documentation should be provided.

Just one question: when you talk about "this.x and super.x", you also 
mean the case in which "this." is implicit, don't you?

In any case, IMHO the @CompileStatic case should behave exactly like 
@CompileDynamic, absolutely!

Mauro

Il 24/11/2017 10:16, Jochen Theodorou ha scritto:
>
>
> Am 24.11.2017 um 09:09 schrieb Mauro Molinari:
>> Il 24/11/2017 01:46, Jochen Theodorou ha scritto:
>>> In my opinion the test is wrong, but I'd like to hear others about 
>>> this.
>>>
>>> And another point. We seem to have no similar test for dynamic 
>>> Groovy. Groovy does use direct field access if the field is 
>>> available on "this". But the question here is if that extends to all 
>>> super classes. In my opinion it should.
>>>
>>> If I get no vetos I will push a fix for this for all current groovy 
>>> versions 
>>
>> I don't know if I understood it right, but are you saying that you 
>> think that invoking b.usingGetter() in your example should not call 
>> A.getX(), but rather access A.x directly? And that, I guess, the same 
>> policy should apply for the corresponding setter?
>
> yes, just that we do not have a test for this.
>
>> I guess you think so because A.x is declared as protected, don't you? 
>> Because if A.x had no modifier, I think that such a change may break 
>> a lot of code that assumes getter/setter are called, in particular 
>> I'm thinking of the write access case for @Bindable fields
> I am thinking so because in
>
>> class X {
>>   def x
>>   def foo(){x}
>> }
>
> we will access the field directly. It will access the field directly, 
> because the field is accessible. In
>
>> class X {
>>   def x
>> }
>> class Y extends X {
>>   def foo(){x}
>> }
>
> the field itself is not accessible and the getter/setter must be used. 
> But in
>
>>  class X {
>>    public x
>>    def getX(){x}
>>  }
>>  class Y extends X {
>>    def foo(){x}
>>  }
>
> the field is accessible, thus I think the access should be done directly.
>
> And then there is the following to consider as well:
>
>>  class X {
>>    public x
>>    def getX(){x}
>>  }
>>  class Y extends X {
>>    def foo(){super.x}
>>  }
>
> This will access the field directly in todays implementation. And this 
> here
>
>> ​ class X {      public x=1
>>    def getX(){x+10}
>>  }
>>  class Y extends X {}
>>  class Z extends Y {
>>    def foo(){super.x}
>>  }
>
> will not access the field directly in todays implementation. And if 
> the field is private or does not exist even super.x would always use 
> the getter, as would this.x
>
> I think the rules should be the following:
>
> this.x and super.x should access the field if the field is accessible. 
> If the field is not accessible, then x has to be accessed as property 
> (which would prefer the getter/setter over the field)
>
> Accessible here means for me first and for all, if the field is 
> defined in the same class, then for this.x the field is always 
> accessible, regardless of any modifier.  If the field is from a super 
> class, the field must be public or protected. Inaccessible fields are 
> to be ignored. Example:
>
>>  ​ class X {       public x
>>     def getX(){x}
>>   }
>>   class Y extends X {
>>     private x
>>   }
>>   class Z extends Y {
>>     def foo(){super.x}
>>   }
>
> should also access the field in X, same for this.x instead of super.x
>
>
> The current implementation feels inconsistent to me. Will this break 
> code? that may very well be. But I think the chance is not that big.
>
> And then finally there is the question of if we do not want this, do 
> we want it different for @CompileStatic?
>
> bye Jochen
>
>


Mime
View raw message