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

Add harnesses for all functions in Alignment #42

Merged
merged 15 commits into from
Aug 20, 2024

Conversation

tautschnig
Copy link
Member

In #33 a contract was added to Alignment::new_unchecked, but its verification was only implicit through Layout, and may be affected by future changes to the contract that was added to Layout. This commit remedies this by adding a separate harness just for Alignment.

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

In model-checking#33 a contract was added to `Alignment::new_unchecked`, but its
verification was only implicit through `Layout`, and may be affected by
future changes to the contract that was added to `Layout`. This commit
remedies this by adding a separate harness just for `Alignment`.
@tautschnig tautschnig changed the title Add harness for Alignment::new_unchecked Add harnesses for all functions in Alignment Jul 24, 2024
Copy link

@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.

Looks good to me. Thanks

library/core/src/ptr/alignment.rs Show resolved Hide resolved
@feliperodri feliperodri self-requested a review July 30, 2024 19:10
library/core/src/ptr/alignment.rs Show resolved Hide resolved
library/core/src/ptr/alignment.rs Show resolved Hide resolved
library/core/src/ptr/alignment.rs Outdated Show resolved Hide resolved
library/core/src/ptr/alignment.rs Outdated Show resolved Hide resolved
@tautschnig tautschnig assigned feliperodri and unassigned tautschnig Aug 9, 2024
Copy link

@feliperodri feliperodri left a comment

Choose a reason for hiding this comment

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

LGTM, but did you try the safety_constraint?

@tautschnig
Copy link
Member Author

LGTM, but did you try the safety_constraint?

No, because I can't get Arbitrary to work in the first place, see #42 (comment). Perhaps all this will work when upgrading Kani?

@celinval
Copy link

LGTM, but did you try the safety_constraint?

No, because I can't get Arbitrary to work in the first place, see #42 (comment). Perhaps all this will work when upgrading Kani?

Please see my reply above

@celinval
Copy link

Why did you revert the impl Arbitrary? Isn't that what we want (except for the transmute call).

@tautschnig
Copy link
Member Author

Why did you revert the impl Arbitrary? Isn't that what we want (except for the transmute call).

I thought it was causing the compile errors, but then I realised it’s #49 that’s needed. Will continue on this once Kani and this repo are back in a working state.

@tautschnig
Copy link
Member Author

Why did you revert the impl Arbitrary? Isn't that what we want (except for the transmute call).

I thought it was causing the compile errors, but then I realised it’s #49 that’s needed. Will continue on this once Kani and this repo are back in a working state.

Now successfully using Arbitrary.

@tautschnig tautschnig merged commit 03c735b into model-checking:main Aug 20, 2024
6 checks passed
@tautschnig tautschnig deleted the align-harness branch August 20, 2024 12:25
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.

4 participants