Navigating traits in 3.9

Hi Guys -

This message feels like a deja vu to me. I think I've written something
similar in the past but it's just incomprehensible to me that I can't
figure out basic relationships in a part of the system that I know
fairly well. So here we go again:

In response to a message that talked about traits and Java interfaces I
was looking at TAccessingMethodDictDescription to find out what the
rough percentage of "provided" and "required" method is. Doing so I
stumbled across #addSelector:withMethod:notifying: and wondered by
myself where its short form of #addSelector:withMethod: went. Browsing
the senders of the former netted me 9 places to look at among which were
four implementors of #addSelector:withMethod:. And so began the tale of
the traits...

The first implementor of #addSelector:withMethod: is in Behavior which
looks like it's just a user of one of the other three and indeed
Behavior lists TPureBehavior in its traits. Looking at TPureBehavior
next, we find that it is using TCompilingBehavior which is also in the
list. Looking at TCompilingBehavior we find #addSelector:withMethod:,
but oddly enough, we also find #addSelector:withMethod:notifying: which
is the method we started from in TAccessingMethodDictDescription. So how
are these related?

Trying to find the users of TAccessingMethodDictDescription leaves us
fishing through menus with no success. "class refs" does nothing, so
does Cmd-Shift-N or any other relevant looking menu item. Switching to
OB doesn't help either. Oh well. So let's instead click through the
individual traits to find where it is used. Hm ... nothing in
Traits-Kernel-Traits. Let's try Kernel-Classes. Ah, ClassDescription
uses it! Well, if ClassDescription has it, so may TraitDescription,
let's check. Bingo!

But wait, there is something odd here. TraitDescription is a subclass of
TraitBehavior and TraitBehavior uses TPureBehavior which is composed
from TCompilingBehavior. Oh, and ClassDescription is a subclass of
Behavior which uses TPureBehavior too? Isn't that just where we
started??? Hello? You still with me? No? Didn't think so.

Seriously, there is something really wrong here. I know that part of the
system very well, and there really should be nothing particularly
complex about those methods. In previous versions of Squeak they are as
plain as it gets. I'm not sure what it is that makes it so hard to
understand the relationship but it is crazy to browse a dozen classes
and traits to find out who is responsible for delegating a trivial
method like this and why (the process does remind me fishing through
endless series of header files though, I only wish I had a Squeak
version of grep but even Cmd-Shift-E didn't help...)

Either it's poor design or it's using patterns that I don't understand.
If the latter, I'm sure a comment or two could help. Which should go
without saying but all but one of the traits in the traits kernel have
no comment whatsoever. And the one that does claims "this eliminates the
need for TraitHolders" which is great considering that we get to know
that one entity with unknown responsibilities replaces another one with
unknown responsibilities ;-)

And I'm still at square one: How exactly are these methods related? What
trait or class would I change if I'd want to change an implementation
and why? And most importantly, how would I find out by myself? What
strategies does one use to understand relationships between traits? What
tools?

Cheers,
   - Andreas

Hi Andreas,

> Seriously, there is something really wrong here.

Yes, and I even know what is wrong!

The biggest problem is the lack of a good UI. Some work has been done  
by Daniel and few other people; very basic support is provided by  
OmniBrowser as I mentioned in the mail to Tim. I didn't put much of  
my work into the UI but tried to make the traits integration as  
smooth as possible, i.e., to make all other parts of the system work.  
Now, the tools support would need some effort to make traits really  
nice to work with. But you know, its open-source... ;)

Anyway, lets go through your problems step by step:

[...]

>  In response to a message that talked about traits and Java  
> interfaces I was looking at TAccessingMethodDictDescription to find  
> out what the rough percentage of "provided" and "required" method is.

The extended Traits OmniBrowser (see TraitsOmniBrowser on  
squeaksource.com) of Daniel does exactly this. For each trait and  
class it shows you the required methods.

> Doing so I stumbled across #addSelector:withMethod:notifying: and  
> wondered by myself where its short form of #addSelector:withMethod:  
> went. Browsing the senders of the former netted me 9 places to look  
> at among which were four implementors of #addSelector:withMethod:.  
> And so began the tale of the traits...

If you had used the OmniBrowser tools as I suggested in the other  
mail you would see that among the 4 implementers of  
#addSelector:withMethod: there are 3 that do not implement the method  
locally but obtain it from a trait. This leaves us with exactly _one_  
case to look at: TCompilingBehavior.

[...]

