Document RFC 1623: static lifetime elision. #37928

Merged
merged 4 commits into from Feb 9, 2017

Projects

None yet
@chriskrycho
Contributor
chriskrycho commented Nov 22, 2016 edited

This should be the last item required for stabilizing RFC 1623 (#35897).

@rust-highfive
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

src/doc/reference.md
-address will have the `static` lifetime. The compiler is, however, still at
-liberty to translate the constant many times, so the address referred to may not
-be stable.
+address will have the `static` lifetime. (See below on [static lifetime elision](#static-lifetime-elision).) The compiler is, however, still at liberty to
@frewsxcv
frewsxcv Nov 22, 2016 Member

Can you wrap this line to match the indentation of the surrounding lines?

@chriskrycho
chriskrycho Nov 22, 2016 Contributor

Agh, yes. I've updated it; I meant to fix that before I pushed it!

@steveklabnik
Contributor

So, @rust-lang/docs , this raises a question. How do we make the distinction between stable and unstable stuff here?

@chriskrycho
Contributor
@GuillaumeGomez
Member

It should have an unstable version. I don't have much better idea. :-/

@chriskrycho
Contributor

I just noted that all the code samples in reference.md get tested. Neat! I'll update the sample accordingly in the morning.

@frewsxcv
Member
frewsxcv commented Nov 23, 2016 edited

I don't have any great ideas either, other than explicitly mentioning that it's unstable in the paragraph above and adding in the #[feature(whatever_feature_name)] in the code example. At least we'll get a warning when it stabilizes.

@steveklabnik
Contributor

Putting this on this week's doc team agenda.

@llogiq
Contributor
llogiq commented Dec 14, 2016

We could change the feature implementation to my first commit, thus stabilizing the feature and removing the extra code adding the feature gate that makes it unstable in the first place. I presume this should make the doc tests pass.

@steveklabnik
Contributor

So, we talked about this at the docs meeting today.

What it really boils down to is this: it'd be a shame to block this based on figuring out the broader story here. However, once we know that the rest of the feature will be stable, we can just land this, then land the stabilization of the feature. @aturon just put the feature into FCP, so let's see how that goes before we merge.

And in the meantime, let's figure out the broader docs story. I'm going to open an issue on the RFCs repo about this.

@chriskrycho
Contributor

Related: I'll have time Friday or Saturday to update the not-the-feature-gate bits of the test that are broken here—there's one piece that needs to be tweaked independent of the stabilization/feature flag issue.

@chriskrycho
Contributor

Kept spacing this; came back to it via other issues. I'll update the non-feature-gate related parts tonight or tomorrow!

@llogiq
Contributor
llogiq commented Jan 3, 2017

@chriskrycho do you need any help?

@chriskrycho
Contributor

@llogiq gah. Slipped my mind. The only thing that needs tweaking is setting ignore on this example. It's re-using items from earlier in the reference, but they're not in scope for that specific example.

@chriskrycho
Contributor

I just pushed a fixed version of the commit which just includes all the required pieces. We'll see what Travis says…

@chriskrycho
Contributor

UGH. I pushed the rebase, but not the fix.

This is how my whole first day back at work has been. 🤦‍♂️

@chriskrycho
Contributor
@steveklabnik
Contributor

No worries!

@est31 est31 referenced this pull request Jan 23, 2017
Merged

Stabilize static lifetime in statics #39265

2 of 2 tasks complete
@llogiq

Actually this is full elision with a 'static default. This means that if the types contains fns, the usual elision rules will apply, only choosing 'static if no elision rule applies.

@chriskrycho
Contributor

@llogiq can you elaborate?

@steveklabnik I seem to have failed in pacifying Travis and the feature gate. Can you take a gander and indicate what I did wrong?

@llogiq
Contributor
llogiq commented Jan 31, 2017

This means if you have a static FOO : &Fn(&Bar) -> &Baz, you actually have a static FOO : &'static Fn<'a>(&'a Bar) -> &'a Baz for some unnamed 'a.

@est31
Contributor
est31 commented Jan 31, 2017

I seem to have failed in pacifying Travis and the feature gate.

I think you can fix it by adding # #![feature(...)] to the doc test. It won't show up in the output then due to the leading #, while fixing the issue. See also the book and the asm documentation, it uses this as well.

@chriskrycho
Contributor

@est31 ah, it's the extra #. Got it.

@llogiq do you have a suggested change to the write-up here?

src/doc/reference.md
+without the lifetimes. Returning to our previous example:
+
+```rust
+#[feature(static_in_const)]
@clarcharr
clarcharr Jan 31, 2017 Contributor

this should be #![feature(static_in_const)]; you forgot the exclamation point.

@chriskrycho
chriskrycho Jan 31, 2017 Contributor

YOU ARE MY HERO

@llogiq
Contributor
llogiq commented Jan 31, 2017

I think I could improve it, but I need to find the time first.

@llogiq

Also after the example, add a paragraph to explain the elision:

In case the static or constant items contain function references or closures, and those work with references, the compiler will try the usual elision rules (link to elision). Failing that, it will default the lifetimes to 'static. This is best explained by an example:

const FUN : fn(&str) -> &str = ..
// per rule #1, this is fn<'a>(&'a str) -> &'a str

const FUNNY : Fn(&Foo, &Bar, &Baz) -> usize = ..
// per rule #2, this is Fn<'a, 'b, 'c>(&'a Foo, &'b Bar, &'c Baz) -> usize

const UNFUNNY : Fn(&Foo, &Bar) -> &Baz = ..
// neither rule applies, so all `&`s are `'static`
src/doc/reference.md
@@ -1271,15 +1271,16 @@ guaranteed to refer to the same memory address.
Constant values must not have destructors, and otherwise permit most forms of
data. Constants may refer to the address of other constants, in which case the
-address will have the `static` lifetime. The compiler is, however, still at
+address will have the `static` lifetime. (See below on [static lifetime
@llogiq
llogiq Feb 2, 2017 Contributor

How about "will have elided lifetimes where applicable, otherwise – in most cases – defaulting to the 'static lifetime.

@est31
Contributor
est31 commented Feb 5, 2017

Now that beta branched, I would like to ask for beta backport nomination, so that already 1.16 would have it stabilized.

@llogiq
Contributor
llogiq commented Feb 5, 2017

Good idea.

@bors bors added a commit that referenced this pull request Feb 7, 2017
@bors bors Auto merge of #39265 - est31:master, r=nrc
Stabilize static lifetime in statics

Stabilize the "static_in_const" feature. Blockers before this PR can be merged:

* [x] The [FCP with inclination to stabilize](#35897 (comment)) needs to be over. FCP lasts roughly three weeks, so will be over at Jan 25, aka this thursday.
* [ ] Documentation needs to be added (#37928)

Closes #35897.
443866e
@steveklabnik
Contributor

@chriskrycho ping! any interest in keeping this going? this is still blocking stabilization.

@est31 I would be strongly against this; stabilization backports are incredibly rare.

@chriskrycho
Contributor

@steveklabnik yeah, should be able to allocate an hour to it tomorrow. I'll pull in @llogiq's comments, as well as fix the build issue, then.

@llogiq
Contributor
llogiq commented Feb 8, 2017

We may also want to show embedded lifetimes, e.g. Fn(&'a Foo) -> Bar<'a> can be shortened to Fn(&Foo) -> Bar.

@chriskrycho
Contributor

@llogiq regarding your last comment: did that change with this RFC/implementation? If not, I'd prefer to note it as something to add in a separate commit/PR. I think we actually need a dedicated section for lifetime elision in general; right now it appears to be documented in detail only in the nomicon.

@chriskrycho
Contributor

Updated with everything other than the embedded lifetimes scenario, and it passed rustup run nightly rustdoc --test ./doc/reference.md so we should finally be good to go. 🤞

@steveklabnik
Contributor

Travis passed!

@bors: r+ rollup

thanks a ton @chriskrycho

@bors
Contributor
bors commented Feb 8, 2017

📌 Commit 4096dd6 has been approved by steveklabnik

@chriskrycho
Contributor

YESSSSSSSSSSS. FINALLLYYYYYYYY.

Sorry for the many delays on this, everyone! Glad it's finally in. So it'll land in, what, 1.17?

@frewsxcv frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 9, 2017
@frewsxcv frewsxcv Rollup merge of #37928 - chriskrycho:document-rfc-1623, r=steveklabnik
Document RFC 1623: static lifetime elision.

This should be the last item required for stabilizing RFC 1623 (#35897).
cc455b3
@bors bors added a commit that referenced this pull request Feb 9, 2017
@bors bors Auto merge of #39663 - frewsxcv:rollup, r=frewsxcv
Rollup of 7 pull requests

- Successful merges: #37928, #39456, #39587, #39589, #39641, #39649, #39662
- Failed merges: #39586, #39595
606e273
@bors bors added a commit that referenced this pull request Feb 9, 2017
@bors bors Auto merge of #39663 - frewsxcv:rollup, r=frewsxcv
Rollup of 7 pull requests

- Successful merges: #37928, #39456, #39587, #39589, #39641, #39649, #39662
- Failed merges: #39586, #39595
9d0267a
@est31
Contributor
est31 commented Feb 9, 2017

@chriskrycho

So it'll land in, what, 1.17?

yes.

@frewsxcv frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 9, 2017
@frewsxcv frewsxcv Rollup merge of #37928 - chriskrycho:document-rfc-1623, r=steveklabnik
Document RFC 1623: static lifetime elision.

This should be the last item required for stabilizing RFC 1623 (#35897).
7f7dc76
@bors bors added a commit that referenced this pull request Feb 9, 2017
@bors bors Auto merge of #39677 - frewsxcv:rollup, r=frewsxcv
Rollup of 9 pull requests

- Successful merges: #37928, #38699, #39589, #39598, #39599, #39641, #39649, #39653, #39671
- Failed merges:
fd2f8a4
@bors bors merged commit 4096dd6 into rust-lang:master Feb 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bors bors added a commit that referenced this pull request Feb 9, 2017
@bors bors Auto merge of #39265 - est31:master, r=petrochenkov
Stabilize static lifetime in statics

Stabilize the "static_in_const" feature. Blockers before this PR can be merged:

* [x] The [FCP with inclination to stabilize](#35897 (comment)) needs to be over. FCP lasts roughly three weeks, so will be over at Jan 25, aka this thursday.
* [x] Documentation needs to be added (#37928)

Closes #35897.
1129ce5
@alexcrichton
Member

Removing beta nomination as sentiment seems to be to not backport (also see #35897)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment