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 tests for issues related to std linking #1801

Merged
merged 2 commits into from
Oct 26, 2022

Conversation

celinval
Copy link
Contributor

Description of changes:

Add tests for #564, #208 and #87.

Resolved issues:

Resolves #564
Resolves #1620

Related RFC:

#1588

Call-outs:

Unfortunately, it looks like there are still issues with with how we are encoding panic threads and dynamic objects which are causing two of the test cases to fail.

I'll document my findings in the original issue for each.

Testing:

  • How is this change tested? New 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.

Unfortunately, it looks like there are still issues with with how we are
encoding panic threads and dynamic objects which are causing two of the
test cases to fail.

I'll document my findings in the original issue for each.
@celinval celinval requested a review from a team as a code owner October 26, 2022 00:12
@@ -0,0 +1,49 @@
// Copyright Kani Contributors
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this example comes from a book, shouldn't we use the copyright for modifications?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point

}

#[kani::proof]
#[kani::unwind(10)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this annotation needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me double check. I think I just copied from the issue description.

@@ -1,12 +1,13 @@
// Copyright Kani Contributors
// SPDX-License-Identifier: Apache-2.0 OR MIT
// This test takes too long with all the std symbols. Use --legacy-linker for now.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be updated. Maybe you can remove and explain that original takes too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

@celinval celinval merged commit f306a44 into model-checking:main Oct 26, 2022
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.

Enable fixme tests Multiple failures in overlapping traits example
2 participants