On 04/15/2018 05:02 PM, Ben Coman wrote:
> On 16 April 2018 at 03:07, Denis Kudriashov <[email protected]> wrote:
>>
>>
>>
>> 2018-04-15 20:40 GMT+02:00 Martin McClure <[email protected]>:
>>>
>>> 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
