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

Complete tests for count intrinsics #883

Merged
merged 3 commits into from
Mar 14, 2022

Conversation

adpaco-aws
Copy link
Contributor

Description of changes:

Adds more complete tests for count intrinsics. Removes the old tests that were based on concrete values.

Resolved issues:

Resolves #26
Part of #727

Call-outs:

Testing:

  • How is this change tested? Existing regression + changes to 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.

@adpaco-aws adpaco-aws requested a review from a team as a code owner March 2, 2022 23:50
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.

Those are nice tests. I added minor comments.

use std::intrinsics::cttz_nonzero;

// Call `cttz_nonzero` with an unconstrained symbolic argument
macro_rules! test_cttz_nonzero {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this named nonzero?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it is testing the cttz_nonzero intrinsic.

@@ -0,0 +1,25 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR MIT
// kani-verify-fail
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this test pass if just one property fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, however they are all encoded the same way so it is very likely that if one stops failing (for any reason) the rest will stop failing too.

// exclude zero
macro_rules! test_ctlz_nonzero {
($ty:ty) => {
let var_nonzero: $ty = 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intended to be assigned to kani::any() instead of 8?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks for catching it! Fixed 😄

@adpaco-aws adpaco-aws merged commit e33c56a into model-checking:main Mar 14, 2022
@adpaco-aws adpaco-aws mentioned this pull request Mar 30, 2022
tedinski pushed a commit to tedinski/rmc that referenced this pull request Apr 22, 2022
* Complete tests for count intrinsics

* Use `kani::any()` instead of concrete value in `test_ctlz_nonzero`
tedinski pushed a commit to tedinski/rmc that referenced this pull request Apr 25, 2022
* Complete tests for count intrinsics

* Use `kani::any()` instead of concrete value in `test_ctlz_nonzero`
tedinski pushed a commit to tedinski/rmc that referenced this pull request Apr 26, 2022
* Complete tests for count intrinsics

* Use `kani::any()` instead of concrete value in `test_ctlz_nonzero`
tedinski pushed a commit that referenced this pull request Apr 27, 2022
* Complete tests for count intrinsics

* Use `kani::any()` instead of concrete value in `test_ctlz_nonzero`
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.

Simplify code for count intrinsics in next CBMC version
3 participants