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
Reusing backend information to construct KCFGs more efficiently #4590
Reusing backend information to construct KCFGs more efficiently #4590
Changes from 26 commits
5630763
6ff9f37
cb9e98c
00d519e
f2158a8
733d089
33fcde3
869e7b2
b20e4c6
a60b914
45fd80b
0018b2a
b0bc50e
8d4ab1f
69b3c98
e865465
12b0fc0
5ce87e8
68ba0b1
3321f48
9c86d7e
207e325
c19f63d
4d695ad
d3d3cae
4d54908
a661700
852fba4
76e9f1e
c3e6e8d
e60d2be
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.
Consider
and adding the
info: str = field(default='')
to eachdataclass
subclass.Alternatively, do not make it an abstract attribute, since the value is not used for most subclasses.
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.
Would it be ok to just have it in the ones where it's needed? The compilation does not seem to complain.
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, but then it shouldn't be present in the abstract class.
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, I removed it from there.
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 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.
Done.
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 does it mean for an extension to be cached?
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.
extend_cterm
can now return two extensions, the first of which is aStep
, and the second of which is a cut-rule, branch, or NDBranch. When that happens, the first one is to be applied, and the second one is to be cached so that it can be automatically applied instead of recomputed when we explore from the node created by that firstStep
.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.
To me, all these assertions indicate that
APRProofExtendResult
could be broken up into multiple subclasses with stronger typing guarantees.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.
100%. I will think about this a bit.
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.
I've redesigned them now.