-
Notifications
You must be signed in to change notification settings - Fork 52
Extend Solver Independent SMTLib2 Parser/Generator #436
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
base: master
Are you sure you want to change the base?
Extend Solver Independent SMTLib2 Parser/Generator #436
Conversation
A problem within JavaSMT is that it allows only one single assert in an smt2 String. This needs to be corrected to support all Strings. Momentarily the Generator can be used as a workaround as it builds Strings with only one assert statement. |
…E CHANGED SOON! -added new test
95f9851
to
6b18557
Compare
… to Floating-Points as Strings
… to Floating-Points as Strings
@DavidGalllob please do not force-push! Force-push rewrites the history and can lead to conflicts locally! Also, please do not resolve threads opened by others, as it is hard to confirm that changes have resolved the issues when they are not shown anymore! |
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.
Please take a look at the marked code segments.
Additionally, please try to add some documentation in appropriate locations to make sure that your additions are maintainable in the future.
public class DummyType { | ||
/** This class ensures Type-safety for DummyFormulas. */ | ||
private int bitvectorLength; | ||
|
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 class is severely overloaded, hard to maintain, massively increases overhead and should be refactored into the appropriate type-classes!
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.
Also, why not simply use the FormulaType
class and its subclasses?
|
||
import org.sosy_lab.java_smt.api.FloatingPointRoundingMode; | ||
|
||
@SuppressWarnings({"all", "overrides"}) |
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.
Please never use the all
suppression. Also, the warnings you got here were correct and told you problems that should be resolved! See the other comment in this class.
private DummyType returnType; | ||
private List<DummyType> argumentTypes; | ||
|
||
public DummyFunction() {} |
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.
Why not use the constructor?
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.
Making this class (and most others in solverless) immutable would reduce error-prone use of methods and known issues of mutability.
import org.sosy_lab.java_smt.api.NumeralFormula.IntegerFormula; | ||
import org.sosy_lab.java_smt.api.NumeralFormula.RationalFormula; | ||
|
||
@SuppressWarnings({"StringCaseLocaleUsage", "rawtypes", "Immutable"}) |
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.
Same as for the type class of this package.
import org.sosy_lab.java_smt.api.NumeralFormula.RationalFormula; | ||
|
||
@SuppressWarnings({"StringCaseLocaleUsage", "rawtypes", "Immutable"}) | ||
public class SMTLIB2Formula |
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 class is severely overloaded, hard to maintain, massively increases overhead and should be refactored into the appropriate formula-classes!
|
||
package org.sosy_lab.java_smt.solvers.SolverLess; | ||
|
||
public class DummyEnv {} |
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.
Whats the purpose of this class?
import org.sosy_lab.java_smt.api.FormulaType.FloatingPointType; | ||
import org.sosy_lab.java_smt.basicimpl.AbstractFloatingPointFormulaManager; | ||
|
||
@SuppressWarnings("StringSplitter") |
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.
Please address the warnings instead of suppressing them.
|
||
@Override | ||
public <R> R visit(FormulaVisitor<R> visitor, Formula formula, SMTLIB2Formula f) { | ||
return null; |
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.
Never return null
!
Also, you are missing a core implementation here for the functionality of the independent SMTLib2 layer.
if (pTargetType.isRationalType()) { | ||
return new SMTLIB2Formula(new DummyType(DummyType.Type.RATIONAL)); | ||
} | ||
return null; |
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.
Never return plain null
for known reasons.
This looks like a switch-case expression.
|
||
@Override | ||
protected SMTLIB2Formula remainder(SMTLIB2Formula pParam1, SMTLIB2Formula pParam2) { | ||
return null; |
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.
Missing implementation?
This PR extends JavaSMT with an independent SMTLib2 parser/generator layer.
Every solver can use the layer to generate or parse SMTLib2, independent of their own implementation.
The parsing directly uses the API of the solver.
Generating tracks all API usage independently into SMTLib2.
Additionally, we offer a complete solver-independent layer, that is usable with no solver installed. This layer can generate SMTLib2, but also parse SMTLib2 to manipulate the given formula.
Ideally, we also want to support usage of the solvers binaries with SMTLib2 directly. This has been tested for Princess, but has not been implemented for all solvers. My suggestion would be to remove/extract it from this PR and work on it separately.
TODO: