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: cross-crate re-exports: correctly render late-bound params in source order even if early-bound params are present #121022

Merged

Conversation

fmease
Copy link
Member

@fmease fmease commented Feb 13, 2024

r? ghost

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend 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 Feb 13, 2024
@fmease fmease added A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate and removed A-rustdoc-json Area: Rustdoc JSON backend labels Feb 13, 2024
@fmease
Copy link
Member Author

fmease commented Feb 13, 2024

Let's see how bad perf is for the unoptimized version. However, the benchmark suite might not properly cover this scenario.

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 13, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
…-src-order, r=<try>

rustdoc: cross-crate re-exports: correctly render late-bound params in source order even if early-bound params are present

r? ghost
@bors
Copy link
Contributor

bors commented Feb 13, 2024

⌛ Trying commit c19c531 with merge 7d7a418...

src/librustdoc/html/format.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Feb 13, 2024

☀️ Try build successful - checks-actions
Build commit: 7d7a418 (7d7a4189aa83796a6cd2ddda11287bd2f24bd0cf)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7d7a418): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.6% [2.6%, 2.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 662.695s -> 662.981s (0.04%)
Artifact size: 308.49 MiB -> 308.50 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 13, 2024
@fmease
Copy link
Member Author

fmease commented Feb 14, 2024

#121022 (comment)

Spurious, not a doc regression.
@bors rollup=maybe

@fmease fmease force-pushed the rustdoc-x-crate-late-bound-lt-src-order branch from c19c531 to d4a5de3 Compare February 14, 2024 18:22
@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Feb 14, 2024
@fmease fmease removed the A-rustdoc-json Area: Rustdoc JSON backend label Feb 14, 2024
// edition:2021

// @has usr/fn.f.html
// @has - '//pre[@class="rust item-decl"]' "fn f<'a, 'b, 'c, 'd, T, const N: usize>(_: impl Copy)"
Copy link
Member Author

@fmease fmease Feb 14, 2024

Choose a reason for hiding this comment

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

Both items used to be rendered like

pub fn f<'b, 'd, T, const N: usize, 'a, 'c>(_: impl Copy)

@fmease fmease marked this pull request as ready for review February 14, 2024 18:24
@rustbot
Copy link
Collaborator

rustbot commented Feb 14, 2024

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

cc @camelid

@fmease
Copy link
Member Author

fmease commented Feb 14, 2024

r? rustdoc

@fmease
Copy link
Member Author

fmease commented Feb 14, 2024

Eh, since I changed some things around, let's check perf again.
@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 14, 2024
@bors
Copy link
Contributor

bors commented Feb 14, 2024

⌛ Trying commit d4a5de3 with merge 13dfdc4...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2024
…-src-order, r=<try>

rustdoc: cross-crate re-exports: correctly render late-bound params in source order even if early-bound params are present

r? ghost
@bors
Copy link
Contributor

bors commented Feb 14, 2024

☀️ Try build successful - checks-actions
Build commit: 13dfdc4 (13dfdc414a5daeb8fc0e3da1010d96dc82a7bd08)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (13dfdc4): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 635.176s -> 636.701s (0.24%)
Artifact size: 306.13 MiB -> 306.07 MiB (-0.02%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 14, 2024
@@ -1339,13 +1339,14 @@ impl GenericParamDefKind {

#[derive(Clone, PartialEq, Eq, Debug, Hash)]
pub(crate) struct GenericParamDef {
pub(crate) did: DefId,
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicking: please use def_id. We tend to use this over did in rustdoc as much as we can and I'd like this trend to continue. :)

pub(crate) name: Symbol,
pub(crate) kind: GenericParamDefKind,
}

impl GenericParamDef {
pub(crate) fn lifetime(name: Symbol) -> Self {
Self { name, kind: GenericParamDefKind::Lifetime { outlives: ThinVec::new() } }
pub(crate) fn lifetime(did: DefId, name: Symbol) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

Same.

@GuillaumeGomez
Copy link
Member

Looks good to me! Just a nit and it'll be ready to go. :)

…n source order even if early-bound params are present
@fmease fmease force-pushed the rustdoc-x-crate-late-bound-lt-src-order branch from d4a5de3 to a8d869e Compare February 15, 2024 00:40
@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Feb 15, 2024
@fmease fmease removed the A-rustdoc-json Area: Rustdoc JSON backend label Feb 15, 2024
@fmease
Copy link
Member Author

fmease commented Feb 15, 2024

@bors r=GuillaumeGomez rollup

@bors
Copy link
Contributor

bors commented Feb 15, 2024

📌 Commit a8d869e 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 Feb 15, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#111106 (Add known issue of let binding to format_args doc)
 - rust-lang#118749 (Make contributing to windows bindings easier)
 - rust-lang#120982 (Add APIs for fetching foreign items )
 - rust-lang#121022 (rustdoc: cross-crate re-exports: correctly render late-bound params in source order even if early-bound params are present)
 - rust-lang#121082 (Clarified docs on non-atomic oprations on owned/mut refs to atomics)
 - rust-lang#121084 (Make sure `tcx.create_def` also depends on the forever red node, instead of just `tcx.at(span).create_def`)
 - rust-lang#121098 (Remove unnecessary else block from `thread_local!` expanded code)
 - rust-lang#121105 (Do not report overflow errors on ConstArgHasType goals)
 - rust-lang#121116 (Reinstate some delayed bugs.)
 - rust-lang#121122 (Enforce coroutine-closure layouts are identical)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f9a0675 into rust-lang:master Feb 15, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 15, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2024
Rollup merge of rust-lang#121022 - fmease:rustdoc-x-crate-late-bound-lt-src-order, r=GuillaumeGomez

rustdoc: cross-crate re-exports: correctly render late-bound params in source order even if early-bound params are present

r? ghost
@fmease fmease deleted the rustdoc-x-crate-late-bound-lt-src-order branch February 15, 2024 15:15
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