Faulty code: forbid read of undeclared variables

[privat]

This is the read pendent of #13201

Unlike the write case, I'm still not sure if that is the right move, as nil is a consistent answer to a buggy read.
Almost all the time, reading an undeclared variable is a bug, but nil is nevertheless consistent.
While some dynamic languages have a YOLO approach, it seems that Pharo tries to encourage some control and fail-fast philosophy. For instance, there are a bunch of runtime errors in Kernel-Exceptions that could have defaulted to be a silent nil.
Maybe this requires more thinking.

However, I did miss in the previous experiment (#13189) the opportunity to check senders because I did not use a distinctive selector. So, at least, let's check senders this time!

[privat]

the failing tests are not an issue that much: mainly codesnippet and other compiler tests to update.
The 3 NewTools are more of a problem, but only because they are inconveniently is another repository.

Numbers are more interesting: in the pristine bootstrapped image, 536 methods remain with undeclared variable reads at compile time.
I expected more, but hoped for less.
It could be bad package dependencies, or just side effects on the order Monticello install class and methods in a same package.

For the first case, there are no real incentives (only warning in the Jenkins log) to have nice dependencies, so it could have been worse.
For the second case, maybe we can teach Monticello to declare all class before compiling the methods.
The added benefit will be fewer warnings in the Jenkins log :p

Another possibility is to recompile these 536 methods.
After recompilation, all runtimeundeclaredread disappear, except one in a test (expectd) and one as symbol in UndeclaredVariable>>#emitValue:

[privat]

Oh, the important part I forgot to write in the previous message: on the first try, bootstrap passed and the very large majority of tests succeeded (above 99.7%). 🎉

I also played (graphically) with the produced image (to inspect some methods) and everything went fine.
It means that the full image (including bootstrap magic and weird elusive random string evaluations) does not seem to rely on undeclared variables at runtime, and that we can go harder on bad pieces of code that does.

After a good night of sleep, I think it's the way forward. We are still very early in Pharo12 (Pharo11 isn't even released!) and it will be easy to backtrack (totally or in a milder form) if for some reason we break too many (or too important) clients.

[privat]

Some updates:

• fix tests. Almost all are some I added myself in previous PR to capture the previous behavior of undeclared variables, it seems that no one cared that much before.
• add tests, to capture more behavior on undeclared variables.
• make both exceptions resumable, so developer can easily recover in hostile images, or after inadvertently orphan ivar that become undeclared variable for instance.
• trick StDebugger.

For this last point, I need to explain more:
StDebugger expects some VariableNotDeclared (badly named class) when a message is sent to a nil that comes from an undeclared variable (awful fragile heuristics are used). To be compatible, we can either emulate the hack (that seems to be very ugly and complex), or we can just compile the undeclared variable used as a receiver, as a literal read (i.e. previous behavior).
Therefore, if the variable become declared, the emitted code is compatible, and if the variable is still undeclared at runtime, then nil from an undeclared variable is used and the debugger is fooled. Once the debugger learn to deal with UndeclaredVariableRead, this hack can be removed.

[privat]

All green on the second try! 🎉
After the StDebugger hack, only 103 method with #runtimeUndeclaredRead remains

I think it's good for me, other improvements can be done in other PR.

[jecisc]

There is now a conflict

[privat]

I think I will wait for approves (or discussion) before solving the conflict

[MarcusDenker]

• it is nice how now the debugger opens when the Undeclared is accessed.
• I think we can remove the hack in visitVariableNode: and even clean up UndefinedObject>>#doesNotUnderstand:, as the hack in the end was just for that: enable test first development even if there is a send to a Undeclared, which was just a DNU and did not allow to create a class easily

[privat]

I solved the merge conflicts, I blinked, and now there are more merge conflicts 😭

[privat]

merge done, CI running

[privat]

test still OK!

[jecisc]

Everything seems good to me.

With the read resumable it should be fine for most things that would have bugged me otherwise :)