-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactoring the primitive at:
#638
base: pharo-12
Are you sure you want to change the base?
Refactoring the primitive at:
#638
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments :)
What happened with the primitiveAtPointerArray
created during last dojo?
{ #category : #'indexing primitive support' } | ||
StackInterpreter >> validateArray: array andIndex: index [ | ||
"This is a helper method to valide if the array and the index parameter are valid. | ||
For example, that the index is an integer, that the array is not a forwarder, etc." | ||
|
||
<inline: true> | ||
"Validate that the receiver is not immediate" | ||
(objectMemory isImmediate: array) | ||
ifTrue: [ ^ self primitiveFailFor: PrimErrInappropriate ]. | ||
|
||
"If the index is not an integer, fail" | ||
"If the array is a forwarder, for example in the case of e.g. object:basicAt:, fail" | ||
(objectMemory isNonIntegerObject: index) | ||
ifTrue: [ ^ self primitiveFailFor: PrimErrBadArgument ]. | ||
(objectMemory isForwarded: array) | ||
ifTrue: [ ^ self primitiveFailFor: PrimErrBadArgument ] | ||
] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take care
- You cannot move these validations outside the main method, because the returns were prepared to cut the flow. If you want to do this you have to check
self successful
after this method is called. - I think that validation is missing also:
argumentCount > 1 "e.g. object:basicAt:"
StackInterpreter >> commonAt: stringy [ | ||
StackInterpreter >> commonAt [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest not changing this method. We know that works and manage all the cases.
Just create the specialised versions and this one will be dead eventually.
Re-doing PR: #633
Summary:
Actually, there was already the primitive primitiveStringAt that was calling the method commonAt: with the flag set to true. So there is no need to add a new primitive and this is not a breaking change for Pharo.
Missing things: