Arbitrary self types v2: book changes. #4174
Conversation
Preliminary book changes for Arbitrary Self Types v2. The listing number is currently out of order and this will need fixing, I suspect, but keeping things there unchanged for reviewability meanwhile.
There was a problem hiding this comment.
Just want to flag up here that I have seen this, and that the content as written seems broadly good! However, I am marking it as “waiting on review” and will be mulling on how best to land it.
This definitely does belong in the reference, and it probably belongs in some kind of guide-like explanatory material somewhere, which might be The Book… but also might not.
At this point in the book, we have not yet talked about self: SomeType at all, other than a quick reference explaining that &self is a short way of writing self: &Self back in Chapter 5. We currently explicitly introduce self: SomeType very briefly in the new ch. 17 (currently available on nightly, will be stable with 1.85) to support discussing self: Pin<&mut Self>, and another brief discussion in the context of self: Box<Self> in ch. 18.
That is suggestive, though: custom self types are relatively advanced material that the book does not cover at all. (See also #4104, which has a similar challenge.)
Net, I am not yet sure which of the following categories this fits into:
- It may belong in the book, in which case:
- This may not be the right spot for it. It might be better situated in the Advanced Traits or Advanced Types sections of what is now Ch. 20.
- If this is the right spot for it—and given the surrounding discussion of
Deref, it may be—, we will need to expand it a bit, given this is the introduction of the whole concept for the first time. We will also want to tweak the examples if we can to fit more with the flow of the types around it in that case.
- It may also not belong in the book at all, as just too niche/advanced a topic to get into. There are, as Lacking explanation of mutable globals (static mut alternatives) #4104 suggests, a fair number of these, and this is an open problem we need to solve at the project level.
@carols10cents and I will think about it and let you all know what we think, but heads up that it's almost certainly going to be after the holidays, as I am off for the next couple weeks!
|
Hi Chris! Thanks for the early feedback, and yeah, I'm not sure either. I don't consider this PR even remotely ready for review but the sort of directional feedback you're giving is exactly what I do need at this stage, so thanks for taking a look at an early draft CL. I'm also away for most of the next three weeks. See you in the new year! |
|
Hello @chriskrycho and @carols10cents - have you thought about whether this is worth including? I don't mind either way but over in this PR I am proposing this feature for stabilization, so I'd like to get the book stuff squared away if you think it's right to include it. Thanks! |
|
I’ll take a gander again later today and get back to you – don’t want to be a blocker! |
There was a problem hiding this comment.
Okay, having taken a close look at this, I think we can and should land it, but with a number of tweaks. See the comments! Thanks for shepherding this feature across the line—this is going to be a really nice improvement to the language!
As an aside: once we get these tweaks made, I’ll merge it, but I may also follow up by moving it to Chapter 20 (Chapter 19 in what is currently on stable, but 20 in 1.85+) under Advanced Traits. I will need to spend some time thinking about how best to integrate it there, though, and I think it’s fine to land it where it is even if I move it later.
There was a problem hiding this comment.
Here, the overall idea seems good, but I want to make a couple tweaks to make it fit better with the flow of the text.
There was a problem hiding this comment.
Per comments below, let’s for now rename the listing to be listing-15-XX. It will become listing-15-14, but I am going to have to do some work to reorder the rest of them, including checking alllll the existing references in the text.
There was a problem hiding this comment.
Thanks for all these changes - I've committed them all to the branch, renamed the files, and made a couple of tweaks to get the example to build. I don't believe we can or should merge this until arbitrary_self_types is stabilized, which hopefully will fix the remaining CI error. Let me know if I should squash the commits or do anything else meanwhile. Cheers! (and incidentally, I originally learned Rust mostly from The New Rustacean!)
There was a problem hiding this comment.
Excellent—I’ll review again tomorrow, and then we can just mark it as blocked till it is stabilized, and get it merged!
There was a problem hiding this comment.
Oh, and I will probably do some work to get it integrated/integrate-able with the print version so that it ends up in the next print edition!
There was a problem hiding this comment.
(You cannot imagine how much it delights me to hear that note about New Rustacean!)
|
Thanks for the review Chris! I've been away for a few days, but I'll get on with making these changes sometime in the next couple of days. Hopefully that fits in with the schedule you mention above. |
|
@adetaylor yep, no problem! |
Co-authored-by: Chris Krycho <[email protected]>
Co-authored-by: Chris Krycho <[email protected]>
Co-authored-by: Chris Krycho <[email protected]>
Co-authored-by: Chris Krycho <[email protected]>
Co-authored-by: Chris Krycho <[email protected]>
Co-authored-by: Chris Krycho <[email protected]>
Preliminary book changes for Arbitrary Self Types v2.
The listing number is currently out of order and this will need fixing, I suspect, but keeping things there unchanged for reviewability meanwhile.
Tracking issue rust-lang/rust#44874