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

Const projection from Pin<&Self> #92

Closed
ogoffart opened this issue Sep 16, 2019 · 9 comments · Fixed by #93
Closed

Const projection from Pin<&Self> #92

ogoffart opened this issue Sep 16, 2019 · 9 comments · Fixed by #93
Labels
A-pin-projection Area: #[pin_project] C-enhancement Category: A new feature or an improvement for an existing one
Milestone

Comments

@ogoffart
Copy link

In my pet crate, i have to work a lot with Pin<&T> but without the mut (because it uses Cell internally, but nothing to do with futures). And i end up needing to do lots of projections.

Would it make sense for the macro to also have a project_const(self: Pin<&'a Self>) -> FooProjectionConst<'a> ?

(Actually, I feel like it should just be project(), and the current &mut Self should be renamed project_mut for consistency with all the other functions that overload const/mut)

Is that something that make sense, or is using Pin<&Self> not a common pattern?

@taiki-e
Copy link
Owner

taiki-e commented Sep 16, 2019

Pin always implements Deref, so you can access fields without projection (i.e, projection should not be necessary for your use-case).

@ogoffart
Copy link
Author

Ah, but the member of the struct are !Unpin themselves. And and would like to get pinned references to them. It should be safe if the containing struct is also in a pin.

@ogoffart
Copy link
Author

Just to be a bit more concrete, the use case is something like that:
I have this struct Property that needs to be Pin, because it's address is registered somewhere else. All its public associated functions are taking self by Pin<&Self>. And it is meant to be used like this:

struct Container {
    aaa: Property,
    bbb: Property,
    ccc: Property,
}

Then, I'd put Container instance on the heap (typically in a Pin<Rc<Container>>
And I would like to be able to do something like container.project().aaa.get(); with only safe code.

@taiki-e
Copy link
Owner

taiki-e commented Sep 18, 2019

Thanks for sharing about your use-case. It certainly seems like a use-case that requires pin-projection.

However, as far as I know, most of the current use-cases of Pin API use Pin<&mut Self>, and the tools related to Pin API (pin-project, pin-utils, ergo-pin) are designed based on it. I'm not sure how common Pin<&Self> is. cc @RalfJung @withoutboats @Nemo157

@RalfJung
Copy link
Contributor

RalfJung commented Sep 18, 2019

The term "const" is very confusing here, that sounds like const fn. I assume you mean a shared reference as opposed to a mutable reference? &T is called a shared reference in Rust, not a "const" reference.

Sure, both make sense for projections, and both have their use-cases. Futures don't really need shared pinned references, but other cases like intrusive collections do.

@Nemo157
Copy link

Nemo157 commented Sep 18, 2019

The standard suffix to identify self-by-shared-reference (if one is used) is _ref. Swapping project to go via shared reference and adding project_mut could also make sense, but even if other cases like intrusive collections become more widely used I think pinned-unique-reference is probably going to be enough of a majority to have it be the default.

Re other utilities: pin-utils should also probably provide unsafe_pinned_ref or similar; but the stack pinning utilities don't need to deal with shared references, they must take an owned value so can always produce a unique reference, which you could then choose to downgrade to a shared reference.

@ogoffart
Copy link
Author

The unsafe_pinned! macro for pin-utils was simple enough that i could copy-paste these few lines of code in my crate and remove the mut :-). But that's not as simple with a procedural macro.

I agree that project_ref would be a better name than project_const.

@taiki-e
Copy link
Owner

taiki-e commented Sep 18, 2019

Sure, both make sense for projections, and both have their use-cases. Futures don't really need shared pinned references, but other cases like intrusive collections do.

Oh, you're right.

even if other cases like intrusive collections become more widely used I think pinned-unique-reference is probably going to be enough of a majority to have it be the default.

I agree with this.


Adding this seems to make sense, so I will add project_ref after merging #90.

@taiki-e
Copy link
Owner

taiki-e commented Sep 18, 2019

By the way, maybe #[project_ref] also needs to be added because .project() and .project_ref() return different projected types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pin-projection Area: #[pin_project] C-enhancement Category: A new feature or an improvement for an existing one
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants