2015-02-01 10:52 GMT+01:00 Ben Coman <btc@openinworld.com>:

Looking into Image locking problems [1] caused by a recursive array such as this...

�� �� literalArray := #( 1 2 3 ).

�� �� literalArray at: 3 put: literalArray.

I find that "literalArray printString" locks the image due to Array>>printOn: use of the recursive #shouldBePrintedAsLiteral method. Now its implementation is identical to #isLiteral and indeed "literalArray isLiteral" also locks the Image. So comparing implementors of #isLiteral... ��

Squeak uses a Set to store all visited elements for shouldBePrintedAsLiteral and this protects against the recursive loop.

shouldBePrintedAsLiteralVisiting: aSet
������ self class == Array ifFalse:
������ ������ [^false].
������ (aSet includes: self) ifTrue:
������ ������ [^false].
������ aSet add: self.
������ ^self allSatisfy: [:each | each shouldBePrintedAsLiteralVisiting: aSet]

isn't there a common pattern to handle this kind of potential endless recursion?

�� Object>>isLiteral �� ^false

�� Boolean>>isLiteral ^true

�� Character>>isLiteral ^true

�� Integer>>isLiteral ^true

�� String>>isLiteral ^true

�� UndefinedObject>>isLiteral ^true

�� ByteArray>>isLiteral ^self class == ByteArray

�� Float>>isLiteral ^self isFinite "^(self - self) = 0.0"

�� ScaledDecimal>>isLiteral ^denominator = 1 or: [(10 raisedTo: scale)\\denominator = 0]

�� Array>>isLiteral ^self class == Array and: [self allSatisfy: [:each | each isLiteral]]

...I find most are very basic (might even say deterministic), with the recursion of Array>>isLiteral seeming an annomaly.�� Also, the big IF condition in Array>>printOn: smells like a design decision being made at runtime (Valloud AMCOS p12). ��

�� �� Array>>printOn: aStream

self shouldBePrintedAsLiteral ifTrue: [self printAsLiteralFormOn: aStream. ^ self].

self isSelfEvaluating ifTrue: [self printAsSelfEvaluatingFormOn: aStream. ^ self].

super printOn: aStream

Flipping between two printString formats seems like selecting between two class types. Indeed, if we had a LiteralArray class, there would be no need for its printOn: to recursively search to determine its form, thus allowing #printStringLimitedTo: to do its thing to protect against infinite recursion.

Also, instead of a recursive Array>>isLiteral we'd have something like

�� LiteralArray>>isLiteral ^true

�� Array>>isLiteral ^false

which seems to align much better with the pattern of the other #isLiteral implementors.

I notice there is both RBArrayNode and RBLiteralArrayNode.

So what are the wider concerns that might apply?

(In particular, I'm not sure how the #isSelfEvaluating (which is also recursive) fits into the big picture)

cheers -ben

[1] https://www.mail-archive.com/pharo-dev@lists.pharo.org/msg25156.html