The Trunk: Collections-ul.564.mcz

> Chris Muller uploaded a new version of Collections to project The Trunk:
> http://source.squeak.org/trunk/Collections-ul.564.mcz
>
> ==================== Summary ====================
>
> Name: Collections-ul.564
> Author: ul
> Time: 3 February 2014, 2:35:36.802 pm
> UUID: 4b9a37ef-df86-40a0-a0dd-8e8b2c04d4ed
> Ancestors: Collections-ul.563
>
> Make sure that Array >> #isLiteral won't get into an infinite recursion, even if the receiver has an recursive structure.
>
> =============== Diff against Collections-ul.563 ===============
>
> Item was changed:
>   ----- Method: Array>>isLiteral (in category 'testing') -----
>   isLiteral
> +
> +       ^self class == Array and: [
> +               self isLiteralIfContainedBy: IdentitySet new ]!
> -       ^self class == Array and: [self allSatisfy: [:each | each isLiteral]]!
>
> Item was added:
> + ----- Method: Array>>isLiteralIfContainedBy: (in category 'testing') -----
> + isLiteralIfContainedBy: parents
> +       " Answer whether the receiver has a literal text form recognized by the compiler. Precondition: the receiver is an instance of Array. "
> +
> +       (parents includes: self) ifTrue: [ ^false ].
> +       parents add: self.
> +       1 to: self size do: [ :index |
> +               | element |
> +               element := self at: index.
> +               (element class == Array
> +                       ifTrue: [ element isLiteralIfContainedBy: parents ]
> +                       ifFalse: [ element isLiteral ]) ifFalse: [ ^false ] ].
> +       parents remove: self.
> +       ^true!

> On 3 February 2014 16:18,  <[hidden email]> wrote:
>> Chris Muller uploaded a new version of Collections to project The Trunk:
>> http://source.squeak.org/trunk/Collections-ul.564.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Collections-ul.564
>> Author: ul
>> Time: 3 February 2014, 2:35:36.802 pm
>> UUID: 4b9a37ef-df86-40a0-a0dd-8e8b2c04d4ed
>> Ancestors: Collections-ul.563
>>
>> Make sure that Array >> #isLiteral won't get into an infinite recursion, even if the receiver has an recursive structure.
>>
>> =============== Diff against Collections-ul.563 ===============
>>
>> Item was changed:
>>   ----- Method: Array>>isLiteral (in category 'testing') -----
>>   isLiteral
>> +
>> +       ^self class == Array and: [
>> +               self isLiteralIfContainedBy: IdentitySet new ]!
>> -       ^self class == Array and: [self allSatisfy: [:each | each isLiteral]]!
>>
>> Item was added:
>> + ----- Method: Array>>isLiteralIfContainedBy: (in category 'testing') -----
>> + isLiteralIfContainedBy: parents
>> +       " Answer whether the receiver has a literal text form recognized by the compiler. Precondition: the receiver is an instance of Array. "
>> +
>> +       (parents includes: self) ifTrue: [ ^false ].
>> +       parents add: self.
>> +       1 to: self size do: [ :index |
>> +               | element |
>> +               element := self at: index.
>> +               (element class == Array
>> +                       ifTrue: [ element isLiteralIfContainedBy: parents ]
>> +                       ifFalse: [ element isLiteral ]) ifFalse: [ ^false ] ].
>> +       parents remove: self.
>> +       ^true!
>
> +1
>
> frank
>

