Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jul 27, 2023

Conversation

fmease
Copy link
Member

@fmease fmease commented Jul 25, 2023

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

);

let proj = projection.map(|p| {
Copy link
Member Author

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.

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 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.

@fmease fmease force-pushed the rustdoc-fix-x-crate-impl-sized branch from 7061f7a to 28d40f1 Compare July 26, 2023 00:13
Comment on lines +866 to +867
if b.is_sized_bound(cx) {
has_sized = true;
Copy link
Member Author

@fmease fmease Jul 26, 2023

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

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

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

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
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2023
…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
11 checks passed
@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
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
Development

Successfully merging this pull request may close these issues.

5 participants