Add implementation of uint8At: #964
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guillep
requested changes
Apr 28, 2025
Comment on lines
+570
to
+575
| VMStackPages >> uint8At: pointer [ | ||
| <doNotGenerate> | ||
| "Read an unsigned 8-bit value from memory." | ||
| self assert: (pointer >= minStackAddress and: [pointer < maxStackAddress]). | ||
| ^objectMemory byteAt: pointer | ||
| ] |
There was a problem hiding this comment.
@Sneharaj7645653 thanks for start working on this!
There is some work needed here yet.
- The method should be defined in
VMClassand not inVMStackPages. Changing the Stack API access should be a separate issue, that I advice we attack later :) - we should rewrite some users to use it instead of the old version
byteAt:. Check the senders ofbyteAt:within the VMMaker package (you can use the filter in the bottom of the list). I see at least 72 of them ;). - Also, we should probably remove the redefinitions of
byteAt:in all the simulator subclasses classes. (Check subclasses ofSpurMemoryManager) - Finally, you should run some tests to check if you missed some code.
There was a problem hiding this comment.
Sure, I will get the changes done.
|
@Sneharaj7645653, after taking into account my comments, we should probably go and modify some C files in the repository to use your new method instead of the old one too ;) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello @guillep!
Here is the implementation of uint8At:
