Fix cannotInterpret: and test many VM callback methods #14211
Conversation
…infinite recursion
…ssage to the receiver (and fail) and do a lookup (and fail)
…sts a successful manual lookup path
privat
changed the title
There was a problem hiding this comment.
Some little comments with committable suggestions.
Tests look good to me :)
About the cannotReturn:, why not just testing it with an exception?
block := self blockWithNonLocalReturn.
self should: [ block value ] raise: CannotReturn.
blockWithNonLocalReturn
^ [ ^ self "Should throw always a cannot return exception because the home will be always dead" ]The about to return is probably a bit trickier to test, because it's called when unwinding the stack because of non-local returns. And it's implemented to look for the next method marked as "interested in the unwind", which boils down to ensure blocks (methods marked with primitive 198). Then the hook make sure of calling the block of the ensure.
However, if you carefully craft a context, you could set your own home context of your own class, and hook the return:through: below?
aboutToReturn: result through: firstUnwindContext
"Called from VM when an unwindBlock is found between self and its home.
Return to home's sender, executing unwind blocks on the way."
self home return: result through: firstUnwindContext|
|
||
| <primitive: 111> | ||
| self primitiveFailed | ||
| ] |
There was a problem hiding this comment.
This method is already available in MirrorPrimitives and there was a previous effort to have it only once in that place (otherwise it was repeated in many places). I propose we remove this method and we use the other.
There was a problem hiding this comment.
The class is not in Kernel, so it introduces an unwanted package dependency with ReflectionMirrors-Primitives.
Kernel should try to be minimal and self-contained. Some small code duplication is a lesser concern.
Moreover, MirrorPrimitives is undocumented and almost not used (only 4 references in the whole image outside of its own package ReflectionMirrors-Primitives, and those 4 are only for tests), its methods are also undocumented and non-discoverable.
The version in Behavior is not that bad, as the class is meaningful for the intent.
| { #category : #'simulation support' } | ||
| Behavior class >> classOf: anObject [ | ||
| "The class of anObject. | ||
| This is equivalent to `anObject class` but without a message to be send to `anObject`. | ||
| This can be very handy if the class of `anObject` is unreliable. | ||
| e.g. no inheritance from ProtoObject, or having a nil methodDict." | ||
|
|
||
| <primitive: 111> | ||
| self primitiveFailed | ||
| ] |
There was a problem hiding this comment.
| { #category : #'simulation support' } | |
| Behavior class >> classOf: anObject [ | |
| "The class of anObject. | |
| This is equivalent to `anObject class` but without a message to be send to `anObject`. | |
| This can be very handy if the class of `anObject` is unreliable. | |
| e.g. no inheritance from ProtoObject, or having a nil methodDict." | |
| <primitive: 111> | |
| self primitiveFailed | |
| ] |
| (self class lookupSelector: aMessage selector) ifNotNil: | ||
| ["Simulated lookup succeeded -- resend the message." | ||
| ^ aMessage sentTo: self]. | ||
| ((Behavior classOf: self) lookupSelector: aMessage selector) ifNotNil: |
There was a problem hiding this comment.
| ((Behavior classOf: self) lookupSelector: aMessage selector) ifNotNil: | |
| ((MirrorPrimitives classOf: self) lookupSelector: aMessage selector) ifNotNil: |
I have a work in progress branch with things more or less equivalent with your proposals. I also want to check directly the behaviour of the VM by instrumenting, diverting and hacking the receiver of these callbacks. I cannot really do that with the image implementation of Context. Anyway, I plan to make a new PR special-methods-for-Context once this one is merged |
|
What is the state of this PR? Tests are green. Some changes were requested but Jean seems to think the changes requested are not needed. Can we integrate this change? |
Fix and test
ProtoObject>>cannotInterpret:as the implementation was broken at many levels.The PR also add systematic tests for the special selectors that the VM might send in specific situations.
These tests try to check the callbacks as they are handled by default in ProtoObject/Object (booooriiiiing), but mostly check custom defined handling trough redefinition in subclasses (with more complex situations).
Tested:
#attemptToAssign:withIndex:#cannotInterpret:#doesNotUnderstand:#mustBeBoolean#run:with:in:What is missing:
#cannotReturn:,#aboutToReturn:through:,#unusedBytecode#conditionalBranchCounterTrippedOn:and#classTrapFor:(I did not look at them yet)