generated from amazon-archives/__template_Apache-2.0
-
Notifications
You must be signed in to change notification settings - Fork 129
RFC: Unstable APIs #2281
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
Merged
celinval
merged 7 commits into
model-checking:main
from
celinval:issue-2279-unstable-api
Apr 5, 2023
Merged
RFC: Unstable APIs #2281
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
45a4248
RFC for unstable APIs
celinval 00030c6
Add PR link and summary link
celinval cd4d939
Change error to compilation error + other PR comments
celinval 59de98c
Merge remote-tracking branch 'origin/main' into issue-2279-unstable-api
celinval 7c2a73d
Address feedback
celinval ab96858
Update rfc/src/rfcs/0006-unstable-api.md
celinval e7ee011
Merge branch 'main' into issue-2279-unstable-api
celinval File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| - **Feature Name:** Unstable APIs | ||
| - **RFC Tracking Issue**: <https://github.com/model-checking/kani/issues/2279> | ||
| - **RFC PR:** <https://github.com/model-checking/kani/pull/2281> | ||
| - **Status:** Under Review | ||
| - **Version:** 0 | ||
|
|
||
| ------------------- | ||
|
|
||
| ## Summary | ||
|
|
||
| Provide a standard option for users to enable experimental APIs and features in Kani, | ||
| and ensure that those APIs are off by default. | ||
|
|
||
| ## User Impact | ||
|
|
||
| The impact should be minimal, and it is very similar to the current experience when they are trying an experimental feature in Kani. | ||
| The unstable API will be given a unique identifier, which the user will enable either via command line or via command argument. | ||
adpaco-aws marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ## User Experience | ||
celinval marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Users will have to invoke Kani with: | ||
| ``` | ||
| --enable-unstable --unstable-feature <feature_identifier> | ||
celinval marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ``` | ||
| in order to enable an API in the Kani library that is marked as unstable. | ||
|
|
||
| In order to mark an API as unstable, we will add the following attribute to the APIs marked as unstable: | ||
|
|
||
| ```rust | ||
| #[kani::unstable(feature="<IDENTIFIER>", issue="<TRACKING_ISSUE_NUMBER>", reason="<OPTIONAL_DESCRIPTION>")] | ||
| pub fn unstable_api() {} | ||
| ``` | ||
|
|
||
| This is similar to the interface used by [the standard library](https://rustc-dev-guide.rust-lang.org/stability.html#unstable). | ||
|
|
||
| If the user tries to run Kani without enabling an unstable feature that is reachable, | ||
celinval marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| the verification may fail if the unstable feature is reachable. | ||
| I.e., the behavior will be similar to unsupported features. | ||
celinval marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| The error will be delayed and only fail during the verification of harnesses that actually trigger the feature. | ||
|
|
||
| ## Detailed Design | ||
|
|
||
| We will add the `--unstable-feature` option to both, `kani-driver` and `kani-compiler`. | ||
celinval marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Kani driver will just pass the information to the compiler. The compiler in its turn will stub | ||
| out any unsupported feature API. | ||
|
|
||
| Let's say we introduce the following API: | ||
| ```rust | ||
| #[kani::unstable(feature="foo_bar", issue="2279", reason="Just an example")] | ||
| pub fn unstable_foo<T>() -> T { | ||
| bar() | ||
| } | ||
| ``` | ||
|
|
||
| The compiler will replace the body of the API with a reachability check if the feature `foo_bar` was | ||
| not enabled. I.e., this will generate the function body similar to running: | ||
|
|
||
| ```rust | ||
| pub fn unstable_foo<T>() -> T { | ||
| kani::unstable("unstable_foo", reason, issue") | ||
celinval marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| ``` | ||
|
|
||
| Where `kani::unstable` is a Kani intrinsic that generate the a new `UnstableFeature` check. | ||
|
|
||
| Finally, we will modify the driver to treat `UnstableFeature` failures the same way it handles | ||
| unsupported features. | ||
|
|
||
| ### API Stabilization | ||
|
|
||
| Once an API has been stabilize, we will remove the `unstable` attributes from the given API. | ||
| If the user tries to enable a feature that was already stabilized, | ||
| Kani will print a warning stating that the feature has been stabilized. | ||
|
|
||
| ### API Removal | ||
|
|
||
| If we decide to remove an API that is marked as unstable, we should follow a regular deprecation | ||
| path (using `#[deprecated]` attribute), and keep the `unstable` flag + attributes, until we are | ||
| ready to remove the feature completely. | ||
|
|
||
| ## Rational and Alternatives | ||
|
|
||
| For this RFC, my suggestion is to only enable experimental features globally for simplicity of use and implementation. | ||
|
|
||
| We could allow users to specify experimental features on a per-harness basis, | ||
| but it could be tricky to make it clear to the user which harness may be affected by which feature. | ||
| The extra granularity would also be painful when we decide a feature is no longer experimental, | ||
| whether it is stabilized or removed. | ||
| In those cases, users would have to edit each harness that enables the affected feature. | ||
|
|
||
|
|
||
| ## Open questions | ||
|
|
||
| - Should we fail the verification if we decide to deprecate an unstable API? | ||
|
|
||
| ## Future possibilities | ||
|
|
||
| - Allow users to enable unstable features on a per-harness base. | ||
celinval marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.