The Trunk: Monticello-dtl.674.mcz

David T. Lewis uploaded a new version of Monticello to project The Trunk:
http://source.squeak.org/trunk/Monticello-dtl.674.mcz

==================== Summary ====================

Name: Monticello-dtl.674
Author: dtl
Time: 27 December 2017, 11:59:29.776456 pm
UUID: c87618b4-007b-4e92-8f2a-60df81d148e8
Ancestors: Monticello-eem.673

When reading MCClassDefinition from a saved version, ensure that the special case of type #compiledMethod is handled for CompiledCode and subclasses.

=============== Diff against Monticello-eem.673 ===============

Item was changed:
  ----- Method: MCStReader>>classDefinitionFrom: (in category 'as yet unclassified') -----
  classDefinitionFrom: aPseudoClass
+ | tokens traitCompositionString lastIndex classTraitCompositionString typeOfSubclass className |
- | tokens traitCompositionString lastIndex classTraitCompositionString |
  tokens := Scanner new scanTokens: aPseudoClass definition.
  traitCompositionString := ((ReadStream on: aPseudoClass definition)
  match: 'uses:';
  upToAll: 'instanceVariableNames:') withBlanksTrimmed.
  classTraitCompositionString := ((ReadStream on: aPseudoClass metaClass definition asString)
  match: 'uses:';
  upToAll: 'instanceVariableNames:') withBlanksTrimmed.
  traitCompositionString isEmpty ifTrue: [traitCompositionString := '{}'].
  classTraitCompositionString isEmpty ifTrue: [classTraitCompositionString := '{}'].
  lastIndex := tokens size.
+
+ className := tokens at: 3.
+ typeOfSubclass := self typeOfSubclass: (tokens at: 2).
+ "Compiled code classes are special cases of the #bytes class type"
+ (#bytes == typeOfSubclass and: [self compiledCodeClassNames includes: className])
+ ifTrue: [typeOfSubclass := #compiledMethod].
+
  ^ MCClassDefinition
+ name: className
- name: (tokens at: 3)
  superclassName: (tokens at: 1)
  traitComposition: traitCompositionString
  classTraitComposition: classTraitCompositionString
  category: (tokens at: lastIndex)
  instVarNames: ((tokens at: lastIndex - 6) findTokens: ' ')
  classVarNames: ((tokens at: lastIndex - 4) findTokens: ' ')
  poolDictionaryNames: ((tokens at: lastIndex - 2) findTokens: ' ')
  classInstVarNames: (self classInstVarNamesFor: aPseudoClass)
+ type: typeOfSubclass
- type: (self typeOfSubclass: (tokens at: 2))
  comment: (self commentFor: aPseudoClass)
  commentStamp: (self commentStampFor: aPseudoClass)!

Item was added:
+ ----- Method: MCStReader>>compiledCodeClassNames (in category 'as yet unclassified') -----
+ compiledCodeClassNames
+ "Answer the names of classes for which the type is #compiledMethod. Traditionally,
+ this was only class CompiledMehod, but later refactorings require that CompiledCode
+ and its subclasses be treated as type #compiledMethod."
+
+ ^{ #CompiledCode . #CompiledBlock . #CompiledMethod }!

Hi David,

    compiledCodeClassNames  is a mistake.  There are no restrictions on the number of subclasses of CompiledCode.  What tells us if a class is that if code is its format which is expressed in the type information in its class definition, /not/ its name.

_,,,^..^,,,_ (phone)

> On Dec 27, 2017, at 8:59 PM, [hidden email] wrote:
>
> compiledCodeClassNames

Hi David,


> On Dec 27, 2017, at 9:37 PM, Eliot Miranda <[hidden email]> wrote:
>
> Hi David,
>
>    compiledCodeClassNames  is a mistake.  There are no restrictions on the number of subclasses of CompiledCode.  What tells us if a class is that if code is its format which is expressed in the type information in its class definition, /not/ its name.
>
> _,,,^..^,,,_ (phone)

If your intent is to ease moving Code between Spur and prior versions then
a) provide some special case for importing prior code, but do not impose that restriction on the current versions.

b) Both the prior version and Spur express the compiled codeness of a class in the class's format, with the name being merely incidental.  One can back port the work to express that through the class definition.

Restricting the code loader by recognizing specific class names puts toad blicks in the way of future code.  It is worse than isKindOf:

>
>> On Dec 27, 2017, at 8:59 PM, [hidden email] wrote:
>>
>> compiledCodeClassNames

Hi Eliot,


On Wed, Dec 27, 2017 at 09:41:49PM -0800, Eliot Miranda wrote:
The failure occurs when reading an MCZ into a V3 image, when MCStReader>>classDefinitionFrom:
creates a new MCClassDefinition from the information provided in a PseudoClass instance. The
PseudoClass does not contain the necessary information about compiled codeness.

This code path does not seem to be followed in a Spur image, I'm not entirely sure why.

> Restricting the code loader by recognizing specific class names puts toad blicks in the way of future code.  It is worse than isKindOf:
>

I agree but don't have a better solution to offer. It would be possible to
use to the actual class to which the PseudoClass refers, but I fear this
would not work in the case of a MCClassDefinition for a class that does not
yet exist in the image.

Dave
 

> On 27-12-2017, at 9:41 PM, Eliot Miranda <[hidden email]> wrote:
>
> Restricting the code loader by recognizing specific class names puts toad blicks in the way of future code.  It is worse than isKindOf:

Not many things are worse than isKindOf: but those damn toad blicks are definitely up there.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Foolproof operation:  All parameters are hard coded.

On Thu, Dec 28, 2017 at 01:17:18AM -0500, David T. Lewis wrote: Here is an example of the failure: When reading an MCZ (Browse button on
an MCFileRepositoryInspector), the reader is loading definitions from the
snapshot/source.st member of the Zip archive. The MCMczReader has failed
(for whatever reason) to load the definitions from snapshot.bin, and has
fallen back on extracting the definitions from snapshot/source.st.

When it reaches a class definition in the source.st stream, the MCStReader
creates an MCClassDefinition. At some point in this process it needs a way
to handle the special case of a class of type #bytes that really should
be #compiledMethod instead.

I put a halt into MCStReader>>classDefinitionFrom: to illustrate where
this is happening, see attached debugger picture.

For actual classes, the typeOfClass is derived from instSpec, which is
derived from the class format number. But no such information is available
from the PseudoClass representation of the class definition that was loaded
from snapshot/source.st.

I'm sure there must be some better way to handle this, but I don't know
how to do it.

Dave