On 04/15/2018 05:02 PM, Ben Coman wrote:
> On 16 April 2018 at 03:07, Denis Kudriashov <dionisi...@gmail.com> wrote:
>>
>>
>>
>> 2018-04-15 20:40 GMT+02:00 Martin McClure <mar...@hand2mouse.com>:
>>>
>>> On 04/15/2018 07:42 AM, Ben Coman wrote:
>>>
>>> The greater prominence of Critiques in Calypso
>>> encourages me to try to clean them out.
>>>
>>> I bumped into a false positive "Temporaries read before written."
>>> that I've condensed to the following example.
>>>
>>>     test
>>>         |x|
>>>         [ x := nil ] whileNil: [ x ifNil: [ x := 1] ]
>>>
>>> Now before I log an Issue, is it feasible to be able to recognise this?
>>> Perhaps only for common looping idioms?
>>>
>>>
>>> In this example, the first runtime reference to x is to send it #ifNil:. So 
>>> technically, x *is* being read before being written, at least if you count 
>>> sending it a message as "reading" (which seems a reasonable interpretation 
>>> to me).
>>
>>
>> Maybe critique can be a bit smarter and check that sent message is known by 
>> nil.
> 
> That sounds like a nice approach.  Browsing the messages of Undefined,
> maybe its worthwhile to restrict this to messages containing the the
> string "nil", because this shows explicit intent and would cover the
> most common//useful cases.
> https://pharo.manuscript.com/f/cases/21704/Critiques-Temporaries-read-before-written-versus-nil-checking-messages

You may break extract method if you change this. I haven't tried, but I
think something like this would break -- extract the last line of this
method:

method
        | x |
        x := self someValue.
        x isNil ifTrue: [Transcript show: 'nil']

You should get something like this:

method
        | x |
        x := self someValue.
        self extractedMethod: x

extractedMethod: x
        x isNil ifTrue: [Transcript show: 'nil']

However, if you change what is considered read before written, then you
would get something like this:

method
        | x |
        x := self someValue.
        self extractedMethod

extractedMethod
        | x |
        x isNil ifTrue: [Transcript show: 'nil']


John Brant

Reply via email to