Skip to content

Conversation

@feliperodri
Copy link
Contributor

@feliperodri feliperodri commented Feb 7, 2023

We can take advantage of any_raw_internal to set all values in an array to nondet.
Kani users may now use any_array() with a loop-free operation for trivial types,
which improves performance.

Signed-off-by: Felipe R. Monteiro [email protected]

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.

@feliperodri feliperodri self-assigned this Feb 7, 2023
@celinval
Copy link
Contributor

celinval commented Feb 7, 2023

I'm curious, how does that work?

@feliperodri feliperodri changed the title Make AnySlice::new() loop free Create Arbitrary::anyArray() Mar 9, 2023
@feliperodri feliperodri changed the title Create Arbitrary::anyArray() Create Arbitrary::any_array() Mar 9, 2023
@feliperodri feliperodri changed the title Create Arbitrary::any_array() Create Arbitrary::any_array() Mar 9, 2023
@feliperodri feliperodri added the [C] Feature / Enhancement A new feature request or enhancement to an existing feature. label Mar 9, 2023
@feliperodri feliperodri marked this pull request as ready for review March 9, 2023 22:12
@feliperodri feliperodri requested a review from a team as a code owner March 9, 2023 22:12
@feliperodri feliperodri requested a review from celinval March 9, 2023 22:31
@zhassan-aws
Copy link
Contributor

Can you clarify what the new version of the PR is doing? Is it providing a new way to create non-det arrays:

let arr: [u8; 10] = kani::Arbitrary::any_array();

? If so, how does it compare to the existing one:

let arr: [u8; 10] = kani::any();

and are there any advantages to the new approach?

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.

Weren't you planning to also make changes to any_slice?

@celinval
Copy link
Contributor

Can you clarify what the new version of the PR is doing? Is it providing a new way to create non-det arrays:

let arr: [u8; 10] = kani::Arbitrary::any_array();

? If so, how does it compare to the existing one:

let arr: [u8; 10] = kani::any();

and are there any advantages to the new approach?

With the new approach, we can directly assign a nondet array without initializing each element individually for types where this operation is safe.

@zhassan-aws
Copy link
Contributor

With the new approach, we can directly assign a nondet array without initializing each element individually for types where this operation is safe.

Can you clarify how this is achieved? The any_array implementation:

[(); MAX_ARRAY_LENGTH].map(|_| Self::any())

seems to be doing the same thing that the Arbitrary implementation for arrays was doing:

[(); N].map(|_| T::any())

@feliperodri feliperodri force-pushed the nondet-anyslice branch 2 times, most recently from cf3a0ee to d956867 Compare March 10, 2023 16:03
@feliperodri
Copy link
Contributor Author

Weren't you planning to also make changes to any_slice?

@celinval no changes to any_slice() on this PR. I want to focus this one only on implementing a version of any_array(). We can address any changes to any_slice() on a separate PR.

@feliperodri
Copy link
Contributor Author

feliperodri commented Mar 10, 2023

With the new approach, we can directly assign a nondet array without initializing each element individually for types where this operation is safe.

Can you clarify how this is achieved? The any_array implementation:

[(); MAX_ARRAY_LENGTH].map(|_| Self::any())

seems to be doing the same thing that the Arbitrary implementation for arrays was doing:

[(); N].map(|_| T::any())

@zhassan-aws you could see this is a refactoring, yes. Maybe I should update the description of this PR to make it more clear.

@feliperodri feliperodri requested a review from celinval March 10, 2023 16:22
@celinval
Copy link
Contributor

With the new approach, we can directly assign a nondet array without initializing each element individually for types where this operation is safe.

Can you clarify how this is achieved? The any_array implementation:

[(); MAX_ARRAY_LENGTH].map(|_| Self::any())

seems to be doing the same thing that the Arbitrary implementation for arrays was doing:

[(); N].map(|_| T::any())

The difference is in the implementation of arbitrary_trivial!() macro, which is how we define the implementation for the trivial types which have no contraint in their values:

            fn any_array<const MAX_ARRAY_LENGTH: usize>() -> [Self; MAX_ARRAY_LENGTH]
            where
                [(); { std::mem::size_of::<[$type; MAX_ARRAY_LENGTH]>() }]:,
            {
                unsafe {
                    crate::any_raw_internal::<
                        [Self; MAX_ARRAY_LENGTH],
                        { std::mem::size_of::<[Self; MAX_ARRAY_LENGTH]>() },
                    >()
                }

Note that we directly call any_raw_internal with the array type.

Copy link
Contributor

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

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

I see. Can you point out how any_slice will use it?

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.

It looks good to me. Please address @zhassan-aws comments before merging.

Copy link
Contributor

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

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

Thanks. Please update the PR description as well.

@feliperodri feliperodri enabled auto-merge (squash) March 10, 2023 22:20
Copy link
Contributor

@zhassan-aws zhassan-aws left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks @feliperodri!

@zhassan-aws
Copy link
Contributor

It would be useful to add a test (perhaps in a follow-up PR) that makes sure that a program involving an array of a primitive type does not involve loops. This can be verified by adding a #[unwind(0)] annotation so that the test would fail if a loop is generated.

We can take advantage of `any_raw_internal` to set all values in an array to nondet.
Kani users may now use `any_array()` with a loop-free operation for trivial types,
which improves performance.

Signed-off-by: Felipe R. Monteiro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[C] Feature / Enhancement A new feature request or enhancement to an existing feature.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants