-
Notifications
You must be signed in to change notification settings - Fork 149
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
Adding new PyK wrappers to LLVM Python Bindings #4426
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.
This looks good to me, but let’s wait for at least one approval from the RV K team before merging.
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.
A couple of minor comments, and I'll let @tothtamas28 provide a final review as well in case I've missed anything.
input_kore_file = kompile._cache_definition(self.HINTS_INPUT_KORE) | ||
hints_file_name = definition_dir.name.replace('.k', '.hints') |
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 looks a bit hacky. Consider implementing a dedicated cache of hints inputs for ProofTraceTest
. Then no renaming is needed, and kompiler
can be dropped as a direct dependency.
(After the renaming, the cache might no longer point to the right file.)
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.
The renaming is just for creating a new file with a custom name instead of simply ‘hints.bin’ or something static like this. This is mainly to avoid the use of 2 different tests appending in the same file (an issue that I found while writing those tests).
Ping @Baltoli |
This PR exposes the new infrastructure created by the LLVm Backend to parse the Hints file.
We introduce the wrappers to make the streaming parser possible to use as a list of events and access it through an integrator. Or not loading all the events at the same time but accessing them one per time through the
next()
function.Depends on: runtimeverification/llvm-backend#1090