Add f16 and f128 to rustdoc's PrimitiveType

[tgross35]

Fix a few places where these primitives were missing from librustdoc. This should fix the CI failures from doc links in #122470.

[rustbot]

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

[tgross35]

@rustbot label +F-f16_and_f128

[fmease]

rustbot has assigned @fmease.

How fitting :P

Jokes aside, does this fix the issues you were seeing?

[fmease]

I don't exactly know anymore how far along #![feature(f16|f128)] is but it would be great if you could add some rustdoc tests for them in this PR demonstrating what you fixed. There are plenty of areas to choose from: Basic rendering, intra-doc links, synthetic auto trait impls, etc.

[tgross35]

rustbot has assigned @fmease.

How fitting :P

I thought the same :D

Jokes aside, does this fix the issues you were seeing?

Yes it did, the changes from #122470 show up now.

I don't exactly know anymore how far along #![feature(f16|f128)] is but it would be great if you could add some rustdoc tests for them in this PR demonstrating what you fixed. There are plenty of areas to choose from: Basic rendering, intra-doc links, synthetic auto trait impls, etc.

Are there any tests for the other float types that I could use as a reference? I am not seeing anything but might not be looking in the right place.

[tgross35]

HIt ctrl+enter at the wrong time, whoops.

Anyway #122470 does not pass CI currently (broken links error) but with this patch applied it does. Is that sufficient for a test or did you mean something else?

[notriddle]

https://github.com/rust-lang/rust/blob/master/tests/rustdoc/primitive/primitive.rs

[fmease]

I thought of something like tests/rustdoc/primitive-link.rs. I would suggest to add an intra-doc link test tests/rustdoc/intra-doc/f16-f128.rs:

#![deny(rustdoc::broken_intra_doc_links)]
#![features(f16, f128)]
//! [`f16`]. [`f128`].

// @has f16_f128/index.html
// @has - '//a/@href' '{{channel}}/std/primitive.f16.html'
// @has - '//a/@href' '{{channel}}/std/primitive.f128.html'

If I'm not mistaken, this test currently fails on master. I know that this test is probably not super important but we can always remove it, extend it or do whatever.

[fmease]

Ah, but notriddle's suggestion might be better since the necessary library/ changes are part of the other PR so my test will probably still fail even with your patch applied.

[tgross35]

I added notriddle's test and verified it fails on master, but passes with this change. Would you still want the intra-doc link test? If so, I can add that after #122470 lands.

Is there a good place to throw an error if rustc_doc_primitive has an unexpected type, to make this a bit more noticeable in the future? I am not sure if there is a reasonable way to do this.

[fmease]

Is there a good place to throw an error if rustc_doc_primitive has an unexpected type, to make this a bit more noticeable in the future? I am not sure if there is a reasonable way to do this.

Yes, we should definitely do that to improve the contributor UX. We have a FIXME here:

rust/src/librustdoc/clean/types.rs

Lines 266 to 272 in 5dbaafd

	let as_primitive = |res: Res<!>| {
	let Res::Def(DefKind::Mod, def_id) = res else { return None };
	tcx.get_attrs(def_id, sym::rustc_doc_primitive).find_map(|attr| {
	// FIXME: should warn on unknown primitives?
	Some((def_id, PrimitiveType::from_symbol(attr.value_str()?)?))
	})
	};

I haven't checked if it would make sense to add a check in this specific location but feel free to experiment and/or look for other places mentioning rustc_doc_primitive. If you have the time and motivation to implement that check, that'd be amazing. Feel free to r? fmease the future PR.

[fmease]

@bors r+ rollup

[bors]

📌 Commit ebc86e6 has been approved by fmease

It is now in the queue for this repository.