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

RFC: Better temporary lifetimes (so e.g. .as_slice() works) #66

Merged
merged 1 commit into from
Jun 19, 2014

Conversation

lilyball
Copy link
Contributor

@lilyball lilyball commented May 4, 2014

@lilyball
Copy link
Contributor Author

lilyball commented May 4, 2014

cc @nikomatsakis

@bvssvni
Copy link

bvssvni commented May 6, 2014

+1

This RFC will make designs like Rust-Graphics more convenient.

@bjadamson
Copy link

+1 this is nice. Any ideas on how difficult the implementation would be?

@pnkfelix
Copy link
Member

pnkfelix commented May 6, 2014

it would probably be good for this RFC to reference Niko's blog post on this topic where he outlines a number of alternatives (that I think include, or at least subsume, the one proposed in the RFC):

http://www.smallcultfollowing.com/babysteps/blog/2014/01/09/rvalue-lifetimes-in-rust/


Since you said you could not think of any drawbacks, here is one I would suggest: some people may actually find the simple-minded current rules attractive, compared to having to reason about potential lifetime extension of an R-value due to how a reference to some part of it was returned.

In other words, some people may think the provided code fragment is ugly:

let x = os::args();
let x = x.slice_from(1);

but compare that to having to reason about when the destructor might run in:

let y = create_rvalue_with_dtor().and_call_method_on_it();
...

@lilyball
Copy link
Contributor Author

lilyball commented May 6, 2014

@pnkfelix The reason I disagree with you is that the latter code snippet is illegal to write today (assuming the method takes &self; if it takes self then it obviously consumes the receiver immediately). What's more, the destructor will run at the same point it will if you say

let y = create_rvalue_with_dtor();
let z = y.and_call_method_on_it();

@lilyball
Copy link
Contributor Author

lilyball commented May 6, 2014

@pnkfelix I just re-read that blog post. I may be worth referencing, but it doesn't actually include my proposed solution here. The options are basically, extend the lifetime of all temporaries, or extend the lifetime in response to the & operator or ref binding. My proposal says that the lifetime of references to temporaries should be tracked to see if they escape into the let binding, and to extend only those temporaries where they do. This would allow the lifetime to be tracked through method calls, which will let me say something like let args = os::args().slice_from(1);.

Theoretically the lifetime could also be tracked to see if it escapes into an enclosing block (e.g. let x = { os::args().slice_from(1) }), but I'm not covering that here. I think it makes sense to extend the lifetime due to let, but I'm not so sure it makes sense to extend the lifetime past a block scope.

@pnkfelix
Copy link
Member

pnkfelix commented May 7, 2014

@kballard the point of my presenting the snippet:

let y = create_rvalue_with_dtor().and_call_method_on_it();
...

was that your proposal, if I understand it correctly, makes it legal.

And thus when the dtor runs for the temporary rvalue is not apparent from the block structure of the program. The dtor may run at the end of this statement, or the temporary value may be kept alive for as long as y is alive.

I was not trying to claim that the snippet is legal today; since you took the time to point out that it is not legal today, I worry that you misunderstood my point, which is why I am belaboring the matter now. The fact that it is not legal today is what leads one to be forced to explicitly bind the temporary rvalue, thus making when its destructor runs immediately obvious from the block structure.

In short, the snippet is meant to show a potential drawback of your proposed change. I am not saying it would be a deal-breaker, but I want to make sure people are aware of it. (assuming that I am actually understanding things correctly.)

@lilyball
Copy link
Contributor Author

lilyball commented May 7, 2014

