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

Print definition axioms for functions and constants. #765

Merged
merged 3 commits into from
Jul 27, 2023

Conversation

zafer-esen
Copy link
Contributor

When a function / constant has a definition axiom, in generated Boogie files, these axioms were not printed inside uses clauses of that function / constant. They were instead printed as top level declarations. This caused issues when using that printed file with the "/prune" option since some of these axioms could now be pruned.

This PR attempts to fix this issue such that axioms that define a constant / function are now (only) printed inside uses clauses of the function / constant they are defining.

When a function / constant has a definition axiom, in generated
Boogie files, these axioms were not printed inside "uses" clauses
of that function / constant. They were instead printed as top level
declarations. This caused issues when using that printed file with
the "/prune" option since some of these axioms could now be pruned.

Axioms that define a constant / function are now (only) printed
inside "uses" clauses of the function / constant they are defining.
@zafer-esen
Copy link
Contributor Author

Would it be possible for @keyboardDrummer and @shazqadeer to review this small PR?

Background
Dafny uses \prune by default. In the presence of definition axioms, verification results were inconsistent between:

  • verifying a program directly from Dafny,
  • printing Boogie files, then verifying those using Boogie.

@atomb pointed out that definition axioms were not printed inside uses clauses when printing Boogie files - which resulted in some axioms to be incorrectly pruned away.

@shazqadeer shazqadeer self-requested a review July 27, 2023 16:13
{
stream.WriteLine(";");
}
if (this.DefinitionAxioms.Any())
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you move

if (this.DefinitionAxioms.Any())
      {
        stream.WriteLine();
        stream.WriteLine("uses {");
        this.DefinitionAxioms.ForEach(axiom => axiom.Emit(stream, level));
        stream.WriteLine("}");
      }

as part of the chained if-then-else construction above? Then you won't need the change on line 2180.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would that work when a function has both a body and definition axioms? I think with the :inline attribute this becomes possible.

Example:

function {:inline} foo(x: int): bool {
    // some body
} uses {
    // some axioms
}

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

{
stream.WriteLine(";");
}
if (this.DefinitionAxioms.Any())
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@shazqadeer
Copy link
Contributor

@zafer-esen : Thanks for this PR. If you are ready to land, leave a note here. Also, I am curious what you are using Boogie for.

@zafer-esen
Copy link
Contributor Author

@shazqadeer, thank you for the review! From my side the PR is ready to be merged.

I am currently working on Dafny with @atomb and mainly experimenting with the monomorphic encoding; however, pruning seems to introduce incompleteness and in some cases errors (related: #560 and Dafny #3217), which seems to be less of an issue in other type encodings.

I came across the bug from this PR while experimenting as I was printing Boogie files from Dafny.

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