Add an instance variable to Class does not work

[PalumboN]

Bug description

I'm trying to add a new instance variable to Class definition but several errors appear.

To Reproduce

1. Go to Class definition and add a new instance variable:

ClassDescription << #Class
	slots: { ... #packageTag . #newVariable "<<<" };
	sharedVariables: { #ObsoleteClasses };
	tag: 'Classes';
	package: 'Kernel-CodeModel'

2. As this is an important class for the VM, a notification warning appears. But it's ok, it is part of an experiment for the VM.

3. On continue you get a first MNU (in Pharo 12):

I defined this method in Metaclass to continue

classInstaller
	^ self soleInstance classInstaller

4. Then, I have a similar problem with getName message:

Again, I defined the method in Metaclass to continue

getName
	^ self name

5. And then I'm stuck in this validation 😞 I don0t know how to continue.

Expected behavior

At the end I want to have newVariable as instance variable in all the classes in the system.

Version information:

• macOS Monterey 12.7.6
• Pharo 11 and 12

[hernanmd]

Something like this badre:

validateClassName
	| nameToValidate |

	name ifNil: [ ^self ].

	"I try to convert to symbol, if there is an error the next guard will catch it"
	[ name := name asSymbol ] on: Error do: [  ].

	name isSymbol ifFalse:[InvalidGlobalName
				signal: 'Global names should be symbols'
				for: name].

	nameToValidate := 	(name indexOfSubCollection: ' class' startingAt: 1) > 0
		ifTrue: [ name copyUpToSubstring: ' class'  ]
		ifFalse: [ name ].
	nameToValidate isValidGlobalName ifFalse: [
		InvalidGlobalName signal: 'Class name is not a valid global name. It must start with uppercase letter and continue with alphanumeric characters or underscore. Default names used in class or trait templates are not allowed.' for: name ].

	DangerousClassNotifier check: name

and good luck with the Segmentation fault :)

[jordanmontt]

Have you tried of directly using the fluid builder and installer to change the definition of Class instead of doing it using Calypso? It might help

[astares]

If the fluid builder approach does not work and fails too: maybe changing it in source files via text editor (locally in git), commiting and loading then via Iceberg works

[MarcusDenker]

I fear this for now requires a full boostrap. That is, do the PR manually and let the CI bootrap the image.

The question is: can we fix the class builder so it can do this? Adding an ivar to Class is ok from a VM perspective (only the first three ivars are directly accessed, and they are defined in Behavior).

But for sure the standard instance migration might be a problem if it is Class that we are changing.

[PalumboN]

Yes, I tried with the fluid builder first, it has the same behaviour.

I didn't try editing the source file and loading it via Iceberg, I'll try.

But I think that the problem is what @MarcusDenker says. I would like to fix the current instance migration algorithm and be able to do this from the image (as with any other class). I'll try to check it in the next VMDojo / Sprint.