David T. Lewis uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-dtl.672.mcz
==================== Summary ====================
Name: Monticello-dtl.672 Author: dtl Time: 6 August 2017, 12:25:45.326051 pm UUID: dfe33bfa-dc27-4c01-a1db-234bd7e4433c Ancestors: Monticello-eem.671
CompiledCode and its subclasses are a special case, and are identified as class type #compiledMethod rather than #bytes. An MCClassDefinition should identify this type correctly, otherwise changes to these classes may not be saved and reloaded correctly.
See MCClassDefinitionTest.
=============== Diff against Monticello-eem.671 ===============
Item was changed: ----- Method: MCClassDefinition>>initializeWithName:superclassName:category:instVarNames:classVarNames:poolDictionaryNames:classInstVarNames:type:comment:commentStamp: (in category 'initializing') ----- initializeWithName: nameString superclassName: superclassString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. category := categoryString. + (self isCompiledCode: name) ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. - name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was changed: ----- Method: MCClassDefinition>>initializeWithName:superclassName:traitComposition:classTraitComposition:category:instVarNames:classVarNames:poolDictionaryNames:classInstVarNames:type:comment:commentStamp: (in category 'initializing') ----- initializeWithName: nameString superclassName: superclassString traitComposition: traitCompositionString classTraitComposition: classTraitCompositionString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. traitComposition := traitCompositionString. classTraitComposition := classTraitCompositionString. category := categoryString. + (self isCompiledCode: name) ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. - name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was added: + ----- Method: MCClassDefinition>>isCompiledCode: (in category 'initializing') ----- + isCompiledCode: className + "Classes of type #compiledMethod are treated specially. CompiledCode + and all of its subclasses, including CompiledMethod, should have this + special type." + + CompiledCode + withAllSubclassesDo: [ :cls | (className = cls name) + ifTrue: [ ^true ]]. + ^ false + !
Hi David,
I like the intent of these changes but want to discuss the implementation.
On Sun, Aug 6, 2017 at 9:25 AM, commits@source.squeak.org wrote:
David T. Lewis uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-dtl.672.mcz
==================== Summary ====================
Name: Monticello-dtl.672 Author: dtl Time: 6 August 2017, 12:25:45.326051 pm UUID: dfe33bfa-dc27-4c01-a1db-234bd7e4433c Ancestors: Monticello-eem.671
CompiledCode and its subclasses are a special case, and are identified as class type #compiledMethod rather than #bytes. An MCClassDefinition should identify this type correctly, otherwise changes to these classes may not be saved and reloaded correctly.
See MCClassDefinitionTest.
=============== Diff against Monticello-eem.671 ===============
Item was changed: ----- Method: MCClassDefinition>>initializeWithName: superclassName:category:instVarNames:classVarNames:poolDictionaryNames: classInstVarNames:type:comment:commentStamp: (in category 'initializing')
initializeWithName: nameString superclassName: superclassString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. category := categoryString.
(self isCompiledCode: name) ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol].
name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse:
[type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was changed: ----- Method: MCClassDefinition>>initializeWithName:superclassName: traitComposition:classTraitComposition:category:instVarNames: classVarNames:poolDictionaryNames:classInstVarNames:type:comment:commentStamp: (in category 'initializing') ----- initializeWithName: nameString superclassName: superclassString traitComposition: traitCompositionString classTraitComposition: classTraitCompositionString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. traitComposition := traitCompositionString. classTraitComposition := classTraitCompositionString. category := categoryString.
(self isCompiledCode: name) ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol].
name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse:
[type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was added:
- ----- Method: MCClassDefinition>>isCompiledCode: (in category
'initializing') -----
- isCompiledCode: className
"Classes of type #compiledMethod are treated specially.
CompiledCode
and all of its subclasses, including CompiledMethod, should have
this
special type."
CompiledCode
withAllSubclassesDo: [ :cls | (className = cls name)
ifTrue: [ ^true ]].
^ false
- !
I think that basing the type on the names of the subclasses of CompiledCode is error prone. For example, loading a package in which CompiledCode did /not/ have type #compiledMethod would result in an MCClassDefinition that reflected the current system rather than the package.
I think the right thing is to base it on the the typeSymbol argument of MCClassDefinition>>name:superclassName:[traitComposition:classTraitComposition:]category:instVarNames:classVarNames:poolDictionaryNames:classInstVarNames:type:comment:commentStamp:. This should be #compiledMethod for CompiledCode and subclasses, derived from Behavior>>typeOfClass. But you're explicitly using #bytes for this in your mocks, as in:
MCClassDefinitionTest>>testKindOfSubclassForCompiledBlock "CompiledCode and its subclasses are a special case." | classDef | "CompiledBlock should be like CompiledMethod, both now are subclassed from CompiledCode" classDef := self mockClass: #CompiledBlock super: #CompiledCode type: #bytes. self assert: #compiledMethod equals: classDef type. self assert: CompiledBlock typeOfClass equals: classDef type.
Surely the correct thing here is
testKindOfSubclassForCompiledBlock "CompiledCode and its subclasses are a special case." | classDef | "CompiledBlock should be like CompiledMethod, both now are subclassed from CompiledCode" classDef := self mockClass: #CompiledBlock super: #CompiledCode type: CompiledBlock typeOfClass. self assert: #compiledMethod equals: classDef type. self assert: CompiledBlock typeOfClass equals: classDef type.
no?
It looks to me like the statement name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. is an old hack for p[re-Spur which didn't bother to distinguish between CompiledMethod and subclasses and #bytes classes such as ByteArray. But at least since 1999 typeOfClass has answered #compiledMethod for CompiledMethod-like classes.
So I propose to delete the line (self isCompiledCode: name) ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. and change the mocks to use #compiledCode instead of #bytes. What do you think?
_,,,^..^,,,_ best, Eliot
On Wed, Aug 9, 2017 at 9:52 AM, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi David,
I like the intent of these changes but want to discuss the
implementation.
On Sun, Aug 6, 2017 at 9:25 AM, commits@source.squeak.org wrote:
David T. Lewis uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-dtl.672.mcz
==================== Summary ====================
Name: Monticello-dtl.672 Author: dtl Time: 6 August 2017, 12:25:45.326051 pm UUID: dfe33bfa-dc27-4c01-a1db-234bd7e4433c Ancestors: Monticello-eem.671
CompiledCode and its subclasses are a special case, and are identified as class type #compiledMethod rather than #bytes. An MCClassDefinition should identify this type correctly, otherwise changes to these classes may not be saved and reloaded correctly.
See MCClassDefinitionTest.
=============== Diff against Monticello-eem.671 ===============
Item was changed: ----- Method: MCClassDefinition>>initializeWithName:superclassName: category:instVarNames:classVarNames:poolDictionaryNa mes:classInstVarNames:type:comment:commentStamp: (in category 'initializing') ----- initializeWithName: nameString superclassName: superclassString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. category := categoryString.
(self isCompiledCode: name) ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol].
name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse:
[type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was changed: ----- Method: MCClassDefinition>>initializeW ithName:superclassName:traitComposition:classTraitCompositio n:category:instVarNames:classVarNames:poolDictionaryNames:cl assInstVarNames:type:comment:commentStamp: (in category 'initializing')
initializeWithName: nameString superclassName: superclassString traitComposition: traitCompositionString classTraitComposition: classTraitCompositionString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. traitComposition := traitCompositionString. classTraitComposition := classTraitCompositionString. category := categoryString.
(self isCompiledCode: name) ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol].
name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse:
[type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was added:
- ----- Method: MCClassDefinition>>isCompiledCode: (in category
'initializing') -----
- isCompiledCode: className
"Classes of type #compiledMethod are treated specially.
CompiledCode
and all of its subclasses, including CompiledMethod, should have
this
special type."
CompiledCode
withAllSubclassesDo: [ :cls | (className = cls name)
ifTrue: [ ^true ]].
^ false
- !
I think that basing the type on the names of the subclasses of CompiledCode is error prone. For example, loading a package in which CompiledCode did /not/ have type #compiledMethod would result in an MCClassDefinition that reflected the current system rather than the package.
I think the right thing is to base it on the the typeSymbol argument of MCClassDefinition>>name:superclassName:[traitComposition: classTraitComposition:]category:instVarNames:classVarNames: poolDictionaryNames:classInstVarNames:type:comment:commentStamp:. This should be #compiledMethod for CompiledCode and subclasses, derived from Behavior>>typeOfClass. But you're explicitly using #bytes for this in your mocks, as in:
MCClassDefinitionTest>>testKindOfSubclassForCompiledBlock "CompiledCode and its subclasses are a special case." | classDef | "CompiledBlock should be like CompiledMethod, both now are subclassed from CompiledCode" classDef := self mockClass: #CompiledBlock super: #CompiledCode type: #bytes. self assert: #compiledMethod equals: classDef type. self assert: CompiledBlock typeOfClass equals: classDef type.
Surely the correct thing here is
testKindOfSubclassForCompiledBlock "CompiledCode and its subclasses are a special case." | classDef | "CompiledBlock should be like CompiledMethod, both now are subclassed from CompiledCode" classDef := self mockClass: #CompiledBlock super: #CompiledCode type: CompiledBlock typeOfClass. self assert: #compiledMethod equals: classDef type. self assert: CompiledBlock typeOfClass equals: classDef type.
no?
It looks to me like the statement name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. is an old hack for p[re-Spur which didn't bother to distinguish between CompiledMethod and subclasses and #bytes classes such as ByteArray. But at least since 1999 typeOfClass has answered #compiledMethod for CompiledMethod-like classes.
So I propose to delete the line (self isCompiledCode: name) ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol].
Oops, I mean replace it with
type := typeSymbol.
and change the mocks to use #compiledCode instead of #bytes. What do you
think?
_,,,^..^,,,_ best, Eliot
Hi David,
I saved the proposed changes to inbox
On Wed, Aug 9, 2017 at 9:58 AM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Wed, Aug 9, 2017 at 9:52 AM, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi David,
I like the intent of these changes but want to discuss the
implementation.
On Sun, Aug 6, 2017 at 9:25 AM, commits@source.squeak.org wrote:
David T. Lewis uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-dtl.672.mcz
==================== Summary ====================
Name: Monticello-dtl.672 Author: dtl Time: 6 August 2017, 12:25:45.326051 pm UUID: dfe33bfa-dc27-4c01-a1db-234bd7e4433c Ancestors: Monticello-eem.671
CompiledCode and its subclasses are a special case, and are identified as class type #compiledMethod rather than #bytes. An MCClassDefinition should identify this type correctly, otherwise changes to these classes may not be saved and reloaded correctly.
See MCClassDefinitionTest.
=============== Diff against Monticello-eem.671 ===============
Item was changed: ----- Method: MCClassDefinition>>initializeW ithName:superclassName:category:instVarNames:classVarNames: poolDictionaryNames:classInstVarNames:type:comment:commentStamp: (in category 'initializing') ----- initializeWithName: nameString superclassName: superclassString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. category := categoryString.
(self isCompiledCode: name) ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol].
name = #CompiledMethod ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was changed: ----- Method: MCClassDefinition>>initializeW ithName:superclassName:traitComposition:classTraitCompositio n:category:instVarNames:classVarNames:poolDictionaryNames:cl assInstVarNames:type:comment:commentStamp: (in category 'initializing')
initializeWithName: nameString superclassName: superclassString traitComposition: traitCompositionString classTraitComposition: classTraitCompositionString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. traitComposition := traitCompositionString. classTraitComposition := classTraitCompositionString. category := categoryString.
(self isCompiledCode: name) ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol].
name = #CompiledMethod ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was added:
- ----- Method: MCClassDefinition>>isCompiledCode: (in category
'initializing') -----
- isCompiledCode: className
"Classes of type #compiledMethod are treated specially.
CompiledCode
and all of its subclasses, including CompiledMethod, should have
this
special type."
CompiledCode
withAllSubclassesDo: [ :cls | (className = cls name)
ifTrue: [ ^true ]].
^ false
- !
I think that basing the type on the names of the subclasses of CompiledCode is error prone. For example, loading a package in which CompiledCode did /not/ have type #compiledMethod would result in an MCClassDefinition that reflected the current system rather than the package.
I think the right thing is to base it on the the typeSymbol argument of MCClassDefinition>>name:superclassName:[traitComposition:cla ssTraitComposition:]category:instVarNames:classVarNames:poo lDictionaryNames:classInstVarNames:type:comment:commentStamp:. This should be #compiledMethod for CompiledCode and subclasses, derived from Behavior>>typeOfClass. But you're explicitly using #bytes for this in your mocks, as in:
MCClassDefinitionTest>>testKindOfSubclassForCompiledBlock "CompiledCode and its subclasses are a special case." | classDef | "CompiledBlock should be like CompiledMethod, both now are subclassed from CompiledCode" classDef := self mockClass: #CompiledBlock super: #CompiledCode type: #bytes. self assert: #compiledMethod equals: classDef type. self assert: CompiledBlock typeOfClass equals: classDef type.
Surely the correct thing here is
testKindOfSubclassForCompiledBlock "CompiledCode and its subclasses are a special case." | classDef | "CompiledBlock should be like CompiledMethod, both now are subclassed from CompiledCode" classDef := self mockClass: #CompiledBlock super: #CompiledCode type: CompiledBlock typeOfClass. self assert: #compiledMethod equals: classDef type. self assert: CompiledBlock typeOfClass equals: classDef type.
no?
It looks to me like the statement name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. is an old hack for p[re-Spur which didn't bother to distinguish between CompiledMethod and subclasses and #bytes classes such as ByteArray. But at least since 1999 typeOfClass has answered #compiledMethod for CompiledMethod-like classes.
So I propose to delete the line (self isCompiledCode: name) ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol].
Oops, I mean replace it with
type := typeSymbol.
and change the mocks to use #compiledCode instead of #bytes. What do you
think?
_,,,^..^,,,_ best, Eliot
-- _,,,^..^,,,_ best, Eliot
Yes by all means, a better implementation would be welcome. I changed it so as to be not-obviously-incorrect, but it is rather hackish both before and after the changes.
By way of background, I stumbled across the issue while trying to bring my old V3 update stream up to date with the CompiledCode changes (I still have not gotten it to work, mainly due to my own limited understanding and recent lack of time). But I have /not/ seen any actual problems when working with my main Spur-64 trunk image. In fact I do not see these methods even being called when working with trunk. Nevertheless, the methods were obviously wrong as written, so I added some tests and updated the methods to be less horrible.
Thanks, Dave
Hi David,
I like the intent of these changes but want to discuss the
implementation.
On Sun, Aug 6, 2017 at 9:25 AM, commits@source.squeak.org wrote:
David T. Lewis uploaded a new version of Monticello to project The Trunk: http://source.squeak.org/trunk/Monticello-dtl.672.mcz
==================== Summary ====================
Name: Monticello-dtl.672 Author: dtl Time: 6 August 2017, 12:25:45.326051 pm UUID: dfe33bfa-dc27-4c01-a1db-234bd7e4433c Ancestors: Monticello-eem.671
CompiledCode and its subclasses are a special case, and are identified as class type #compiledMethod rather than #bytes. An MCClassDefinition should identify this type correctly, otherwise changes to these classes may not be saved and reloaded correctly.
See MCClassDefinitionTest.
=============== Diff against Monticello-eem.671 ===============
Item was changed: ----- Method: MCClassDefinition>>initializeWithName: superclassName:category:instVarNames:classVarNames:poolDictionaryNames: classInstVarNames:type:comment:commentStamp: (in category 'initializing')
initializeWithName: nameString superclassName: superclassString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. category := categoryString.
(self isCompiledCode: name) ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol].
name = #CompiledMethod ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was changed: ----- Method: MCClassDefinition>>initializeWithName:superclassName: traitComposition:classTraitComposition:category:instVarNames: classVarNames:poolDictionaryNames:classInstVarNames:type:comment:commentStamp: (in category 'initializing') ----- initializeWithName: nameString superclassName: superclassString traitComposition: traitCompositionString classTraitComposition: classTraitCompositionString category: categoryString instVarNames: ivarArray classVarNames: cvarArray poolDictionaryNames: poolArray classInstVarNames: civarArray type: typeSymbol comment: commentString commentStamp: stampStringOrNil name := nameString asSymbol. superclassName := superclassString ifNil: ['nil'] ifNotNil: [superclassString asSymbol]. traitComposition := traitCompositionString. classTraitComposition := classTraitCompositionString. category := categoryString.
(self isCompiledCode: name) ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol].
name = #CompiledMethod ifTrue: [type := #compiledMethod]
ifFalse: [type := typeSymbol]. comment := commentString withSqueakLineEndings. commentStamp := stampStringOrNil ifNil: [self defaultCommentStamp]. variables := OrderedCollection new. self addVariables: ivarArray ofType: MCInstanceVariableDefinition. self addVariables: cvarArray sorted ofType: MCClassVariableDefinition. self addVariables: poolArray sorted ofType: MCPoolImportDefinition. self addVariables: civarArray ofType: MCClassInstanceVariableDefinition.!
Item was added:
- ----- Method: MCClassDefinition>>isCompiledCode: (in category
'initializing') -----
- isCompiledCode: className
"Classes of type #compiledMethod are treated specially.
CompiledCode
and all of its subclasses, including CompiledMethod, should have
this
special type."
CompiledCode
withAllSubclassesDo: [ :cls | (className = cls name)
ifTrue: [ ^true ]].
^ false
- !
I think that basing the type on the names of the subclasses of CompiledCode is error prone. For example, loading a package in which CompiledCode did /not/ have type #compiledMethod would result in an MCClassDefinition that reflected the current system rather than the package.
I think the right thing is to base it on the the typeSymbol argument of MCClassDefinition>>name:superclassName:[traitComposition:classTraitComposition:]category:instVarNames:classVarNames:poolDictionaryNames:classInstVarNames:type:comment:commentStamp:. This should be #compiledMethod for CompiledCode and subclasses, derived from Behavior>>typeOfClass. But you're explicitly using #bytes for this in your mocks, as in:
MCClassDefinitionTest>>testKindOfSubclassForCompiledBlock "CompiledCode and its subclasses are a special case." | classDef | "CompiledBlock should be like CompiledMethod, both now are subclassed from CompiledCode" classDef := self mockClass: #CompiledBlock super: #CompiledCode type: #bytes. self assert: #compiledMethod equals: classDef type. self assert: CompiledBlock typeOfClass equals: classDef type.
Surely the correct thing here is
testKindOfSubclassForCompiledBlock "CompiledCode and its subclasses are a special case." | classDef | "CompiledBlock should be like CompiledMethod, both now are subclassed from CompiledCode" classDef := self mockClass: #CompiledBlock super: #CompiledCode type: CompiledBlock typeOfClass. self assert: #compiledMethod equals: classDef type. self assert: CompiledBlock typeOfClass equals: classDef type.
no?
It looks to me like the statement name = #CompiledMethod ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. is an old hack for p[re-Spur which didn't bother to distinguish between CompiledMethod and subclasses and #bytes classes such as ByteArray. But at least since 1999 typeOfClass has answered #compiledMethod for CompiledMethod-like classes.
So I propose to delete the line (self isCompiledCode: name) ifTrue: [type := #compiledMethod] ifFalse: [type := typeSymbol]. and change the mocks to use #compiledCode instead of #bytes. What do you think?
_,,,^..^,,,_ best, Eliot
squeak-dev@lists.squeakfoundation.org