-
Notifications
You must be signed in to change notification settings - Fork 31
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
Assertion expressions #690
Conversation
Add Program.getSpecificationType() Add Program.hasReachabilitySpecification() Add Program.setSpecification(SpecificationType,AbstractAssert) Remove Program.setSpecification(AbstractAssert) Remove AbstractAssert.getType() Remove AbstractAssert.isSafetySpec() Remove AbstractAssert.setType(String) Remove AbstractAssert.toStringWithType() Add ProgramBuilder.setAssertExists(AbstractAssert) Add ProgramBuilder.setAssertForall(AbstractAssert) Add ProgramBuilder.setAssertNotExists(AbstractAssert) Remove ProgramBuilder.setAssert(AbstractAssert) Replace AssertionHelper.parseAssertion(ProgramBuilder,String)
Add EncodingContext.encodeFinalExpressionAsBoolean(Expression)
…RuleContext,ParserRuleContext) Remove AssertionHelper
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 love PRs that mostly remove code 😍
private String name; | ||
private AbstractAssert spec; | ||
private AbstractAssert filterSpec; // Acts like "assume" statements, filtering out executions | ||
private SpecificationType specificationType = SpecificationType.ASSERT; |
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.
Do I understand correctly that by default we assume the specification type is inline assertions and all litmus parsers are supposed to set this to something different?
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.
It seems so. I kinda dislike the distinction between forall
and assert
, since the latter is conceptually an instance of the former. But I guess it still is an improvement of what we had before.
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.
The output format of the condition has regressed:
- The output does not attach Tid prefixes, resulting in specs that look like
(X == 4) && (X == 5)
but actually mean(0:X == 4) && (1:X == 5)
. - A sequence of conditions
A && B && C
is now printed with parenthesis for all subexpressions:((A && B) && C)
which causes a lot visual overhead for many tests.
Comparison:
// Previously:
Condition exists (ok==bv64(1) && x==bv64(1) && y==bv64(1) && z==bv64(2) && 0:bv64 X2==bv64(0) && 0:bv64 X6==bv64(0) && 1:bv64 X1==bv64(1) && 1:bv64 X5==bv64(0) && 2:bv64 X2==bv64(1) && 2:bv64 X5==bv64(0))
// Now (notice the double occurence of X5):
Condition EXISTS ((((((((((ok == bv64(1)) && (x == bv64(1))) && (y == bv64(1))) && (z == bv64(2))) && (bv64 X2 == bv64(0))) && (bv64 X6 == bv64(0))) && (bv64 X1 == bv64(1))) && (bv64 X5 == bv64(0))) && (bv64 X2 == bv64(1))) && (bv64 X5 == bv64(0)))
I think we need a smart printer for expressions that avoids parenthesis for nested associative operations and prints function ids in front of registers (configurable).
…ssertion-expressions
dartagnan/src/main/java/com/dat3m/dartagnan/expression/ExpressionPrinter.java
Outdated
Show resolved
Hide resolved
dartagnan/src/main/java/com/dat3m/dartagnan/expression/ExpressionPrinter.java
Outdated
Show resolved
Hide resolved
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.
LGTM
This PR replaces
AbstractAssert
withExpression
. It primarily removes duplicated code. It also changes the handling of user assertions:PropertyEncoder
now completely controls their encoding (as opposed toModelChecker
having to inject them into the spec field first).