> Trying to find the users of TAccessingMethodDictDescription leaves  
> us fishing through menus with no success. "class refs" does  
> nothing, so does Cmd-Shift-N

Of course not. There aren't any references to traits in methods --  
just as there are usually no references to abstract classes.
Again, the problem is a missing UI. To get the users of a trait, do  
"TAccessingMethodDictDescription users"

[...]
> But wait, there is something odd here. TraitDescription is a  
> subclass of TraitBehavior and TraitBehavior uses TPureBehavior  
> which is composed from TCompilingBehavior. Oh, and ClassDescription  
> is a subclass of Behavior which uses TPureBehavior too? Isn't that  
> just where we started??? Hello? You still with me? No? Didn't think  
> so.


no, I'm not with you anymore -- because I can't see anything odd here!
It's not that complicated, really. Let me explain it.

Firstly, the classes hierarchy is still the same as it has been in  
the last century...

Behavior - ClassDescription - Class/Metaclass

... mainly with changes regarding adding and removing methods and  
managing the composition of traits. Parallel to that we have a very  
similar class hierarchy for traits:

TraitBehavior - TraitDescription - Trait/ClassTrait

Secondly, to share behavior between those two hierarchies we use  
traits. For example, between Behavior and TraitBehavior there is one  
trait named TPureBehavior (in this case it saves us from duplicating  
about 100 methods). The same way the classes further down the  
hierarchy share traits. So, that's pretty straightforward, isn't it?

What's probably adding a lot of confusion is that those traits are  
again decomposed into sub-traits. This wasn't done to avoid code  
duplication but rather as an attempt to factor out methods into  
logical groups. It seems that this just adds unnecessary confusion  
because without good tools the decomposition is not obvious.

So, since we are at it, lets see if removing those extra traits helps  
you... The attached changeset flattens down all sub-traits (not much  
tested, hence use at your own risk) and leaves us with only one trait  
for each pair of classes.

Now, if _this_ does not help you to understand how this all works...

Cheers,
Adrian

[...]

Hi Adrian -

> Anyway, lets go through your problems step by step:

Thanks, it is very much appreciated. BTW, I'm slowly getting a feel for
what exactly seems to be so hard about traits and their relationships
and I'll talk a little more about it below.

>> Browsing the senders of the former netted me 9 places to look at
>> among which were four implementors of #addSelector:withMethod:. And so
>> began the tale of the traits...
>
> If you had used the OmniBrowser tools as I suggested in the other mail
> you would see that among the 4 implementers of #addSelector:withMethod:
> there are 3 that do not implement the method locally but obtain it from
> a trait. This leaves us with exactly _one_ case to look at:
> TCompilingBehavior.

That's strictly speaking correct, but lacks one critical bit of
information namely what the relationships between the other listed
entities are and whether they are relevant for the user. If this were in
a (single inheritance) class tree, I would want to look at the hierarchy
to figure out which of these places may turn out to be relevant. And
this information (like Behavior using TPureBehavior using
TCompilingBehavior) turned out to be quite important later on. With
"just" looking at TCompilingBehvavior it's like looking at a class
implementing some message without knowing whether that's  a superclass
or a subclass of the place you're interested in (because if I understand
it correctly, a "user" of TCompilingDescription could just implement
that method and would show up just like TCompilingDescription). As a
user, right now it feels as if you've found "two interesting places"
without being able to correlate them.

>> Trying to find the users of TAccessingMethodDictDescription leaves us
>> fishing through menus with no success. "class refs" does nothing, so
>> does Cmd-Shift-N
>
> Of course not. There aren't any references to traits in methods -- just
> as there are usually no references to abstract classes.
> Again, the problem is a missing UI. To get the users of a trait, do
> "TAccessingMethodDictDescription users"

I think this needs fixing. Key relationships have always have menu
entries in the browsers - we don't ask people to print "FooClass
superclass" any more than we ask them to print "FooClass subclasses" to
figure out its dependencies. A simple fix could be to change the wording
from "class refs" to "users of ..." since that works for both classes
and traits and can do the right thing dynamically based on the selection.

Same goes for "show hierarchy" where we really want to print a combined
tree for subclasses and traits to make clear what the relationships are.

[... snip ...]
> For example, between Behavior and TraitBehavior there is one trait named
> TPureBehavior (in this case it saves us from duplicating about 100
> methods). The same way the classes further down the hierarchy share
> traits. So, that's pretty straightforward, isn't it?

