-
Notifications
You must be signed in to change notification settings - Fork 44
RPC trace cleanup: remove simplification logs #3880
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
Conversation
241b566
to
9fe027e
Compare
84d86aa
to
d93ebe4
Compare
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.
While we are at it, we could maybe clean up more - the RewriteSimplified
constructor in RewriteTrace
and all related code is one example.
(can also merge this and proceed with other things, though)
EquationTrace (..), | ||
pattern CollectEquationTraces, | ||
pattern NoCollectEquationTraces, | ||
eraseStates, |
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.
Am I right that we shouldn't export EquationTrace
and eraseStates
any more here?
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, thanks, removed them
I ran the @jberthold sounds good, I'll do a bit more clean-up. |
@jberthold I did not fully remove the I think it is probably enough clean-up at that point. |
I think it is only used for error reporting but nvm 🤷 |
We probably need to update K once runtimeverification/k#4367 is merged to fix Kore integration tests:
|
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.
Nice! Great to see the cleanup :)
Fixes #3871
Note that removing the simplification log request parameters and the simplification log entries form the response is same, because
pyk
is already modified to not recognize them, see:LogEntry
typeBut even considering that, we should run
pyk
's integration tests before merging.