> So, I see this as having some of the same properties as the effing
> MCInfoProxy's in that we may find we need to patch Array (!!) at some
> point in the future again -- If any other method besides #isLiteral in
> Array, ArrayedCollection, SequenceableCollection, Collection, Object,
> or ProtoObject happens to invoke a recursive function like #isLiteral
> did, we're hosed.  (and yet, no complaints from anyone in the
> community, hmm how ironic..)
>
>
> On Mon, Feb 3, 2014 at 10:28 AM, Frank Shearar <[hidden email]> wrote:
>> On 3 February 2014 16:18,  <[hidden email]> wrote:
>>> Chris Muller uploaded a new version of Collections to project The Trunk:
>>> http://source.squeak.org/trunk/Collections-ul.564.mcz
>>>
>>> ==================== Summary ====================
>>>
>>> Name: Collections-ul.564
>>> Author: ul
>>> Time: 3 February 2014, 2:35:36.802 pm
>>> UUID: 4b9a37ef-df86-40a0-a0dd-8e8b2c04d4ed
>>> Ancestors: Collections-ul.563
>>>
>>> Make sure that Array >> #isLiteral won't get into an infinite recursion, even if the receiver has an recursive structure.
>>>
>>> =============== Diff against Collections-ul.563 ===============
>>>
>>> Item was changed:
>>>   ----- Method: Array>>isLiteral (in category 'testing') -----
>>>   isLiteral
>>> +
>>> +       ^self class == Array and: [
>>> +               self isLiteralIfContainedBy: IdentitySet new ]!
>>> -       ^self class == Array and: [self allSatisfy: [:each | each isLiteral]]!
>>>
>>> Item was added:
>>> + ----- Method: Array>>isLiteralIfContainedBy: (in category 'testing') -----
>>> + isLiteralIfContainedBy: parents
>>> +       " Answer whether the receiver has a literal text form recognized by the compiler. Precondition: the receiver is an instance of Array. "
>>> +
>>> +       (parents includes: self) ifTrue: [ ^false ].
>>> +       parents add: self.
>>> +       1 to: self size do: [ :index |
>>> +               | element |
>>> +               element := self at: index.
>>> +               (element class == Array
>>> +                       ifTrue: [ element isLiteralIfContainedBy: parents ]
>>> +                       ifFalse: [ element isLiteral ]) ifFalse: [ ^false ] ].
>>> +       parents remove: self.
>>> +       ^true!
>>
>> +1
>>
>> frank
>>
>

> On 3 February 2014 16:58, Chris Muller <[hidden email]> wrote:
>> Well, I don't much like it.  Remember our discussion a couple of
>> months ago, we were wondering whether LRUCache was even useful at all?
>>  Granted, Levente said the prior _implementation_ was not useful,
>> which he addressed with this new one.
>>
>> But now what was once a simple and fast check in Array>>#isLiteral now
>> allocates an IdentityDictionary every time, just to accomodate this
>> strange implementation of LRUCache.
>
> You mean "just to fix broken behaviour which relied on certain
> unstated assumptions being true to avoid crashing horribly" :)
> #isLiteral was a bit naive, but I suppose it's not so obvious a danger
> as a cons cell.
>
> But anyway, this is why I wondered about writing a more general
> solution. For instance, writing a DepthFirstfTraversal that explicitly
> was safe against cyclic structures.

> On Mon, Feb 3, 2014 at 12:19 PM, Frank Shearar <[hidden email]> wrote:
>> On 3 February 2014 16:58, Chris Muller <[hidden email]> wrote:
>>> Well, I don't much like it.  Remember our discussion a couple of
>>> months ago, we were wondering whether LRUCache was even useful at all?
>>>  Granted, Levente said the prior _implementation_ was not useful,
>>> which he addressed with this new one.
>>>
>>> But now what was once a simple and fast check in Array>>#isLiteral now
>>> allocates an IdentityDictionary every time, just to accomodate this
>>> strange implementation of LRUCache.
>>
>> You mean "just to fix broken behaviour which relied on certain
>> unstated assumptions being true to avoid crashing horribly" :)
>> #isLiteral was a bit naive, but I suppose it's not so obvious a danger
>> as a cons cell.
>>
>> But anyway, this is why I wondered about writing a more general
>> solution. For instance, writing a DepthFirstfTraversal that explicitly
>> was safe against cyclic structures.
>
> Right now I'm just interested in getting the release done.