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

Decide on name for derive(SmartPtr) #129104

Closed
traviscross opened this issue Aug 14, 2024 · 68 comments
Closed

Decide on name for derive(SmartPtr) #129104

traviscross opened this issue Aug 14, 2024 · 68 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-derive_smart_pointer `#![feature(derive_smart_pointer)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@traviscross
Copy link
Contributor

traviscross commented Aug 14, 2024

We need to resolve the open question on:

...regarding what name to use for the thing formerly known an #[derive(SmartPtr)] or #[derive(SmartPointer)].

Tracking:

@traviscross traviscross added T-lang Relevant to the language team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. I-lang-nominated Nominated for discussion during a lang team meeting. labels Aug 14, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 14, 2024
@traviscross traviscross removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 14, 2024
@ChrisDenton
Copy link
Member

I'll say I don't like the term "smart pointer" in Rust. It has a more specific meaning in C++ but porting that terminology over to Rust has been less than smooth. Like the Deref docs still imply that anything implementing the Deref trait is a "smart pointer" which is a bit of stretch on its own. But now #[derive(SmartPtr)] doesn't imply Deref and everything gets even more muddy.

I'd rather we use terms we can fully define without confusion. This feels similar to the "object safe" debate in that one aspect.

@Darksonn
Copy link
Contributor

One option is to name it in relation to what it enables. For example: #[derive(DynamicDispatch)] or #[derive(DynamicPtr)].

@rustbot label F-derive_smart_pointer

@crlf0710
Copy link
Member

My personal two cents :)

#[repr(owned_pointer)] // this also implies `#[repr(transparent)]` somehow
struct MyBox<T: ?Sized>(Box<T>);

@Darksonn
Copy link
Contributor

@crlf0710 Owned pointer is not good because it can be used with pointers that don't have ownership of the value.

I propose to use the name #[derive(DynamicPtr)]. Any objections?

@nikomatsakis
Copy link
Contributor

One option is to name it in relation to what it enables

I like this principle...

For example: #[derive(DynamicDispatch)] or #[derive(DynamicPtr)].

...but neither of these is "self explanatory" to me. What it enables specifically is the possibility of self: MyType<dyn Bar> appearing in traits and supporting dynamic dispatch, basically? Is that it? I am trying to page this back in.

@Darksonn
Copy link
Contributor

Darksonn commented Oct 2, 2024

It does two things. One, it allows coercions from MySmartPointer<ConcreteType> to MySmartPointer<dyn MyTrait>.

fn main() {
    let ptr: MySmartPointer<i32> = MySmartPointer(Box::new(4));

    // This coercion would be an error without the derive.
    let ptr: MySmartPointer<dyn MyTrait> = ptr;
}

The other thing is that it makes self: MySmartPointer<Self> object-safe in traits.

trait MyTrait {
    // Arbitrary self types is enough for this.
    fn func(self: MySmartPointer<Self>);
}

// But this requires #[derive(SmartPointer)].
fn call_func(value: MySmartPointer<dyn MyTrait>) {
    value.func();
}

@nikomatsakis
Copy link
Contributor

After discussion on Zulip, Alice and I would like to propose derive(DynCapablePointer).

@nikomatsakis
Copy link
Contributor

I'm going to go ahead and move to merge to move this up the lang priority

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Oct 2, 2024

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 2, 2024
@scottmcm
Copy link
Member

scottmcm commented Oct 2, 2024

Is it just dyn or is it general unsizing? For example, does it allow Foo<[i32; 10]> to Foo<[i32]> as well?

@compiler-errors
Copy link
Member

It's general unsizing, since this macro expands to an implementation of CoerceUnsized.

@nikomatsakis
Copy link
Contributor

@rfcbot fcp cancel

Canceling FCP because we are live discussing alternatives.

@rfcbot
Copy link

rfcbot commented Oct 2, 2024

@nikomatsakis proposal cancelled.

