From: "Anthony Hannan" <ajh18 at cornell.edu>
> This bug also exists in the new closure compiler, because it reuses the
> same Class>>bindingsOf: method that Peter just fixed.  Peter's fix
> works,

Anthony,

It indeed seems to work but I did notice a minor inaccuracy that could cause
incorrect operation under certain scenarios. I will repost. I'll add a
version number to the change set.

> but he also changed Class>>declare: to no longer raise a notifier
> when filing-in a class var that shadows another var.

Hmm. Interesting that you call that "raising a notifier". Perhaps this is
Squeak lingo, but I would rather call sending self error: 'blabla'
"signaling an error" or "throwing up a debugger".

Sure, I am now aware that an #error: can be resumed in Squeak, but this
hardly makes throwing up a debugger something innocent.

Defining a class variable that shadows another var is neither an error nor
something that needs to be debugged, so I don't see the reason for sending
self error:, *at all*.

Mind you, before I fixed the lookup logic, the class variable would not work
correctly if used from a subclass of the defining class. So, in the past, it
was a problem. The self error: had some merit because it protected against
that (I don't know if that was why it was put in there, of course).

But with this fixed, there is no reason not to simply proceed without saying
anything.

Also consider that if I install a program with 273 class variables that each
have a reasonable chance of shadowing an existing global (or a pool
variable, for that matter), I should not be required to press 'proceed'
hundreds of times to install it. Or am I missing something?

If one is really worried that a class variable shadows another shared
variable, and one is going to be bitten by it (rather hypothetical I would
say), another mechanism than self error: should be used. Doesn't squeak have
something like a Notification that can be raised and caught, but by default
doesn't do anything?

Something like
newClassVars do: [:newClassVar | | doesItShadowAnotherSharedVar |
doesItShadowAnotherSharedVar := ... .
doesItShadowAnotherSharedVar ifTrue: [GeneralNotification new description:
newClassVar , ' shadows another shared variable' ; raise]
]

But it still seems completely unneeded to me. The old notifier had a
justification in that the class variable might not be handled correctly.
With the fix, this justification is gone and I don't see another good one.

> I think we should
> still keep the notifier, which serves as a warning but still allows you
> to proceed pass it and load the shadow var anyway.

As I said, I don't believe it serves as a warning (it also says "Error: ..."
quite distincly at the top of the window), and it is nothing special.
Shadowing happens.

So I'm against flagging when shadowing happens and I'm not planning to
include something to flag this in my fix. Someone else can do it of course.
In the end, I'm only trying to help and I don't decide what gets in the
distribution :-).

Thanks for the feedback. I'm glad your compiler code is not affected by my
proposed fix.

Regards,

Peter van Rooijen
Amsterdam

> To get the closure compiler, go to
> http://minnow.cc.gatech.edu/squeak/ClosureCompiler.
>
> Cheers,
> Anthony