Skip to content

Various updates in preparation for Z3 version update - #585

Open
marcoeilers wants to merge 8 commits into
masterfrom
meilers_z3_416
Open

Various updates in preparation for Z3 version update#585
marcoeilers wants to merge 8 commits into
masterfrom
meilers_z3_416

Conversation

@marcoeilers

Copy link
Copy Markdown
Contributor

We might want to update our used Z3 version from 4.8.7 to 4.16.0 soon. This version leads to some issues with the current version of Carbon, which I'm trying to address in this PR:

  • Doing several asserts related to quantified permissions that we emit without triggers inside their own if (*) { ... assume false } block, to prevent them from being triggered in subsequent assertions.
  • Dropping some Z3 flags
  • Rewriting the axiom that states that for predicate P(x, y), P(x1, y1) == P(x2, y2) ==> x1 == x2 && y1 == y2 to one that uses getter functions for each parameter and therefore should not be multiplicative in the same way
  • Some small changes to sequence axioms
  • Introducing a timeout in the standard test suite

@alexanderjsummers alexanderjsummers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most of my comments are out-of-interest; the minor fix to the one axiom seems worth doing now

Comment thread src/main/scala/viper/carbon/verifier/BoogieInterface.scala
"/proverOpt:O:smt.ARITH.RANDOM_INITIAL_VALUE=true",
"/proverOpt:O:smt.CASE_SPLIT=3",
"/proverOpt:O:smt.DELAY_UNITS=true",
"/proverOpt:O:NNF.SK_HACK=true",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume this option has gone? Not sure what it did, to be honest

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It still exists, and the other removed ones also still exist IIRC.
The changes align our options with the ones Dafny uses (https://github.com/dafny-lang/dafny/blob/f3c2fedfb2b88272af5b64f5e45d803a3bc0043a/docs/DafnyRef/UserGuide.md?plain=1#L2751), which is also on the Z3 version we want to move to (4.16.0). My impression was that we at some point basically just copied the options Dafny used back then, so if they've switched things up, we probably should as well.
I believe this improved performance or completeness with newer Z3, but I'll recheck.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I checked, and the options change essentially makes no difference for performance or completeness, neither with old Z3 4.8.7 nor with new Z3 4.16.0. I still think we should make the change, since it would re-align the options we use in Silicon and Carbon, and those with Dafny as well.

Comment thread src/main/scala/viper/carbon/verifier/BoogieInterface.scala
Comment thread src/main/scala/viper/carbon/verifier/BoogieInterface.scala
@marcoeilers
marcoeilers marked this pull request as ready for review August 14, 2026 11:56
@marcoeilers

Copy link
Copy Markdown
Contributor Author

The changes do have some consequences independent of the Z3 version: Since more asserts are wrapped into if (*) { ...; assume false }, these asserts are not visible to the following code (that's the whole point). As a result, if these asserts fail, the rest of the code is verified without the assumption that the failed asserts holds, which we'd get in current Carbon. So we get a few more repeated errors than we currently do.

IMO this is worth it, since any later error that is currently hidden by some asserted-assumed-quantifier for which we don't have good triggers is also a potential performance problem. Any other solution to the same problem that I can think of (like adding artificial triggers to make sure the asserted quantifiers are never triggered) would have the same consequences. @Dev-XYS @alexanderjsummers Please speak up if you disagree.

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