Andreas,

I just noticed that my code overrides your patch of the method
ClassBuilder>>reshapeClass:to:super. So, here is a changeset that fixes that
and includes both your and my changes.

In addition, I also found out what causes the problem when we change the
superclass of Class. The attached changeset also fixes this issue. The
problem lies in the method ClassBuilder>>update:to:. Let's assume that we
have a class A with an obsolete subclass B. This means that 'A
obsoleteSubclasses' yields B. In update:to:, we already have a new class B*
(with superclass A*), which conforms to the new format. In addition, we
create the temporary class B^ (with superclass A), which takes over all the
instances from B. Now, we add B^ as an obsoleteSubclass of A and then, we
use Object>>becomeForward: to make B into B*. However, since B was still
registered as an obsolete subclass of A, this makes B* an obsoleteSubclass
of A. But this is wrong, and therefore I added the line

oldClass isObsolete ifTrue: [tmpClass superclass removeObsoleteSubclass:
oldClass].

close to the end of this method.

Using this line, reshaping classes was working fine and I was able to change
the superclass of Class without any problems. However, when I did a
consitency check of the class hierarchy, (is every regular subclass
correctly registered as a subclass and is every obsolete subclass correctly
registered as an obsolete subclass), I noticed that there is still something
wrong.

Debugging these issues, I found another two bugs in the method
ClassBuilder>>update:to:

1) We use Object>>clone to create the class tmpClass which is identical to
oldClass. However, since the ObsoleteSubclasses are not stored in the class
object itself (but in a weak dictionary on the class side of Behavior), this
does not add the obsolete subclasses of the oldClass to the tmpClass. So, I
had to add some code which adds all the obsolete subclasses of oldClass also
to tmpClass before oldClass gets converted to tmpClass.

2) At one point, we use Object>>becomeForward: to replace oldClass by
newClass. However, at this point, both oldClass and newClass have entries in
the ObsoleteSubclasses dictionary. As a consequence, this dictionary
afterwards contains two identical keys, and as you can imagine, this makes
the dictionary act in very weird way. (The dictionary returns sometimes the
value that is associated to the first and sometimes the one that is
associated to the second value). In order to fix that, I had to add the line

oldClass removeAllObsoleteSubclasses

before the execution of Object>>becomeForward:.


After fixing these issues, I have done several experiments that result in a
recompilation of the whole image and at every point, the class hierarchy has
been completely consistent! Compared to the mess that happened before, this
means in particular:

- No obsolete classes are stored in the SystemDictionary
- B is obsolete and 'B superclass' yields A  <-->  'A obsoleteSubclasses'
contains B
- B is not obsolete and 'B superclass' yields A  <-->  'A subclasses'
contains B

In order to bring your image in a consitent state, you have to do the
following:

1) File in the attached changeset.
The patched ClassBuilder methods make sure that there are no inconsitencies
generated in the future).

2) Execute 'ClassBuilder cleanupAndCheckClassHierarchy'.
This cleans up (fixes) the class hierarchy, removes all the obsolete classes
in the SystemDictionary and checks the class hierarchy for consistency.


Please check out whether this works for you. If yes, we should make it go
into the image...

Cheers,
Nathanael


