Skip to content

rustdoc: fix cross-crate impl Sized & impl ?Sized#114059

Merged
bors merged 2 commits intorust-lang:masterfrom
fmease:rustdoc-fix-x-crate-impl-sized
Jul 27, 2023
Merged

rustdoc: fix cross-crate impl Sized & impl ?Sized#114059
bors merged 2 commits intorust-lang:masterfrom
fmease:rustdoc-fix-x-crate-impl-sized

Conversation

@fmease
Copy link
Member

@fmease fmease commented Jul 25, 2023 •
edited
Loading

Previously, cross-crate impl-Trait (APIT, RPIT, etc.) that only consists of a single Sized bound (modulo outlives-bounds) and ones that are ?Sized were incorrectly rendered. To give you a taste (before vs. after):

- fn sized(x: impl ) -> impl 
+ fn sized(x: impl Sized) -> impl Sized

- fn sized_outlives<'a>(x: impl 'a) -> impl 'a
+ fn sized_outlives<'a>(x: impl Sized + 'a) -> impl Sized + 'a

- fn maybe_sized(x: &impl ) -> &impl 
+ fn maybe_sized(x: &impl ?Sized) -> &impl ?Sized

- fn debug_maybe_sized(x: &impl Debug) -> &impl ?Sized + Debug
+ fn debug_maybe_sized(x: &(impl Debug + ?Sized)) -> &(impl Debug + ?Sized)

Moreover, we now surround impl-Trait that has multiple bounds with parentheses if they're the pointee of a reference or raw pointer type. This affects both local and cross-crate docs. The current output isn't correct (rustc would emit the error ambiguous + in a type if we fed the rendered code back to it).

Best reviewed commit by commit :)

@rustbot label A-cross-crate-reexports

@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2023

r? @jsha

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 25, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2023

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

cc @camelid

fmease
fmease commented Jul 25, 2023
View reviewed changes
src/librustdoc/clean/mod.rs Outdated
Copy link
Member Author

@fmease fmease Jul 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a refactor. I found the map followed by and_then, try-operator and if-let quite hairy.

fmease
fmease commented Jul 25, 2023
View reviewed changes
src/librustdoc/clean/mod.rs Outdated
Copy link
Member Author

@fmease fmease Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This further enlarges the preexisting bound cleaning duplication of clean_ty_generics and clean_middle_opaque_bounds. Let me know if this is okay for the time being, I plan on consolidating these implementations when I overhaul bound cleaning next to fix #113015.

@rustbot rustbot added the A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate label Jul 25, 2023
@fmease

This comment was marked as outdated.

Sign in to view
@fmease fmease force-pushed the rustdoc-fix-x-crate-impl-sized branch from 7061f7a to 28d40f1 Compare July 26, 2023 00:13
fmease
fmease commented Jul 26, 2023
View reviewed changes
src/librustdoc/clean/mod.rs
Comment on lines +866 to +867
if b.is_sized_bound(cx) {
has_sized = true;
Copy link
Member Author

@fmease fmease Jul 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this, I don't think it handles ~const Sized (TraitBoundModifier::MaybeConst) correctly.
I pretty sure it results in has_sized == false making us add + ?Sized to it.
Let me know if I should fix it here or in a follow-up (I guess there might be some more misuses of is_sized_bound).

Edit: impl ~const Sized is actually not allowed (at the moment?) since Sized is not a #[const_trait].

Copy link
Member

@GuillaumeGomez GuillaumeGomez Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That answers the question then. ;)

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jul 27, 2023

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 27, 2023

📌 Commit 28d40f1 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors 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 Jul 27, 2023
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 27, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2023
@bors
Auto merge of rust-lang#114130 - GuillaumeGomez:rollup-69ihuaj, r=Gui…
b73e9a4 
…llaumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#114032 (typos)
 - rust-lang#114059 (rustdoc: fix cross-crate `impl Sized` & `impl ?Sized`)
 - rust-lang#114088 (Bump syn dependency)
 - rust-lang#114091 (docs: fmt::Debug*: Fix comments for finish method.)
 - rust-lang#114109 (Docs: Fix URL for `rmatches`)
 - rust-lang#114117 (Restore region uniquification in the new solver 🎉  )
 - rust-lang#114123 (Turns out opaque types can have hidden types registered during mir validation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1fa0c4d into rust-lang:master Jul 27, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 27, 2023
@fmease fmease deleted the rustdoc-fix-x-crate-impl-sized branch July 27, 2023 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@jsha jsha

Labels

A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.73.0

Development

Successfully merging this pull request may close these issues.

5 participants

@fmease @rustbot @GuillaumeGomez @bors @jsha