[squeak-dev] The Trunk: Monticello-dtl.672.mcz

Eliot Miranda eliot.miranda at gmail.com
Wed Aug 9 16:58:29 UTC 2017


On Wed, Aug 9, 2017 at 9:52 AM, Eliot Miranda <eliot.miranda at 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 at 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
>



-- 
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20170809/52532217/attachment.html>


More information about the Squeak-dev mailing list