> -----Original Message-----
> From: squeak-dev-admin at lists.squeakfoundation.org
> [mailto:squeak-dev-admin at lists.squeakfoundation.org]On Behalf Of
> Nathanael Scharli
> Sent: Freitag, 15. Februar 2002 18:04
> To: squeak-dev at lists.squeakfoundation.org
> Subject: RE: Mess with obsolete classes???
>
>
> Andreas,
>
> > > Of course, I can modify the line (*) in the method
> > > 'ClassBuilder>>mutate:to:' so that also these strange
> > > obsolete subclasses are included, but it seems to me that
> > > it would be better to tackle the *real* problem, which
> > > is the fact that these obsolete classes are stored in
> > > such a messy (inconsistent) way.
> >
> > Hey, the "real" problem is that we don't know how they got there ;-)
>
> Well, I know ;-)
> This whole issue with the obsolete classes that are stored in the system
> dictionary (with strings as keys!!) was so annoying that I just
> had to hunt
> it down. After going through the ClassBuilder I found out that the problem
> is in the method ClassBuilder>>reshapeClass:to:super:. This
> method contains
> the following piece of code:
>
> 	"Export the new class into the environment"
> 	aClass isMeta ifFalse:[
> 		"Derefence super sends in the old class"
> 		self fixSuperSendsFrom: aClass.
> 		"Export the class"
> 		[environ at: newClass name put: newClass]
> 			on: AttemptToWriteReadOnlyGlobal do:[:ex|
> ex resume: true].
> 		environ at: newClass name put: newClass.
> 		"And use the ST association in the new class"
> 		self fixSuperSendsTo: newClass].
>
> This piece is responsible for exporting a newly created class into the
> SystemDictionary. However, if the original class (aClass) is
> obsolete, also
> the newly created class (newClass) is obslete and this piece of
> code exports
> it into the System Dictionary!! (And since the name of an
> obsolete class is
> a string (see Class>>obsolete), the entry even has a string as its key and
> this does not really make sense in a IdentityDictionary).
>
> The attached changeset fixes this problem by only adding the new class to
> the SystemDictionary if the original class is not obsolete. In
> addition (and
> this is even more important), I modified this method so that a
> newly created
> obsolete class gets correctly added to the obsolete subclasses of its
> superclass (by sending addObsoleteSubclass).
>
> Using this method should guarantee, that all obsolete classes are
> generated
> the right way. This means:
> 1) No obsolete classes in the SystemDictionary
> 2) Every obsolete class is in the obsleteClasses (weak array) of its
> superclass
>
>
> Now, the only thing we have to do is to fix the mess that is
> already in the
> image. Therefore, I wrote some methods (I temporarily put them into the
> class-side of ClassBuilder) which clean that up.
>
> Just load the attached changeset into your image and evaluate:
>
> ClassBuilder cleanupObsoleteClasses
>
> This remove all the obsolete classes from the system dictionary and fixes
> the class hierarchy. In the Transcript it shows, how many obsolete classes
> are in the system before and after this cleanup. In my case, there were
> about 150 obsolete classes before and only 11 afterwards! Alltogether, the
> image got about 80K smaller.
>
> So, I think this was really worth it ;)
>
> Cheers,
> Nathanael
>
> PS: Since the new version of ClassBuilder>>reshapeClass:to:super: does not
> enter obsolete classes into the SystemDictionary, I am not 100%
> sure whether
> the call of ClassBuilder>>fixSuperSendsTo: (which occurs in the
> same method)
> still does the right thing (in case of obsolete classes). Can you have a
> look at that to make sure that it is correct?
>
>
> > -----Original Message-----
> > From: squeak-dev-admin at lists.squeakfoundation.org
> > [mailto:squeak-dev-admin at lists.squeakfoundation.org]On Behalf Of Andreas
> > Raab
> > Sent: Freitag, 15. Februar 2002 11:17
> > To: squeak-dev at lists.squeakfoundation.org
> > Subject: RE: Mess with obsolete classes???
> >
> >
> > Nathanael,
> >
> > > Nevertheless, I think that there is also a problem caused by
> > > the circular meta-structure. This does not occur when you add
> > > a new instance variable to 'Class', but it occurs when you change
> > > the superclass of 'Class'. (In fact, this is what I did in my image).
> >
> > Ah, yes. That's slightly different ;-)
> >
> > > What happens is that the ClassBuilder wants to change the
> > > shape of 'Class' and it therefore calls 'ClassBuilder>>mutate:to:'
> > > for all the subclasses. At one point, 'Class class' is mutated and
> > > this reshapes its instance 'Class'. (Your fix made this work
> > > correctly). After all the subclasses are reshaped,
> > > the ClassBuilder calls 'ClassBuilder>>update:to:' to update
> > > 'Class' itself. But since the instance has already been touched
> > before,
> > > it is in an inconsistent state and it brakes.
> >
> > Hm. I still think that this should work. Simply because when we reshape
> > a class we temporarily have two classes X and X'. X is the original one
> > and X' the new superclass for all the subclasses of X. Only when all
> > subclasses of X have been converted X is changed into X'. For your Class
> > example, this would mean we first change the shape of Class into Class'
> > then recompile the entire metaclass hierarchy (which will update both
> > Class class and Class' class) and then - after all this is done - we
> > change Class into Class'. Since the recompilation process ought to be
> > atomical with respect to updating subclasses there should be no visible
> > difference for the class builder.
> >
> > At least that's the theory ;-)
> >
> > > This is the reason why I modified the ClassBuilder so that it
> > > changes 'Class class' and its instance at the end of the process
> > > and not in the middle. This ensures that 'Class' is still in the
> > > expected shape when it is used as an argument to
> > >'ClassBuilder>>update:to:'. (With other words, my change
> > > makes sure that 'ClassBuilder>>update:to:' is called first
> > > with the argument 'Class' and then with 'Class class' instead of doing
> > > it the other way round). Considering the circular meta-structure of
> > Squeak
> > > this seems a reasonable way to do it without applying too many dirty
> > > tricks. What do you think?
> >
> > Everything that works is reasonable here :-) My interest is more on the
> > theoretical side - it just "ought to work" and I am wondering why it
> > doesn't.
> >
> > > Okay, now let me come back to the issue of the strange
> > > obsolete classes sored in the SystemDictionary.
> >
> > That's clearly some bug.
> >
> > > 1) Why do these classes not appear as obsolete subclasses of their
> > > superclasses?
> >
> > I have no idea whatsoever. This should never be the case.
> >
> > > 2) Why are these classes stored in the SystemDictionary (with
> > > strings as keys!)?
> >
> > Again, I have no idea. This just absolutely shouldn't be the case.
> >
> > > 3) Shouldn't we change this so that the class hierarchy is
> > > consistent (means if a class B has a superclass A, then B is
> > > either a regular subclass or an obsolete subclass of A).
> >
> > By all means, yes! However, we should also change it so that we never
> > even get into the inconsistent state - if we just new how it got there
> > in the first place...
> >
> > > Of course, I can modify the line (*) in the method
> > > 'ClassBuilder>>mutate:to:' so that also these strange
> > > obsolete subclasses are included, but it seems to me that
> > > it would be better to tackle the *real* problem, which
> > > is the fact that these obsolete classes are stored in
> > > such a messy (inconsistent) way.
> >
> > Hey, the "real" problem is that we don't know how they got there ;-)
> >
> > Cheers,
> >   - Andreas
> >
> >
> >
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ClassBuilderFix.6.cs
Type: application/octet-stream
Size: 14183 bytes
Desc: not available
Url : http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20020219/3e63fd38/ClassBuilderFix.6.obj