@pnkfelix Ok, I get you. I don't agree that it's an issue in practice, though. If you're just trying to understand the code you're reading, well, just look at the type of y. If it contains a lifetime (besides 'static) then the lifetime had to come from your create_rvalue_with_dtor() result, which means it's prolonging the life of the rvalue. And if you don't know the type of y, well, then I think you have bigger things to worry about for your understanding of the code than when the destructor is called.

Conversely, if you're writing the code and you want precise control over when the destructor runs, then put it in its own variable.

@nikomatsakis
Copy link
Contributor

I have come to the opinion that (1) we need a more flexible set of temporary lifetime rules and (2) the only way to have a more flexible set of rules is to employ region inference. I haven't had time to read this RFC yet, so I can't say whether it describes what I have in mind. I think there are however several good and convincing reasons to have more flexible rules, and it's not just convenience -- it's about expressiveness, safety, and abstraction.

@nikomatsakis
Copy link
Contributor

I'll try to write up my thoughts in more detail soon.

@nikomatsakis
Copy link
Contributor

Generally I feel pretty positive. This proposal is an interesting
middle ground between full inference and syntactically based
rules. Effectively, it extends the current purely syntactic rules with
a small amount of semantic information. It will certainly alleviate
the major pain points in today's system and also addresses my single
biggest concern.

For reference, my biggest concern with today's rules is that writing
let x = Foo { a: &a(), b: &b() } will compile successfully, but
writing let x = Foo(&a(), &b()), where Foo is a constructor
function will not.

One thing I also like about this proposal is that it leaves open the
door to using full inference later, if we wanted to handle examples like:

let y = {
  let x = &mut ...;
  ...;
  x
};

I think supporting constructs like that could be important, particularly as the output macro expressions, but it's a bridge we can cross later.

@thehydroimpulse
Copy link

👍 This opens the doors to more expressive APIs.

@nikomatsakis
Copy link
Contributor

Conclusion in meeting was that we liked this general approach. I will merge RFC accordingly and open a tracking issue. No one is currently assigned or planning to implement it yet.

Some unresolved issues that will have to be addressed and specified more precisely:

  1. This implies that the lifetimes of temporaries is not known until after typeck. I think this is ok but it is a phase change which can sometimes be tricky (currently temporary lifetimes are known before typeck).
  2. We have to specify the precise rules over when a temporary is extended. There are various subtle cases to be considered:
  • Clearly we must consider whether the parameter type is a reference with a lifetime that also appears in the return type. Does the variance in the return type matter? (I think: no, too subtle and not worth it.)
  • When do we decide what the type of the parameter is? Do we consider the declared type, the type after inference, or a hybrid?

Some examples where this matters:

fn identity<T>(x: T) -> T { x }

// Are these the same or different?

foo(&3);
foo::<&int>(&3);

My take: Probably we should just considered the fully inferred type.

In contrast, I plan to recommend that coercions are only applied based on the types of parameters after substitution of explicit type parameters but before anything else. But that is different, because we must decide what coercions to do during typeck, whereas we have the liberty (I think) of deciding temporary lifetimes after typeck. Also that plan is not decided. Anyway, these are open questions to be settled during implementation and afterwards.

@nikomatsakis nikomatsakis merged commit 2d3ff42 into rust-lang:master Jun 19, 2014
@nikomatsakis
Copy link
Contributor

Tracking issue: rust-lang/rust#15023

@lilyball lilyball deleted the better-temporary-lifetimes branch August 27, 2014 03:21
withoutboats pushed a commit to withoutboats/rfcs that referenced this pull request Jan 15, 2017
@Centril Centril added the A-lifetimes Lifetime related proposals. label Nov 23, 2018
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2022
…ro, r=m-ou-se

Add a stack-`pin!`-ning macro to `core::pin`.

  - rust-lang#93178

`pin!` allows pinning a value to the stack. Thanks to being implemented in the stdlib, which gives access to `macro` macros, and to the private `.pointer` field of the `Pin` wrapper, [it was recently discovered](https://rust-lang.zulipchat.com/#narrow/stream/187312-wg-async-foundations/topic/pin!.20.E2.80.94.20the.20.22definitive.22.20edition.20.28a.20rhs-compatible.20pin-nin.2E.2E.2E/near/268731241) ([archive link](https://zulip-archive.rust-lang.org/stream/187312-wg-async-foundations/topic/A.20rhs-compatible.20pin-ning.20macro.html#268731241)), contrary to popular belief, that it is actually possible to implement and feature such a macro:

```rust
let foo: Pin<&mut PhantomPinned> = pin!(PhantomPinned);
stuff(foo);
```
or, directly:

```rust
stuff(pin!(PhantomPinned));
```

  - For context, historically, this used to require one of the two following syntaxes:

      - ```rust
        let foo = PhantomPinned;
        pin!(foo);
        stuff(foo);
        ```

      -  ```rust
         pin! {
             let foo = PhantomPinned;
         }
         stuff(foo);
         ```

This macro thus allows, for instance, doing things like:

```diff
fn block_on<T>(fut: impl Future<Output = T>) -> T {
    // Pin the future so it can be polled.
-   let mut fut = Box::pin(fut);
+   let mut fut = pin!(fut);

    // Create a new context to be passed to the future.
    let t = thread::current();
    let waker = Arc::new(ThreadWaker(t)).into();
    let mut cx = Context::from_waker(&waker);

    // Run the future to completion.
    loop {
        match fut.as_mut().poll(&mut cx) {
            Poll::Ready(res) => return res,
            Poll::Pending => thread::park(),
        }
    }
}
```

  - _c.f._, https://doc.rust-lang.org/1.58.1/alloc/task/trait.Wake.html

And so on, and so forth.

I don't think such an API can get better than that, barring full featured language support (`&pin` references or something), so I see no reason not to start experimenting with featuring this in the stdlib already 🙂

  - cc `@rust-lang/wg-async-foundations` \[EDIT: this doesn't seem to have pinged anybody 😩, thanks `@yoshuawuyts` for the real ping\]

r? `@joshtriplett`

___

# Docs preview

https://user-images.githubusercontent.com/9920355/150605731-1f45c2eb-c9b0-4ce3-b17f-2784fb75786e.mp4

___

# Implementation

The implementation ends up being dead simple (so much it's embarrassing):

```rust
pub macro pin($value:expr $(,)?) {
    Pin { pointer: &mut { $value } }
}
```

_and voilà_!

  - The key for it working lies in [the rules governing the scope of anonymous temporaries](https://doc.rust-lang.org/1.58.1/reference/destructors.html#temporary-lifetime-extension).

<details><summary>Comments and context</summary>

This is `Pin::new_unchecked(&mut { $value })`, so, for starters, let's
review such a hypothetical macro (that any user-code could define):
```rust
macro_rules! pin {( $value:expr ) => (
    match &mut { $value } { at_value => unsafe { // Do not wrap `$value` in an `unsafe` block.
        $crate::pin::Pin::<&mut _>::new_unchecked(at_value)
    }}
)}
```

Safety:
  - `type P = &mut _`. There are thus no pathological `Deref{,Mut}` impls that would break `Pin`'s invariants.
  - `{ $value }` is braced, making it a _block expression_, thus **moving** the given `$value`, and making it _become an **anonymous** temporary_.
    By virtue of being anonynomous, it can no longer be accessed, thus preventing any attemps to `mem::replace` it or `mem::forget` it, _etc._

This gives us a `pin!` definition that is sound, and which works, but only in certain scenarios:

  - If the `pin!(value)` expression is _directly_ fed to a function call:
    `let poll = pin!(fut).poll(cx);`

  - If the `pin!(value)` expression is part of a scrutinee:

    ```rust
    match pin!(fut) { pinned_fut => {
        pinned_fut.as_mut().poll(...);
        pinned_fut.as_mut().poll(...);
    }} // <- `fut` is dropped here.
    ```

Alas, it doesn't work for the more straight-forward use-case: `let` bindings.

```rust
let pinned_fut = pin!(fut); // <- temporary value is freed at the end of this statement
pinned_fut.poll(...) // error[E0716]: temporary value dropped while borrowed
                     // note: consider using a `let` binding to create a longer lived value
```

  - Issues such as this one are the ones motivating rust-lang/rfcs#66

This makes such a macro incredibly unergonomic in practice, and the reason most macros out there had to take the path of being a statement/binding macro (_e.g._, `pin!(future);`) instead of featuring the more intuitive ergonomics of an expression macro.

Luckily, there is a way to avoid the problem. Indeed, the problem stems from the fact that a temporary is dropped at the end of its enclosing statement when it is part of the parameters given to function call, which has precisely been the case with our `Pin::new_unchecked()`!

For instance,

```rust
let p = Pin::new_unchecked(&mut <temporary>);
```

becomes:

```rust
let p = { let mut anon = <temporary>; &mut anon };
```

However, when using a literal braced struct to construct the value, references to temporaries can then be taken. This makes Rust change the lifespan of such temporaries so that they are, instead, dropped _at the end of the enscoping block_.

For instance,
```rust
let p = Pin { pointer: &mut <temporary> };
```

becomes:

```rust
let mut anon = <temporary>;
let p = Pin { pointer: &mut anon };
```

which is *exactly* what we want.

Finally, we don't hit problems _w.r.t._ the privacy of the `pointer` field, or the unqualified `Pin` name, thanks to `decl_macro`s being _fully_ hygienic (`def_site` hygiene).

</details>

___

# TODO

  - [x] Add compile-fail tests with attempts to break the `Pin` invariants thanks to the macro (_e.g._, try to access the private `.pointer` field, or see what happens if such a pin is used outside its enscoping scope (borrow error));
  - [ ] Follow-up stuff:
      - [ ] Try to experiment with adding `pin!` to the prelude: this may require to be handled with some extra care, as it may lead to issues reminiscent of those of `assert_matches!`: rust-lang#82913
      - [x] Create the tracking issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Lifetime related proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants