Show whether a trait is object-safe on its document page

[EFanZh]

So that it is easy for user to recognize object-safe traits.

[csmoe]

rust/compiler/rustc_middle/src/ty/mod.rs

Line 1896 in 2fb1dee

pub fn is_object_safe(self, key: DefId) -> bool {

for the rustdoc side, just query the doc context with a given trait_def_id.

[vramana]

@rustbot claim

[vramana]

@csmoe Can you tell what are the relevant parts of rustdoc that needs to be changed?

[csmoe]

@vramana sorry for late rely :)

rust/src/librustdoc/clean/inline.rs

Lines 213 to 220 in ac923d9

	let is_auto = cx.tcx.trait_is_auto(did);
	clean::Trait {
	unsafety: cx.tcx.trait_def(did).unsafety,
	generics,
	items: trait_items,
	bounds: supertrait_bounds,
	is_auto,
	}

Add a is_object_safe field to clean::Trait, its value can be queried with cx.tcx.is_object_safe(did) like is_auto.

Then you can append the object-safety info into the render part here

rust/src/librustdoc/html/render/print_item.rs

Line 462 in ac923d9

fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Trait) {

I know little about the render code, @jyn514 could you leave some notes about that?

[jyn514]

No, please do not add more fields to Trait. Query is_object_safe at the place it is used.

I don't know much about render, @GuillaumeGomez may have suggestions.

[GuillaumeGomez]

I'm actually not sure what they want the end result to look like, so I don't know where to tell them to update the code. ;)

[tsoutsman]

I'm actually not sure what they want the end result to look like, so I don't know where to tell them to update the code. ;)

@GuillaumeGomez maybe it could be a cube icon to the right of the new copy item import to clipboard button? On hover, there would be a tooltip saying Item is object-safe or Item is not object-safe. I'm not sure where you get icons for rustdoc, but heroicons have a cube and cube-transparent icon that could be used for object-safe, and object-unsafe traits respectively. The stroke-width could be reduced to fit in with the rest of the icons in rustdoc.

If @vramana doesn't want to work on it I'd be happy to give it a go.

[jsha]

Following up from the PR with some UI discussion.

I'd prefer not to add more icons up at the top of the page; we're already a bit crowded with icons. And there's no icon that has a broadly understood meaning of "object safe".

I like the approach in #89553, to write out "This trait is object-safe." But that PR puts it all the way at the top of the page, which is very high priority - and I would say object safety is more medium priority, so it should be lower on the page. One possibility would be to make an <h2> subheading in the top-doc docblock, but that results in using a lot of space for this. Our h2's are big!

What about this: Under "Implementors", we could have at the very top "This trait is object safe. References to types that implement this trait can be automatically coerced to dyn Foo". My reasoning here is that conceptually, dyn Foo is somewhat like an implementor of the trait.

[jyn514]

My reasoning here is that conceptually, dyn Foo is somewhat like an implementor of the trait.

I don't think this is necessarily true, you can have a dyn Foo that doesn't implement Foo. But putting the text under "implementors" seems as good a place as any 👍

[EFanZh]

@jyn514 Just out of curiosity, how can I have a dyn Foo that doesn’t implement Foo?

[jyn514]

@EFanZh I don't think that's actually true, I misremembered.

[jsha]

We have one 👍🏻 on:

Under "Implementors", we could have at the very top "This trait is object safe. References to types that implement this trait can be automatically coerced to dyn Foo".

Would anyone else like to weigh in? Good enough to proceed with implementation?

[GuillaumeGomez]

Any update on this? :)

[camelid]

I'm not sure if it's already been suggested, but what about only showing a note if the trait is not object-safe?

[jsha]

I like @camelid's suggestion to only show when a trait is not object safe, since that's the more unusual condition. If we do that, it doesn't fit logically under "Implementors" anymore. If we proceed with only marking non-object-safe traits, here are a couple of ideas:

• Add a section after the top-doc, but before "Implementors", called "Object Safety", and put the text there.
• Add information to the documentation of any method that causes the trait to be not object-safe.

[GoldsteinE]

I think it’s still fine to mention under “Implementors” like “This trait is not object safe: dyn Trait doesn’t implement Trait”, but I also like the idea of the new section.

I see this issue doesn’t have an assignee currently, can I claim this?

[GuillaumeGomez]

You can but since we didn't reach a consensus, any PR would hang around until we reach one.