- Pharo-dev - lists.pharo.org

[Pharo 7.0-dev] Build #766: 21688-OSWindowDriver-is-not-correcly-cleaned-on-shutdown

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #766 was: SUCCESS. The Pull Request #1191 was integrated: "21688-OSWindowDriver-is-not-correcly-cleaned-on-shutdown" Pull request url: https://github.com/pharo-project/pharo/pull/1191 Issue Url: https://pharo.fogbugz.com/f/cases/21688 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

April 16, 2018

Esteban's ChangeLog week of 9 April 2018

by estebanlm＠gmail.com

Hello! This is my weekly ChangeLog, from 9 April 2018 to 15 April 2018. You can see it in a better format by going here: http://log.smallworks.eu/web/search?from=9/4/2018&to=15/4/2018 ChangeLog ========= 13 April 2018: -------------- * I just promoted new VMs as stable. This are the builds corresponding to: 201804122226 (07c6dc3) this passed the build tests so 'it should' be ok :) 12 April 2018: -------------- * on iceberg, closed [https://github.com/pharo-vcs/iceberg/issues/628](https://github.com/pharo-v… to fix a compatibility error. 11 April 2018: -------------- * Ok, while waiting for the Pharo CI to comeback to life (so I can release Iceberg 0.7), I managed to fix a small issue it was annoying me since a lot of time: [https://github.com/estebanlm/opensmalltalk-vm/issues/4](https://github.com/… . This is because I still want to produce real headlesss VMs so I need to slowly move things in that direction. Anyway... since this was an image-side problem more than a VM problem, I have a [case: 21688](https://pharo.fogbugz.com/f/cases/21688) in Pharo issue tracker and a [PR](https://github.com/pharo-project/pharo/pull/1191) 9 April 2018: ------------- * I just finished iceberg [issue 594](https://github.com/pharo-vcs/iceberg/issues/594) : new Iceberg 0.7 (to be released) to load also on Pharo 6.1. cheers! Esteban

April 16, 2018

Re: [Pharo-dev] Critiques - Temporaries read before written.

by Ben Coman

On 16 April 2018 at 03:07, Denis Kudriashov <dionisiydk(a)gmail.com> wrote: > > > > 2018-04-15 20:40 GMT+02:00 Martin McClure <martin(a)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-befor… cheers -ben

April 15, 2018

Re: [Pharo-dev] Critiques - Temporaries read before written.

by Eliot Miranda

