On 04/15/2018 05:02 PM, Ben Coman wrote:
On 16 April 2018 at 03:07, Denis Kudriashov <dionisiydk@gmail.com> wrote:
2018-04-15 20:40 GMT+02:00 Martin McClure <martin@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...
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