Well, as far as that goes, yes. But I'm still confused. Let's see: After
visualizing (e.g., writing down) who depends on what, what I gathered so
far is that TCompilingDescription is used by TPureBehavior which is used
by Behavior and TraitBehavior. And TCompilingDescription itself
implements both #addSelector:withMethod: as well as
#addSelector:withMethod:notifying:.

Simply put my question is: How does TAccessingMethodDictDescription come
into play here? Why does it implement #addSelector:withMethod:notifying:
given that it is not a user of TCompilingDescription and therefore not
responsible for managing adding/removing selectors (which is all handled
in TCompilingDescription)? If there is a relationship between the two
and that method, were is that relationship stated?

A "second order" question is: Why is it even possible to use traits in
such a conflicting way? Shouldn't there be some way of ensuring that a
protocol that is defined by a trait in a superclass doesn't get
redefined nilly-willy by a trait in a subclass?

Along the same lines, are you aware that TraitDescription uses
TTransformationCompatibility and ClassTrait (being a subclass of
TraitDescription) uses it, too? Is there a point which I'm missing to
that duplication? (I doubt it, in which case it's an interesting data
point about the complexities of understanding relationships with traits
and classes since I'd assume that at least a dozen people have looked at
it and missed that duplication)

> What's probably adding a lot of confusion is that those traits are again
> decomposed into sub-traits. This wasn't done to avoid code duplication
> but rather as an attempt to factor out methods into logical groups. It
> seems that this just adds unnecessary confusion because without good
> tools the decomposition is not obvious.

Yes, that's definitely part of it. The main problem as I see it, is that
you're creating two tree structures, one for the traits using each
other, and one for the class hierarchy, and then you "mix in" selected
nodes of the traits tree into the class tree. And since there are no
requirements about which traits you can mix into which classes it is
perfectly possible to create "logical cycles" where one trait uses
another but in the class hierarchy it's exactly the other way around (a
superclass using the "sub-trait" and the subclass using the
"super-trait"). That's an extreme example but points out the fundamental
problem of mapping these tree structures into each other.

Interestingly, this cannot happen in most MI systems I'm aware about.
Usually the rules of what can be inherited from where are pretty
strictly aligned with the hierarchies (if the ability to inherit from
the same tree exists at all).

> Now, if _this_ does not help you to understand how this all works...

It does. OTOH, it also makes me understand a lot better what I dislike
about traits - it seems to boil down to the same old MI arguments (which
are basically: hard to understand and difficult to use "right" because
of the complexity of the resulting structures).

Cheers,
   - Andreas

Hi Andreas,

On Oct 22, 2006, at 22:50 , Andreas Raab wrote:
yes, this kind of information would be very helpful but is missing.

[...]
again, I agree.

> Same goes for "show hierarchy" where we really want to print a  
> combined tree for subclasses and traits to make clear what the  
> relationships are.

I attached a small screenshot of the TraitsOmniBrowser which for each  
class or trait shows a hierarchical view of how traits are composed.  
The browser probably needs some more work to make it reliable (just  
run into a problem) but then it would make a lot of sense to  
integrate it into OB which comes with the main distribution.




[...]

> Well, as far as that goes, yes. But I'm still confused. Let's see:  
> After visualizing (e.g., writing down) who depends on what, what I  
> gathered so far is that TCompilingDescription is used by  
> TPureBehavior which is used by Behavior and TraitBehavior. And  
> TCompilingDescription itself implements both  
> #addSelector:withMethod: as well as  
> #addSelector:withMethod:notifying:.

no, TCompilingDescription is not used by TPureBehavior. I guess, you  
are confusing TCompilingDescription with TCompilingBehavior.
It does not look conflicting to me. At the level of Description the  
implementation of Behavior is extended (note also that  
TAccessingMethodDictDescription>>addSelectorSilently:withMethod:  
sends to super). In general, I think its a question of taste. (Do you  
want to be warned each time you override a method from the superclass?)

> Along the same lines, are you aware that TraitDescription uses  
> TTransformationCompatibility and ClassTrait (being a subclass of  
> TraitDescription) uses it, too? Is there a point which I'm missing  
> to that duplication? (I doubt it, in which case it's an interesting  
> data point about the complexities of understanding relationships  
> with traits and classes since I'd assume that at least a dozen  
> people have looked at it and missed that duplication)

Yep, you are right. Somebody added the trait a while back and I  
haven't noticed this problem!

[...]

Cheers,
Adrian

