Whilst using Monticello’s ‘browse’ option to look at some code I tripped over a problem I just knew was going to be annoying someday - Sets with actual nils included as elements. That means that any enumeration has to explicitly handle the possibility of an item being a nil, instead of the normal and proper expectation that nils mean nothing and don’t get involved in Set>do:.
So as a result in SHParserST80>resolvePartial: we have c sharedPools do: [:p | (p hasBindingThatBeginsWith: aString) ifTrue: [^#incompleteIdentifier]]. where c is an SHMCClassDefinition and the result of the #sharedPools should be a properly empty set. However, SHMCClassDefinition>sharedPools | d | d := Set new. classDefinition poolDictionaries do:[:each | d add: (Smalltalk at: each asSymbol ifAbsent:[nil]) ]. ^d … carefully adds a nil to the Set. So we get ‘nil hasBinding…’ and kaboom. *If* I assume that the real expectation is for the shared pools list to be like those elsewhere and not include nils, then a simple fix to | d | d := Set new. classDefinition poolDictionaries do:[:each | Smalltalk at: each asSymbol ifPresent:[:v| d add: v] ]. ^d … appears to make things happier. However, I don’t know anything about the intended working of Shout and I’d rather not simply stomp around in here. Also the code as-was has been around since 2010 so maybe something else has more recently changed and this is not the right thing to fix. Committed to inbox in the hope someone au fait with the code can look into it. tim -- tim Rowledge; [hidden email]; http://www.rowledge.org/tim Don't document the program; program the document. |
On Wed, Aug 16, 2017 at 03:31:40PM -0700, tim Rowledge wrote:
> Whilst using Monticello???s ???browse??? option to look at some code I tripped over a problem I just knew was going to be annoying someday - Sets with actual nils included as elements. That means that any enumeration has to explicitly handle the possibility of an item being a nil, instead of the normal and proper expectation that nils mean nothing and don???t get involved in Set>do:. > > So as a result in SHParserST80>resolvePartial: we have > c sharedPools do: [:p | (p hasBindingThatBeginsWith: aString) ifTrue: [^#incompleteIdentifier]]. > where c is an SHMCClassDefinition and the result of the #sharedPools should be a properly empty set. > > However, SHMCClassDefinition>sharedPools > | d | > d := Set new. > classDefinition poolDictionaries do:[:each | > d add: (Smalltalk at: each asSymbol ifAbsent:[nil]) ]. > ^d > > ??? carefully adds a nil to the Set. So we get ???nil hasBinding?????? and kaboom. > > *If* I assume that the real expectation is for the shared pools list to be like those elsewhere and not include nils, then a simple fix to > | d | > d := Set new. > classDefinition poolDictionaries do:[:each | > Smalltalk at: each asSymbol ifPresent:[:v| d add: v] ]. > ^d > ??? appears to make things happier. However, I don???t know anything about the intended working of Shout and I???d rather not simply stomp around in here. Also the code as-was has been around since 2010 so maybe something else has more recently changed and this is not the right thing to fix. > > Committed to inbox in the hope someone au fait with the code can look into it. I have no particular expertise here, but your fix looks good. The original code seem to be abusing its knowledge that the implementation of Set would silently ignore an object added to the set if the object happens to be an instance of UndefinedObject. So changing the method so that it no longer assumes that adding nil is the same thing as not adding anything at all seems right. Dave |
Free forum by Nabble | Edit this page |