Hi Martin, > On Apr 15, 2018, at 11:40 AM, Martin McClure <martin(a)hand2mouse.com> wrote: > >> 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). How so? The first run-time reference to xcould be in either of the two non-unlined blocks. The assignment to x in [ x := nil ] could precede the read if (as of course it does, but we can't guarantee) BlockClosure>>whileNil: chooses to evaluate it before [ x ifNil: [ x := 1] ]. The issue is that the code is ambiguous; it depends on the implementation of whileNil: and hence should be reported as "may" rather than "shall". >> >> Anyway, the workaround is simple enough... >> test >> |x| >> x := nil. "silence critiques" >> [ x := nil ] whileNil: [ x ifNil: [ x := 1] ] > Probably not a terrible idea to be explicit about initializing to nil, thereby revealing the developer's intent that this variable be nil rather than relying on the default initialization to nil. I've never agreed with this. It is in the language spec that all pointer variables, including temporaries, are initialized with nil, and repeating an initialization shows ignorance, and makes me suspect the rest of the code. Smalltalk intentionally ensures that all variables are initialised and for good reason. It's similar, but worse, than people adding an explicit ifFalse: [nil] to an ifTrue: when the default return value for a false ifTrue: is indeed nil. We should strive for literacy, not bastardise things for the ignorant since repeating a mistake is a route to establishing the mistake as common parlance hence losing elegance and concision. > So I'd lean towards leaving the critique as-is. If a developer knows what they did was safe, they can either ignore the critique, exclude the critique, or put in the explicit initialization to nil. I think I prefer explicit initialization. The critique is wrong. It can only say "may be read before written" and a human being in the same situation would soon observe that the receiver block is always evaluated before the argument block and so not earn at all. Benis right; this is a false positive. However, saying "may" is at least in the spirit of the language given the possibility of reimplementing whileNil:. > > Regards, > -Martin

April 15, 2018

Re: [Pharo-dev] Critiques - Temporaries read before written.

by John Brant

> On Apr 15, 2018, at 9:42 AM, Ben Coman <btc(a)openinworld.com> 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? In general one would need to look at the implementor to know which blocks are always evaluated and in what order. For example, someone could have defined whileNil: as: whileNil: aBlock aBlock value In this case, it isnât a false positive since the "x := nilâ is never executed and the first use of âxâ is the âx ifNil: â¦â. > Perhaps only for common looping idioms? It does look for common looping idioms. It treats the four whileTrue/False messages different than normal messages. It also treats ifTrue:ifFalse:/ifFalse:ifTrue: messages differently. #whileNil: isnât what I call a common looping idiom as it seems to have been added in 2007 (about 13 years after the read before written tester was written), and has 0 senders in my image. I think a better change would be adding knowledge about and:/or: combined with ifTrue:, whileTrue:, etc. For example, (self someCondition and: [(x := self value) > 0]) ifTrue: [Transcript show: x printString] In this case, x is reported as potentially read before written, but it canât be. John Brant

April 15, 2018

Re: [Pharo-dev] Critiques - Temporaries read before written.

by Denis Kudriashov

2018-04-15 20:40 GMT+02:00 Martin McClure <martin(a)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. > > > Anyway, the workaround is simple enough... > test > |x| > x := nil. "silence critiques" > [ x := nil ] whileNil: [ x ifNil: [ x := 1] ] > > Probably not a terrible idea to be explicit about initializing to nil, > thereby revealing the developer's intent that this variable be nil rather > than relying on the default initialization to nil. > > So I'd lean towards leaving the critique as-is. If a developer knows what > they did was safe, they can either ignore the critique, exclude the > critique, or put in the explicit initialization to nil. I think I prefer > explicit initialization. > > Regards, > -Martin >

April 15, 2018

Re: [Pharo-dev] Critiques - Temporaries read before written.

by Martin McClure

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). > > Anyway, the workaround is simple enough... > Â Â test > Â Â Â Â |x| > Â Â Â Â x := nil. "silence critiques" > Â Â Â Â [ x := nil ] whileNil: [ x ifNil: [ x := 1] ] Probably not a terrible idea to be explicit about initializing to nil, thereby revealing the developer's intent that this variable be nil rather than relying on the default initialization to nil. So I'd lean towards leaving the critique as-is. If a developer knows what they did was safe, they can either ignore the critique, exclude the critique, or put in the explicit initialization to nil. I think I prefer explicit initialization. Regards, -Martin

April 15, 2018

Re: [Pharo-dev] Critiques - Temporaries read before written.

by Cyril Ferlicot

On dim. 15 avr. 2018 at 16:43, Ben Coman <btc(a)openinworld.com> 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? > > Anyway, the workaround is simple enough... > Hi Ben, I do not have an answer regarding the variables written in the blocks, but for false positive you can also just exclude a rule for the scope of the method, class, package and image IIRC. There was a presentation at esug about it. I'm on phone and it's hard to search for the link currently. test > |x| > x := nil. "silence critiques" > [ x := nil ] whileNil: [ x ifNil: [ x := 1] ] > > cheers -ben > -- Cyril Ferlicot https://ferlicot.fr

April 15, 2018

Critiques - Temporaries read before written.

by Ben Coman

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? Anyway, the workaround is simple enough... test |x| x := nil. "silence critiques" [ x := nil ] whileNil: [ x ifNil: [ x := 1] ] cheers -ben

April 15, 2018

DelayScheduler cleanup & Bootstrap

by Ben Coman

I am doing a pass to cleanup the DelayScheduler hierarchy. I'll be refactoring of the hierarchy to aid code understanding and eliminate redundant instance variables from the original code. The target structure is.... DelayNullScheduler -- minimum interface to avoid errors in rest of system (6 empty methods) |__DelayBasicScheduler -- fully working sans race protection (equivalent to Courageous scheduler) |__DelaySpinScheduler -- plus race protection Can someone advise how/where the Bootstrap loads Delay and the DelaySchedulers? where the invocation of Delay_class>>#initialize selects the delay scheduler. I need to determine whether during transition the Bootstrap might need to temporarily hop to a different scheduler, and how to provide the new structure in the Image for operational testing before switching. cheers -ben

April 15, 2018