-
Notifications
You must be signed in to change notification settings - Fork 525
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
Explain our stability attributes #424
Conversation
src/stability.md
Outdated
# unstable | ||
|
||
The `#[unstable(feature = "foo", issue = "1234", reason = "lorem ipsum")]` attribute explicitly | ||
marks an item as unstable. This infects all sub-items, where the attribute doesn't have to be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens for
#[stable]
mod foo {
#[unstable]
fn bar() {}
}
Or
#[unstable]
mod foo {
#[stable]
fn bar() {}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I be explaining that? I assumed that to be obvious, but since you're asking it seems that it is not. It's essentially the same rules that pub
has
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a good idea to add a note re. "essentially pub
".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first example makes some amount of sense to me intuitively, but the second doesn't. How do I use a stable function inside of an unstable module? Wouldn't I get a warning just by accessing the module to import the function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar to pub
, this pattern is often used when the function is reexported elsewhere, but the module is still unstable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is worth explaining more, actually. In particular, there is a limitation here in our stability check which does make the location of the stable item in the unstable module available to stable code, so the module cannot be renamed or removed any more. The document should note that problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The relevant Rust bug is rust-lang/rust#15702.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; thanks!
(I don't have r+ here -- @pietroalbini can that be arranged for?)
I guess ask Niko? |
cc @nikomatsakis ^--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Looks good to me, unsure if you wanted to add more information or not, ended not being very clear to me from the conversations. |
src/stability.md
Outdated
the features need an implementation in `qualify_min_const_fn.rs`. For example the `const_fn_union` | ||
feature gate allows accessing fields of unions inside stable `const fn`s. The rules for when it's | ||
ok to use such a feature gate are that behavior matches the runtime behavior of the same code | ||
(see also https://www.ralfj.de/blog/2018/07/19/const.html). This means that you may not create a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be made into an actual link please? e.g. See this [blog post](https://www.ralfj.de/blog/2018/07/19/const.html)
.
Hmm... I was trying to update this. It seems I broke it... I will create a new PR and merge it... |
I also left some comments in the text that seem to be gone now...?!? |
r? @Centril