Skip to content
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

new memory behaviour #2189

Merged
merged 3 commits into from
Jul 5, 2023
Merged

new memory behaviour #2189

merged 3 commits into from
Jul 5, 2023

Conversation

Graur
Copy link
Contributor

@Graur Graur commented Jun 29, 2023

Closes: #761


PR-Codex overview

This PR focuses on ensuring that the memory can only accept one type of data primitives. It also implements error handling for overflow and strict typing for different data types.

Detailed summary

  • Added test cases for strict typing and overflow scenarios in memory
  • Implemented error messages for overflow scenarios
  • Updated comments and TODOs for future improvements

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@Graur Graur marked this pull request as ready for review June 29, 2023 09:33
@Graur
Copy link
Contributor Author

Graur commented Jun 29, 2023

@volodya-lombrozo @yegor256 Please have a look at new strictly typed memory object. I wrote some tests to suggest this new behaviour

@Graur Graur marked this pull request as draft June 29, 2023 10:15
Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Graur Overall, I appreciate the idea, but I'm unclear about the rationale behind these restrictions. Would it be possible for you to provide some links or further explanation to help me understand the reasons better? Thank you.

[e]
e > @
nop
$.equal-to "Memory type missmatch: expected <int>, got <float>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Graur Let's imagine, that int and float have the same size - 4 bytes (like in Java). Should we check their types? Or we might compare just size?

Copy link
Contributor Author

@Graur Graur Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@volodya-lombrozo Yes, you are right. Thanks!
Moreover there is no chance to compare types if I try to implement it in EO. So we decided to avoid this change

[e]
e > @
nop
$.equal-to "Not enough memory to write: expected <2^63 - 1>, got <2^63>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Graur Should we throw an exception in that case? Maybe it's better just to write -9,223,372,036,854,775,808?

Copy link
Contributor Author

@Graur Graur Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@volodya-lombrozo I like it, thanks!

@Graur
Copy link
Contributor Author

Graur commented Jul 5, 2023

@volodya-lombrozo The main reason is to make memory work with low level code more efficiently. We need to start work with memory as a strictly typed object. Since we will change our Java code to low level instructions for a new processor. Maybe @yegor256 can give some details

@Graur
Copy link
Contributor Author

Graur commented Jul 5, 2023

@yegor256 Please have a look

@Graur Graur marked this pull request as ready for review July 5, 2023 06:58
@yegor256
Copy link
Member

yegor256 commented Jul 5, 2023

@rultor merge

@rultor
Copy link
Contributor

rultor commented Jul 5, 2023

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit f9df2ae into objectionary:master Jul 5, 2023
@rultor
Copy link
Contributor

rultor commented Jul 5, 2023

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 11min)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'memory' must be strictly typed
4 participants