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

Change suffix optimization to use theory-based restrictions #81

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

FredrikTaquist
Copy link
Collaborator

@FredrikTaquist FredrikTaquist commented Apr 17, 2024

Suffix optimization was hard-coded in the SymbolicSuffix class to only use optimizations for equality between suffix parameters and fresh parameters. This would make it very difficult to add new optimizations and optimizations for theories other than equality.

This PR changes the way suffix optimization is implemented such that theory-specific restrictions can be placed on suffix parameters. This is handled by the theories themselves, and the SymbolicSuffix class is now agnostic of which kinds of restrictions are available.

In addition, this change in suffix optimization also fixes a long-standing and, until recently, undetected bug where optimization for equality between suffix values would still be used, even if suffix optimization was disabled.

Copy link
Collaborator

@pfg666 pfg666 left a comment

Choose a reason for hiding this comment

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

How we build the SymbolicSuffixRestrictionBuilder is not quite right. How about adding a method to the TreeOracle which returns it, and calling this method e.g., from with RaLambda to build it?

E.g., getSymbolicSuffixRestrictionBuilder()

Other than that, I think we can merge.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be useful to have some basic documentation on the purpose of this class.

return new SDT(sdtChildren);
}

public LabeledSDT get(int l) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Get what? Perhaps getLabeledSDT,

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.

2 participants