Clarify object safety rules for methods striked from the vtable #965

guswynn · 2021-02-18T00:55:14Z

When trying to figure out how async_trait object safety worked and thinking about Stream::next's object safety, I found it very hard to understand

This clarifies that object-safety can still be possible even if a method is striked from the vtable, and that self and where Self: Sized` are the ONLY ways to do that.
This was only really explained clearly in the examples below, so this includes this clarification in the object safety rules in the beginning of the section

Separately, NonDispatchable seems like you are out of luck, but this pattern: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=5529e6424895cccda19d4dd81f389b69, where we impl the trait for &T where T: Trait + ?Sized is actually quite common in the std (albeit with self parameter's so you don't need the (&obj_ref) dance to get method resolution to be happy, see https://rust-lang.zulipchat.com/#narrow/stream/249502-wg-async-foundations.2Fstream-trait-rfc/topic/Object.20safety/near/226749056 for more info).

This seems like a pattern we should explain somewhere, but is the reference the right place for it? I have not mentioned it in this PR, but would be curious what people think

src/items/traits.md

Havvy

I don't think these changes make it clear at all that the methods are not part of the vtable, mainly because it causes convoluted logic. Specifically, it has the structure of:

* All assoc fns must have X (Y implies X) OR
** Not have A AND
** Be M but don't S
** OTHERWISE Zeta

I had to spend five minutes just to figure out what was trying to be said. The whole list should be reorganized to say that the associated function is dispatchable (and the criteria for that) or non-dispatchable (and again, which criteria are for that). Do you want to do that?

Unrelated to this change in particular, but it should also probably say Sized is not a supertrait instead of saying Self: Sized.

Havvy · 2021-02-20T07:44:50Z

src/items/traits.md

@@ -325,6 +326,7 @@ fn main() {
 [RFC 255]: https://github.com/rust-lang/rfcs/blob/master/text/0255-object-safety.md
 [associated items]: associated-items.md
 [method]: associated-items.md#methods
+[supertraits]: traits.md#supertraits


Suggested change

[supertraits]: traits.md#supertraits

[supertraits]: #supertraits

guswynn · 2021-02-20T15:31:44Z

@Havvy I agree it's somewhat convoluted. Unfortunately, the reality is that object safety is somewhat convoluted. I do like your suggestion, though it would be a sign it departure from the original rfc structure, which I am not sure if it's a constraint people want to keep or not

Havvy · 2021-02-20T19:56:54Z

Keeping the original RFC structure for documentation is definitely not a constraint. If it can be improved, it should.

When trying to figure out how `async_trait` object safety worked and thinking about `Stream::next`'s object safety, I found it very hard to understand This clarifies that object-safety can still be possible even if a method is striked from the vtable, and that `self` and `where `Self: Sized` are the ONLY ways to do that. This was only really explained clearly in the examples below, so this includes this clarification in the object safety rules in the beginning of the section --- Separately, NonDispatchable seems like you are out of luck, but this pattern: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=5529e6424895cccda19d4dd81f389b69, where we impl the trait for &T where T: Trait + ?Sized is actually quite common in the std (albeit with `self` parameter's so you don't need the `(&obj_ref)` dance to get method resolution to be happy, see https://rust-lang.zulipchat.com/#narrow/stream/249502-wg-async-foundations.2Fstream-trait-rfc/topic/Object.20safety/near/226749056 for more info). This seems like a pattern we should explain somewhere, but is the reference the right place for it? I have not mentioned it in this PR, but would be curious what people think Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>

guswynn · 2021-02-22T16:48:21Z

@Havvy let me know what you think of this change!

Havvy

I'll approve it with my suggested edits.

The things in parenthesis should be in the Note: format, but I can do that later; especially since mixing it with a list is going to be interesting.

Havvy · 2021-02-24T07:46:09Z

src/items/traits.md

-* [`Rc<Self>`]
-* [`Arc<Self>`]
-* [`Pin<P>`] where `P` is one of the types above
+* All associated functions must "dispatchable from a trait object" or "explicitly non-dispatchable from a trait object":


Suggested change

* All associated functions must "dispatchable from a trait object" or "explicitly non-dispatchable from a trait object":

* All associated functions must either be dispatchable from a trait object or be explicitly non-dispatchable:

Havvy · 2021-02-24T07:47:22Z

src/items/traits.md

-      and
-    * Be a [method] that does not use `Self` except in the type of the receiver.
+* All [supertraits] must also be object safe.
+* It must not require `Self: Sized` (i.e. `Sized` must not be a [supertrait][supertraits])


Suggested change

* It must not require `Self: Sized` (i.e. `Sized` must not be a [supertrait][supertraits])

* `Sized` must not be a [supertrait][supertraits]. In other words, it must not require `Self: Sized`.

Let's avoid the latinisms like "i.e."

Havvy · 2021-03-31T08:28:29Z

@guswynn Any status update on this?

guswynn · 2021-03-31T18:32:25Z

@Havvy sorry about the delay!

Havvy · 2021-04-01T02:41:45Z

💟 Thanks!

And no worries about the delay. I know I've spent more than enough time delaying on things I've been needing to do myself.

Update books ## nomicon 1 commits in 6fe476943afd53a9a6e91f38a6ea7bb48811d8ff..8551afbb2ca6f5ea37fe58380318b209785e4e02 2021-03-10 07:28:57 +0900 to 2021-04-01 21:58:50 +0900 - Add example of thinking about Send/Sync's soundness (rust-lang/nomicon#259) ## reference 10 commits in fd97729e2d82f8b08d68a31c9bfdf0c37a7fd542..e1abb17cd94cd5a8a374b48e1bc8134a2208ed48 2021-03-28 14:29:19 -0700 to 2021-04-07 08:09:48 -0700 - Update introduction.md (rust-lang/reference#1004) - clarify UB for raw ptr deref (rust-lang/reference#1000) - Update lint level documentation. (rust-lang/reference#998) - Add rustdoc to tool lints. (rust-lang/reference#997) - Link to ptr::addr_of on raw pointer docs (rust-lang/reference#993) - apply rust-lang/reference#950 to STYLE.md (rust-lang/reference#980) - Tuple Passover rust-lang/reference#2 (rust-lang/reference#990) - Fix typo in macros-by-example.md (rust-lang/reference#996) - Clarify object safety rules for methods striked from the vtable (rust-lang/reference#965) - Add const generic args to const contexts. (rust-lang/reference#995) ## rust-by-example 1 commits in 29d91f591c90dd18fdca6d23f1a9caf9c139d0d7..c80f0b09fc15b9251825343be910c08531938ab2 2021-03-23 09:03:39 -0300 to 2021-04-08 10:28:17 -0300 - fix compile bug with panic! (rust-lang/rust-by-example#1433) ## rustc-dev-guide 11 commits in 0687daa..a9bd2bb 2021-03-28 13:33:56 -0400 to 2021-04-09 18:12:21 -0400 - Improve formatting and update info in "method lookup" section - Change wording a bit: `module` => `crate` - fix typo (rust-lang/rustc-dev-guide#1107) - fix typo - Mention CI build of LLVM in build instruction - Fix rustdocs test command typo (rust-lang/rustc-dev-guide#1103) - Update the "LLVM updates" section - Fix a link about Rustdoc internals - Add quickstart for adding a new optimization (rust-lang/rustc-dev-guide#1094) - Add back example of {{cwd}} (rust-lang/rustc-dev-guide#1099) - Document test input normalization ## embedded-book 1 commits in d3f2ace94d51610cf3e3c265705bb8416d37f8e4..569c3391f5c0cc43433bc77831d17f8ff4d76602 2021-03-17 07:53:09 +0000 to 2021-04-07 08:32:11 +0000 - Make it easier to copy and paste example commands. (rust-embedded/book#289)

bjorn3 reviewed Feb 18, 2021

View reviewed changes

src/items/traits.md Outdated Show resolved Hide resolved

Havvy reviewed Feb 20, 2021

View reviewed changes

guswynn force-pushed the patch-1 branch from 687d09c to 397f118 Compare February 22, 2021 16:46

guswynn force-pushed the patch-1 branch from 397f118 to bab2c58 Compare February 22, 2021 16:47

Havvy requested changes Feb 24, 2021

View reviewed changes

comments

1151724

Havvy approved these changes Apr 1, 2021

View reviewed changes

Havvy merged commit 26ad13f into rust-lang:master Apr 1, 2021

ehuss mentioned this pull request Apr 14, 2021

Update books rust-lang/rust#84191

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify object safety rules for methods striked from the vtable #965

Clarify object safety rules for methods striked from the vtable #965

guswynn commented Feb 18, 2021

Havvy left a comment

Havvy Feb 20, 2021

guswynn commented Feb 20, 2021

Havvy commented Feb 20, 2021

guswynn commented Feb 22, 2021

Havvy left a comment

Havvy Feb 24, 2021

Havvy Feb 24, 2021

Havvy commented Mar 31, 2021

guswynn commented Mar 31, 2021

Havvy commented Apr 1, 2021

	[supertraits]: traits.md#supertraits
	[supertraits]: #supertraits

	* All associated functions must "dispatchable from a trait object" or "explicitly non-dispatchable from a trait object":
	* All associated functions must either be dispatchable from a trait object or be explicitly non-dispatchable:

	* It must not require `Self: Sized` (i.e. `Sized` must not be a [supertrait][supertraits])
	* `Sized` must not be a [supertrait][supertraits]. In other words, it must not require `Self: Sized`.

Clarify object safety rules for methods striked from the vtable #965

Clarify object safety rules for methods striked from the vtable #965

Conversation

guswynn commented Feb 18, 2021

Havvy left a comment

Choose a reason for hiding this comment

Havvy Feb 20, 2021

Choose a reason for hiding this comment

guswynn commented Feb 20, 2021

Havvy commented Feb 20, 2021

guswynn commented Feb 22, 2021

Havvy left a comment

Choose a reason for hiding this comment

Havvy Feb 24, 2021

Choose a reason for hiding this comment

Havvy Feb 24, 2021

Choose a reason for hiding this comment

Havvy commented Mar 31, 2021

guswynn commented Mar 31, 2021

Havvy commented Apr 1, 2021