@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 2, 2024
@traviscross
Copy link
Contributor Author

traviscross commented Oct 2, 2024

Update: See here. I'm no longer proposing #[derive(UnsizeInner)].

Maybe the logic below still argues for #[derive(UnsizePointee)]. Let's discuss and see about that or something better.


@rfcbot fcp merge

We talked through this in the planning meeting today, and in the end, the proposal is:1

#[derive(UnsizeInner)]

So I propose that for FCP merge. Read on for why.

(Please see the footnote 1 for some description of the history of proposals.)

We rejected #[derive(DynReceiver)] after confirming that this does not in fact derive Receiver, which would make that name weird.

We then considered #[derive(DynCapablePointer)]. There are some problems with this.

First, and trivially, we'd earlier leaned toward saying Ptr rather than Pointer out of consistency with existing things.

Second, we'd just decided to call traits that are compatible with dyn "dyn compatible" rather than "dyn capable" (in our dicta, not in the language, but still). We considered whether this is in fact different, and decided that it isn't really.

That left us with #[derive(DynCompatiblePtr)]. That's just kind of an unpleasantly long name, so we considered shorter alternatives like #[derive(DynPtr)] and #[derive(DynablePtr)]. We momentary liked DynablePtr.

But there's a meaningful problem with using "dyn" here. The inner type can also be sliced, and that really has nothing to do with it being "dynable". So "dyn" here implies the wrong causality. The true causality is that the things are dynable because they can be unsized, and they can be sliced also because they can be unsized.

On that basis, we considered #[derive(UnsizablePtr)]. Against this was a potential ambiguity about whether this is "un(sizeable) ptr" or "(unsize)able ptr" (that is, "unable to be sizable" or "able to be unsized"). However, we didn't really want to drop back to UnsizeCompatiblePtr. For one thing, there's some precedent that we're setting in this choice. If we do an XCompatibleThing, then it's probably going to make it less likely that we'd ever do XableThing instead, and in most cases, if something is "X compatible", then it's going to be "Xable", so it'd be an unfortunate precedent.

Two observations at this point led to the penultimate solution:

  • We're not really talking about the capabilities of the pointer here, we're talking about the capabilities with respect to the pointee.
  • Traits like Unsize and Copy already imply capabilities -- we don't say Copyable or CopyCompatible.

That landed us on #[derive(UnsizePointee)]. Then one more observation, that...

  • We say "inner" a lot in Rust, e.g. in the standard library. We don't often say "pointee", and it makes sense that we're affecting the "inner type" here.

...led to the ultimate proposal here:

#[derive(UnsizeInner)]

That is, we're asking to get back a type where we can act to unsize the inner type. (In terms of how it sounds, consider, comparably, e.g. derive(CopyInner), if that were a thing).

That's where we landed. How did we do? What does the rest of the team think?

Footnotes

  1. In the meeting, the sentiment was pointed toward something with "unsize" when the meeting ended. Then Josh, Alice, and I landed on UnsizePointee. Then in later discussion (and due to thought evoked by writing this up), Josh and I tweaked that to UnsizeInner. The canceled FCP directly below was for UnsizePointee. 2

@rfcbot
Copy link

rfcbot commented Oct 2, 2024

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 2, 2024
@dingxiangfei2009
Copy link
Contributor

dingxiangfei2009 commented Oct 4, 2024

@nikomatsakis By extension, I suppose that we want to switch the attribute of the macro from #[pointee] to #[referent] as well.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Oct 9, 2024

@joshtriplett writes on Zulip:

I personally prefer Pointee as being unambiguously "what a pointer points to", rather than having one more thing with "ref" in it, and this one not being about references.

While I think "referent" is a more elegant name, I don't feel super strongly about this. Looking at the (limited) votes on this comment, I see that Pointee and Referent are currently tied.

@rust-lang/lang or @rust-lang/libs, others want to weigh in? (I recommend you leave an emoji on this comment.)