Adrian Lienhard wrote:
You are right, it's TCompilingBehavior that I mean. But this just points
to the next issue: If I look at a browser I see both
TCompilingDescription and TCompilingBehavior next to each other. None of
the names make sense to me (what exactly is a "compiling behavior" and a
"compiling description"?) and there is no way for me to find out how
these entities are related other than by trying to find "matching
points" in the class hierarchy. Looking at TCompilingBehavior's users I
find TPureBehavior; looking at TCompilingDescription's users I find ...
"an IdentitySet(ClassDescription ClassDescription TraitDescription)".
Huh? Anyone else seeing this? An identity set with a duplicate entry?

In any case, unless I know already that TPureBehavior is used by
Behavior and TraitBehavior and that those are superclasses of the
classes listed here, there is basically no way to find out how these
entities relate. So basically, to find out how to traits are related,
I'm projecting them into the class hierarchy, look at the projected
inheritence tree and conclude from that projection how the traits are
related. I'm sorry, but this is ridiculous.

But they are _not_ superclasses. There is no (stated) relation between
those traits whatsoever. And that's exactly what I asked for: How do I
find out that these entities are indeed related and that the overlap in
the selector isn't just a random conflict?

Cheers,
   - Andreas

Hi Andreas,

on Mon, 23 Oct 2006 10:35:13 +0200, you wrote:

> ... Looking at TCompilingBehavior's users I find TPureBehavior; looking  
> at TCompilingDescription's users I find ... "an  
> IdentitySet(ClassDescription ClassDescription TraitDescription)". Huh?  
> Anyone else seeing this? An identity set with a duplicate entry?

Confirmed, printing "array fourth == array fifth" in an inspector on  
TCompilingDescription => true.

/Klaus

Klaus D. Witzel wrote:
> on Mon, 23 Oct 2006 10:35:13 +0200, you wrote:
>> ... Looking at TCompilingBehavior's users I find TPureBehavior;
>> looking at TCompilingDescription's users I find ... "an
>> IdentitySet(ClassDescription ClassDescription TraitDescription)". Huh?
>> Anyone else seeing this? An identity set with a duplicate entry?
>
> Confirmed, printing "array fourth == array fifth" in an inspector on
> TCompilingDescription => true.

And not only that. All of the following traits have duplicate users and
it's not limited to ClassDescription/TraitDescription either.

TAccessingMethodDictDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

TBasicCategorisingDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

TBehaviorCategorization users
   an IdentitySet(Trait Class Class Class)

TCommentDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

TCompilingDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

TCopyingDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

TFileInOutDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

TPrintingDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

TPureBehavior users
   an IdentitySet(TraitBehavior Behavior Behavior)

TTestingDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

TTraitsCategorisingDescription users
   an IdentitySet(ClassDescription ClassDescription TraitDescription)

How can an IdentitySet include two identical elements?!

The only thing I can think of is that the identityHash of an element  
changed. When ClassBuilder creates a new class to change the class  
references it does an #elementsForwardIdentityTo: which claims that  
"...the identityHashes remain with the pointers rather than with the  
objects so that the objects in this array should still be properly  
indexed in any existing hashed structures after the mutation.".

If not this, what else could cause the problem?

Adrian

Btw, sending a #rehash cleans the IdentitySets up


On Oct 23, 2006, at 19:54 , Andreas Raab wrote:

Trying to identify the problem made me stumble into ClassBuilder and
looking at the core method ClassBuilder>>name: className inEnvironment:
env subclassOf: newSuper type: type instanceVariableNames: instVarString
classVariableNames: classVarString poolDictionaries: poolString
category: category unsafe: unsafe.

