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

Introduce a kani macro for checking coverage #2011

Merged
merged 6 commits into from
Dec 16, 2022

Conversation

zhassan-aws
Copy link
Contributor

@zhassan-aws zhassan-aws commented Dec 16, 2022

Description of changes:

Introduce a kani::cover macro that can be used for checking whether certain conditions can occur without causing verification to fail.

Resolved issues:

Resolves #696

Related RFC:

Optional #1905

Call-outs:

This PR does not yet introduce reachability checks for cover properties. Thus, both unreachable and reachable but unsatisfiable properties will be reported as "UNSATISFIABLE". I'm planning to introduce reachability checks in a follow-up PR because I didn't want to complicate this one too much.

Testing:

  • How is this change tested? Added a few tests.

  • Is this a refactor change? No

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@zhassan-aws zhassan-aws requested a review from a team as a code owner December 16, 2022 02:22
@zhassan-aws zhassan-aws changed the title Introduce a kani cover macro Introduce a kani macro for checking coverage Dec 16, 2022
@tedinski
Copy link
Contributor

Hmm, was this always going to be a macro, not just a function, and I missed that when I glanced at the RFC before?

Why a macro over just a function? Is it just the messages part? Is that essential?

I want to make sure the disadvantages of a macro (complexity, compile time, IDE autocomplete problems) are actually needed for this...

@zhassan-aws
Copy link
Contributor Author

Hmm, was this always going to be a macro, not just a function, and I missed that when I glanced at the RFC before?

No, it was originally a function, but the discussion on the RFC led to introducing a macro. See #1906 (comment).

The function still exists though, and is public, so can be used directly.

Copy link
Contributor

@celinval celinval left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

kani-driver/src/cbmc_property_renderer.rs Outdated Show resolved Hide resolved
kani-driver/src/cbmc_property_renderer.rs Show resolved Hide resolved
library/kani/src/lib.rs Show resolved Hide resolved
Description: "cover condition: sorted != arr"\
main.rs:13:5 in function cover_sorted

** 0 of 2 cover properties unsatisfiable
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we invert the report? I think we should report the number of covers that were satisfied.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree an inverted report is better (I made the change). The reason I didn't do it initially, is that an inverted report is a bit inconsistent with the report for normal checks where we reported the number of things that went wrong (failures) as opposed to those that were successful (similarly, the unsatisfiable covers are the ones that are unexpected).

I'm pro inverting the summary for normal checks as well (perhaps in a separate PR).

Copy link
Contributor

@celinval celinval left a comment

Choose a reason for hiding this comment

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

Thanks!

@zhassan-aws zhassan-aws merged commit 81a8f8f into model-checking:main Dec 16, 2022
@zhassan-aws zhassan-aws deleted the cover-properties branch December 16, 2022 23:41
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.

Create a cover statement to RMC
3 participants