@nikomatsakis
Copy link
Contributor

OK, the people have spoken lol. I'm going to flip back to Pointee.

Image

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Oct 9, 2024

@rfcbot resolve pointee-or-referent — oh, wait, I already did

@nikomatsakis
Copy link
Contributor

I checked @joshtriplett's box based on this Zulip comment.

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Oct 9, 2024
@rfcbot
Copy link

rfcbot commented Oct 9, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Oct 9, 2024
@traviscross
Copy link
Contributor Author

traviscross commented Oct 9, 2024

To reiterate, given the back and forth, what we're FCPing here is the decision to go with:

#[derive(CoercePointee)]

@scottmcm
Copy link
Member

scottmcm commented Oct 9, 2024

Sure, CoercePointee works for me.

@rfcbot reviewed

@kupiakos
Copy link
Contributor

kupiakos commented Oct 11, 2024

My only issue with CoercePointee is that this is coercion of a pointee, but this derive only applies to the unsizing kind:

struct Foo<T>(Box<T>);
fn coerce_pointee<'a>(x: Foo<&'static i32>, _: &'a i32) -> Foo<&'a i32> {
    x
}

Personally I prefer UnsizePointee as it describes the specific type of coercion it enables - an unsizing coercion rather than a lifetime coercion.

@tmandry
Copy link
Member

tmandry commented Oct 11, 2024

That's true. I think I'm okay with it because we still allow the other kinds of coercion that make sense for a pointer (your code sample already compiles, in other words). I can't think of another kind of coercion we would want to allow behind a pointer type that isn't either already allowed or covered by this derive.

On a related note, I don't hate the idea of sticking with the already established CoerceUnsized, but I do think both CoercePointee and UnsizePointee are better and clearer, and I still prefer the first (which are are FCP'ing).

@traviscross traviscross removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Oct 16, 2024
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Oct 19, 2024
@rfcbot
Copy link

rfcbot commented Oct 19, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@Darksonn
Copy link
Contributor

What name did we end up with?

@lqd
Copy link
Member

lqd commented Oct 22, 2024

CoercePointee, cf #129104 (comment) — though maybe whether the std::marker:: prefix is indeed required (from #129104 (comment)) could use one last clarification :3

@tmandry
Copy link
Member

tmandry commented Oct 23, 2024

I think it would still be required, I don't recall any discussion about the path and my understanding was that this would not be in the standard prelude to begin with.

@Darksonn
Copy link
Contributor

The intent is that this will not be in the prelude.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 27, 2024
…to-coerce-referent, r=compiler-errors

Rename macro `SmartPointer` to `CoercePointee`

As per resolution rust-lang#129104 we will rename the macro to better reflect the technical specification of the feature and clarify the communication.

- `SmartPointer` is renamed to `CoerceReferent`
- `#[pointee]` attribute is renamed to `#[referent]`
- `#![feature(derive_smart_pointer)]` gate is renamed to `#![feature(derive_coerce_referent)]`.
- Any mention of `SmartPointer` in the file names are renamed accordingly.

r? `@compiler-errors`

cc `@nikomatsakis` `@Darksonn`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 27, 2024
…-coerce-referent, r=compiler-errors

Rename macro `SmartPointer` to `CoercePointee`

As per resolution rust-lang#129104 we will rename the macro to better reflect the technical specification of the feature and clarify the communication.

- `SmartPointer` is renamed to `CoerceReferent`
- `#[pointee]` attribute is renamed to `#[referent]`
- `#![feature(derive_smart_pointer)]` gate is renamed to `#![feature(derive_coerce_referent)]`.
- Any mention of `SmartPointer` in the file names are renamed accordingly.

r? `@compiler-errors`

cc `@nikomatsakis` `@Darksonn`
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Oct 31, 2024
@Darksonn
Copy link
Contributor

Should this be closed now that #131284 is merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-derive_smart_pointer `#![feature(derive_smart_pointer)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests