Add f16 and f128 to rustdoc's PrimitiveType by tgross35 · Pull Request #123581 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation14 Commits1 Checks11 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Fix a few places where these primitives were missing from librustdoc. This should fix the CI failures from doc links in #122470.
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
Status: Awaiting review from the assignee but also interested parties.
Relevant to the rustdoc team, which will review and decide on the PR/issue.
labels
Some changes occurred in src/librustdoc/clean/types.rs
cc @camelid
This was referenced
Apr 7, 2024
rustbot has assigned @fmease.
How fitting :P
Jokes aside, does this fix the issues you were seeing?
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.
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.
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?
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.
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.
Fix a few places where these primitives were missing from librustdoc.
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.
Is there a good place to throw an error if
rustc_doc_primitivehas 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:
| 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.
📌 Commit ebc86e6 has been approved by fmease
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 4 pull requests
Successful merges:
- rust-lang#123410 (Relax framework linking test)
- rust-lang#123446 (Fix incorrect 'llvm_target' value used on watchOS target)
- rust-lang#123579 (add some more tests)
- rust-lang#123581 (Add
f16andf128to rustdoc'sPrimitiveType)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#123581 - tgross35:f16-f128-rustdoc-updates, r=fmease
Add f16 and f128 to rustdoc's PrimitiveType
Fix a few places where these primitives were missing from librustdoc. This should fix the CI failures from doc links in rust-lang#122470.
Labels
`#![feature(f16)]`, `#![feature(f128)]`
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the rustdoc team, which will review and decide on the PR/issue.