The Inbox: Kernel.dve-fbs.831.mcz

A new version of Kernel was added to project The Inbox:
http://source.squeak.org/inbox/Kernel.dve-fbs.831.mcz

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

Name: Kernel.dve-fbs.831
Author: fbs
Time: 6 January 2014, 1:30:13.195 pm
UUID: b4878bfa-b720-b94c-9166-091e0d074a89
Ancestors: Kernel-fbs.829

If a subclass contains an inst var named 'foo' and you try add a same-named inst var to a superclass, you get a very unhelpful error message. This attempts to tell you which inst vars conflict (in case there are several), and in which subclass the inst vars already exist.

(It doesn't handle all cases: if you have 'foo' in SubclassA and 'bar' in SubclassB, your error message will complain about only the first conflict.)

=============== Diff against Kernel-fbs.829 ===============

Item was changed:
  ----- Method: ClassBuilder>>validateInstvars:from:forSuper: (in category 'validation') -----
  validateInstvars: instVarArray from: oldClass forSuper: newSuper
  "Check if any of the instVars of oldClass conflict with the new superclass"
+ | usedNames temp |
- | instVars usedNames temp |
  instVarArray isEmpty ifTrue:[^true]. "Okay"
  newSuper allowsSubInstVars ifFalse: [
  self error: newSuper printString, ' does not allow subclass inst vars. See allowsSubInstVars.'. ^ false].
 
  "Validate the inst var names"
  usedNames := instVarArray asSet.
  usedNames size = instVarArray size
  ifFalse:[ instVarArray do:[:var|
  usedNames remove: var ifAbsent:[temp := var]].
  self error: temp,' is multiply defined'. ^false].
  (usedNames includesAnyOf: self reservedNames)
  ifTrue:[ self reservedNames do:[:var|
  (usedNames includes: var) ifTrue:[temp := var]].
  self error: temp,' is a reserved name'. ^false].
 
  newSuper == nil ifFalse:[
  usedNames := newSuper allInstVarNames asSet.
  instVarArray do:[:iv|
  (usedNames includes: iv) ifTrue:[
  newSuper withAllSuperclassesDo:[:cl|
  (cl instVarNames includes: iv) ifTrue:[temp := cl]].
  (DuplicateVariableError new)
  superclass: temp;
  variable: iv;
  signal: iv,' is already defined in ', temp name]]].
+ oldClass == nil ifFalse:[ | dups conflicts |
+ dups := oldClass subclasses collect: [:cls | {cls. cls instVarNames}].
+ dups
+ detect: [:data | conflicts := data second intersection: instVarArray. conflicts notEmpty]
+ ifFound: [:data |
+ DuplicateVariableError new
- oldClass == nil ifFalse:[
- usedNames := Set new: 20.
- oldClass allSubclassesDo:[:cl| usedNames addAll: cl instVarNames].
- instVars := instVarArray.
- newSuper == nil ifFalse:[instVars := instVars, newSuper allInstVarNames].
- instVars do:[:iv|
- (usedNames includes: iv) ifTrue:[
- (DuplicateVariableError new)
  superclass: oldClass;
+ variable: conflicts first;
+ signal: ('{1} {2} in {3}' format: {conflicts asCommaString. conflicts size > 1 ifTrue: ['are'] ifFalse: ['is']. data first})]
+ ifNone: [nil]].
- variable: iv;
- signal: iv,' is already defined in a subclass of ', temp name]]].
  ^true!

On 6 January 2014 13:30,  <[hidden email]> wrote:
* A branch because there's already a Kernel-fbs.830 in Inbox, and I
don't want to muck around with version numbers
* This is just a hack to address a problem I found: it needs proper
tests, and it needs proper review, hence it being in the Inbox.

At some later point, it'd be really nice to be able to catch a
DuplicateVariableError and be able, within the debugger, to perform a
Pull Up Instvar refactoring (*). As a long term project, I'd like to
blur more and more the distinction between Debugger and Browser, in
the sense of the latter being a means of editing code.

frank

(*) Yes, this means I'd like us to pick up the Refactoring Browser
code again, in a later-than-4.5 Squeak.

I'll muck around with version numbers for you if it means we can avoid
introducing a new condition into the trunk repository.  Currently,
only external projects use MC "branches", which not supported by any
repository type other than FileBased.


On Mon, Jan 6, 2014 at 7:38 AM, Frank Shearar <[hidden email]> wrote:

On 6 January 2014 19:54, Chris Muller <[hidden email]> wrote:
> I'll muck around with version numbers for you if it means we can avoid
> introducing a new condition into the trunk repository.  Currently,
> only external projects use MC "branches", which not supported by any
> repository type other than FileBased.

I'm happy to (*) unbranch the change before I apply it to trunk: I
just want people to see it so they can tell me when my hand's about to
get chewed up by the gears.

(*) View the changes the mcz will make to a clean Kernel, apply
manually, then commit that to trunk.

frank