-
Notifications
You must be signed in to change notification settings - Fork 62
IndexManager clean #793
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
IndexManager clean #793
Conversation
…s from most places, fix unit testing
|
Discount TPTP: |
Indexing/IndexManager.cpp
Outdated
| * IMPORTANT: Keep these values distinct from each other! | ||
| */ | ||
| void IndexManager::provideIndex(IndexType t, Index* index) | ||
| INDEX_ID_IMPL(AlascaIndex<ALASCA::Demodulation::Lhs>, 0) |
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.
If someone has a better idea to implement this, please let me know. I looked at solution like this and the one in the comments but they don't seem to work for all compilers/linkers.
|
Regression over TPTP, induction&synthesis benchmarks and SMTLIB showed essentially no difference (1 lost in induction, 2 lost and 1 gained in SMTLIB). I think the PR is ready to merge. |
This PR adds some type safety to
IndexManager, such that the matching the index enum values and casted-to index type does not have to be done by the caller. There is still some risk, namely that we map index types to the same values, but I hope that this will be solved by some upcoming feature of C++20/23 where there areconstexprtype IDs and other nice things.The other improvement is that now
IndexManagercannot be supplied from the outside toSaturationAlgorithmwhich allows for using the actualIndexManagerinstance during unit testing instead of the hackysetTestIndices.Regression is TBD, but there shouldn't be anyting drastic.
Otter TPTP:
master unsat: 10299 (0) sat: 1020 (0)
branch unsat: 10300 (1) sat: 1020 (0)