Skip to content

rustdoc: correctly render the return type of cross-crate async fns #116084

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Sep 25, 2023
Merged

rustdoc: correctly render the return type of cross-crate async fns #116084

merged 1 commit into from
Sep 25, 2023
+72 −44

Conversation

fmease
Copy link
Member

@fmease fmease commented Sep 23, 2023

Fixes #115760.

@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2023

r? @GuillaumeGomez

(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 Sep 23, 2023
@fmease fmease added the A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate label Sep 23, 2023
GuillaumeGomez
GuillaumeGomez reviewed Sep 23, 2023
View reviewed changes
GuillaumeGomez
GuillaumeGomez reviewed Sep 23, 2023
View reviewed changes
@fmease fmease force-pushed the rustdoc-fix-x-crate-async-fn branch from 6c9d795 to 386f401 Compare September 23, 2023 22:04
@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2023

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

cc @camelid

GuillaumeGomez
GuillaumeGomez reviewed Sep 24, 2023
View reviewed changes
@fmease fmease force-pushed the rustdoc-fix-x-crate-async-fn branch from 386f401 to 025a2cd Compare September 25, 2023 13:57
@GuillaumeGomez
Copy link
Member

Looks all good to me now, thanks! r=me once CI pass

@fmease
Copy link
Member Author

fmease commented Sep 25, 2023

@bors r=GuillaumeGomez

@bors
Copy link
Collaborator

bors commented Sep 25, 2023

📌 Commit 025a2cd has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 25, 2023

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Sep 25, 2023
@bors
Copy link
Collaborator

bors commented Sep 25, 2023

⌛ Testing commit 025a2cd with merge 0288f2e...

@bors
Copy link
Collaborator

bors commented Sep 25, 2023

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 0288f2e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 25, 2023
@bors bors merged commit 0288f2e into rust-lang:master Sep 25, 2023
@rustbot rustbot added this to the 1.74.0 milestone Sep 25, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0288f2e): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.4% [0.3%, 0.8%] 9
Regressions ❌
(secondary)		 0.7% [0.3%, 0.9%] 17
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.4% [0.3%, 0.8%] 9

Max RSS (memory usage)

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)		 3.3% [3.3%, 3.3%] 1
Regressions ❌
(secondary)		 2.1% [1.6%, 3.2%] 3
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 3.3% [3.3%, 3.3%] 1

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)		 1.4% [1.4%, 1.4%] 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: 630.479s -> 632.544s (0.33%)
Artifact size: 317.20 MiB -> 317.20 MiB (-0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Sep 26, 2023
@fmease fmease deleted the rustdoc-fix-x-crate-async-fn branch September 26, 2023 05:08
@fmease
Copy link
Member Author

fmease commented Sep 26, 2023

Huh weird I did not expect a perf regression. sugared_async_return_type clones the type, that's what I noticed.

@rylev
Copy link
Member

rylev commented Sep 26, 2023

@fmease @GuillaumeGomez would it be possible to try not to clone? The perf regression here is large enough to make it hard to ignore.

@fmease
Copy link
Member Author

fmease commented Sep 26, 2023

I'm gonna look into the regression

@GuillaumeGomez
Copy link
Member

@fmease: Don't hesitate to ask if you need some help

@fmease fmease mentioned this pull request Sep 27, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 27, 2023
@bors
Auto merge of rust-lang#116195 - fmease:rustdoc-investigate-perf-regr…
935f41f 
…ession, r=<try>

[perf] rustdoc: investigate recent perf regression

Investigate perf regression caused by rust-lang#116084.

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 29, 2023
@bors
Auto merge of rust-lang#116195 - fmease:rustdoc-investigate-perf-regr…
67eb8cd 
…ession, r=<try>

[perf] rustdoc: investigate recent perf regression

Investigate perf regression caused by rust-lang#116084.

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 29, 2023
@bors
Auto merge of rust-lang#116195 - fmease:rustdoc-investigate-perf-regr…
ccf59a5 
…ession, r=<try>

[perf] rustdoc: investigate recent perf regression

Investigate perf regression caused by rust-lang#116084.

r? `@ghost`
@fmease
Copy link
Member Author

fmease commented Sep 30, 2023 •
edited
Loading

#116195 should almost entirely fix the perf regressions introduced in this PR if I read things right.
See #116195 (comment).

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 30, 2023
@bors
Auto merge of rust-lang#116195 - fmease:rustdoc-investigate-perf-regr…
5282e5e 
…ession, r=GuillaumeGomez

rustdoc: speed up processing of cross-crate fns to fix a perf regression

* The first commit doesn't affect perf but get's rid of a `.clone()` and a bunch of lines of code. I can drop it if you'd like me to
* The second commit, *“reduce the amount of `asyncness` query executions”*, addresses the perf regression introduced in rust-lang#116084

r? `@ghost`
@fmease fmease added the perf-regression-triaged The performance regression has been triaged. label Oct 4, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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.74.0
Development

Successfully merging this pull request may close these issues.

cargo doc: shows different return types for re-exported methods
6 participants
@fmease @rustbot @GuillaumeGomez @bors @rust-timer @rylev