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.
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
Add Boolean.toByte operation #932
base: v6.0.0
Are you sure you want to change the base?
Add Boolean.toByte operation #932
Changes from all commits
3d2c364
9344877
0f6d445
1071735
790a971
957b42c
1cdc398
9e1a5e1
0f76431
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What is the reason for this change?
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.
Not sure if this can be done inline like this or maybe we need to add a class in
transformers
and a new (or existing?)OpCode
for boolean.toByteThere 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 guess opcode is not needed, from investigating MethodCallSerializer
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.
yes, opcode is not necessary. Another question here is how to make sure VersionContext.current has correct version in place when this code is executed as part of the application/tooling compiling ErgoTree?
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.
This
property
runs the test block for each activated script version / ergotree version. Something is causingSigmaParser
to execute a different code path between test cases,SBoolean.getMethods
is only called at the start when version isv0
so it returnsSeq()
for methods, presumably even forv3
sincegetMethods
isn't called when the version is set tov3
and the test fails with a "Missing method" error.I'm not sure if this is intended and methods are cached somewhere or if there's a global var not getting reset between test cases, or something else. Need to debug deeper
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.
There's caching indeed, in types.scala: 370
changing
lazy val
todef
is fixing tests.I think
methods
should be reworked in a way that for every version methods should be cached, and thendef methods
should return value from cache forVersionContext.current
(or calculate value and cache it).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.
yes, the cache should be versioned, the suggested change sounds good.
Basically the cache needs to change from Seq[SMethod] to a function Version => Seq[SMethod], which can be implemented with the help of Map[Byte, Seq[SMethod]] under the hood.