This makes me wonder what the intend of the latest changes there were.
They do look rather suspicious to me. Could someone with the initials
"rw" please explain the reasoning for these changes? It's the last
person who touched that method and unfortunately there is no method
history any longer :-((( (the version found in 3.8 from '04 looks
approximately correct so any changes inbetween would be interesting)

Cheers,
   - Andreas


Andreas Raab wrote:

hi Andreas,

long time no see :-) rw is me.

I had a quick look at the method in the RC2 image. I added a trigger  
in this method for the change mechanism to tell:
* whether the category of a class changed,
* whether the class definition changed,
* or when it is a new class that was added.

The implementation checks for these occurences and is validated by  
the corresponding unit tests.

Do you think there is problems with it ?

Hi Roel -

Roel Wuyts wrote:
> long time no see :-) rw is me.

Ah yes, should have thought of that.

> The implementation checks for these occurences and is validated by the
> corresponding unit tests.
>
> Do you think there is problems with it?

I am suspecting there is an issue with traits registration due to the
copy of the original class you are creating in ClassBuilder. Which seems
to be required by the notification mechanism but strikes me as
conflicting by definition, since it asks for both, the old *and* the new
class to be accessible at the same time. Which, by virtue of the
invariants guaranteed in ClassBuilder, should never ever happen (and I
am VERY suspicious of just throwing incomplete copies of classes around).

Question: Wouldn't it be more logical, less conflicting and just
generally better design to drop the requirement of having both classes
accessible and instead simply use a ClassDefinition object for the old
class? This may look a lot like a class except that it isn't a behavior
and therefore not subject to ClassBuilder invariants.

Cheers,
   - Andreas

Hi Andreas,

that part of the code was not me, I think.

For the change notification I only asumed to get the *name* of a  
renamed or removed element to be passed around (whether a class or a  
method or whatever else), not the removed/renamed element itself (for  
the reasons you mention). But the version in the image is not exactly  
mine (whoever put it in there made some changes for better or for  
worse), and this version does not seem to be from me. But ok...

I can fix this, I guess (I think that Nathanael was the maintainer  
for the class builder, but I do not know whether he is still around  
for this)...

You need this urgently, I assume ?

PS: Can you reply personally to me as well ? I do not follow the  
squeak list as often as I should :-( But I do fix things that get  
included in the releases :-)


On 24 Oct 2006, at 10:09, Andreas Raab wrote:

Hi Roel -

Roel Wuyts wrote:
> For the change notification I only asumed to get the *name* of a renamed
> or removed element to be passed around (whether a class or a method or
> whatever else), not the removed/renamed element itself (for the reasons
> you mention). But the version in the image is not exactly mine (whoever
> put it in there made some changes for better or for worse), and this
> version does not seem to be from me. But ok...

Yeah, it's quite possible. Like I said since the history is gone so I
can't be sure who changed what but since your initials were the last I
thought you might know what these changes are intended for.

> I can fix this, I guess (I think that Nathanael was the maintainer for
> the class builder, but I do not know whether he is still around for
> this)...
>
> You need this urgently, I assume ?

I really don't, since I don't use 3.9 at all. It just seems "broken by
design" and I was curious if this is what's causing the traits problem.

> PS: Can you reply personally to me as well ? I do not follow the squeak
> list as often as I should :-( But I do fix things that get included in
> the releases :-)

Sure, can do that. I just don't do it generally unless people ask me
because personally, I'm just the opposite ;-)

Cheers,
   - Andreas

> I really don't, since I don't use 3.9 at all. It just seems "broken by
> design" and I was curious if this is what's causing the traits problem.

I am using 3.9 for all serious (industrial) work. It works well by
design, without having to apply hundreds of patches manually before
being productive.

The only exception is the Graphics/Balloon bug that I reported here:
http://bugs.impara.de/view.php?id=5222

Roel, I would really appreciate if this bug in the
ClassBuilder/ChangeNotification could be fixed.

Cheers,
Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

Lukas Renggli wrote:
> I am using 3.9 for all serious (industrial) work. It works well by
> design, without having to apply hundreds of patches manually before
> being productive.

With "it" being broken by design I meant the exposure of the "before"
and "after" version of a class in the notification mechanism, not 3.9 in
general. Sorry for the confusing choice of words.

Cheers,
   - Andreas

Hi everyone,

I wanted to note that Andrew Black and I have also touched the change
notification integration into class builder last summer during the
Traits work.

The story as I remember it - the traits work required massive recreation
of classes due to adding variables somewhere around behavior. It turned
out that this corrupted the subclasses/superclass relationship, and we
traced the bug to the code you're looking at. If I remember correctly,
the problem was that for SCN, CB creates a copy of the class, but this
was not always cleaned up because CB uses an early return to signal
failure. IIRC, my fix was to add the send of #ensure: you can see is now
there.

I didn't, however, add the copying of the class itself - that was
already there.

To move forward - anyone else been touching that code, and possibly
remembers what the copies were added for? AFAICT,
ModifiedClassDefinitionEvent does indeed keep copies of the new and old
definitions, and from looking at their uses, I think Andreas' suggestion
of a "record object" would probably do just fine. At a second look, I
would guess that the use of copies of the old class dates to the
addition of the "anyChanges" method, apparently by NathanaelS.

Daniel Vainsencher


Roel Wuyts wrote: