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.

Sign up for GitHub

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

Jump to bottom

Rustdoc render async function re-export #64599

Merged
merged 5 commits into from
Sep 25, 2019
Merged

Rustdoc render async function re-export #64599

merged 5 commits into from
Sep 25, 2019

Conversation

csmoe
Copy link
Member

@csmoe csmoe commented Sep 19, 2019

Closes #63710
r? @nikomatsakis

@csmoe csmoe force-pushed the doc_async_reexport branch from 042503e to ab3fcc0 Compare September 19, 2019 05:55
@Centril
Copy link
Contributor

Centril commented Sep 19, 2019

Won't this still render with -> impl Trait?

@csmoe
Copy link
Member Author

csmoe commented Sep 19, 2019

@Centril fixing associate_method, build_external_function seems not the one to blame.

@csmoe csmoe force-pushed the doc_async_reexport branch from ab3fcc0 to 73f5bac Compare September 19, 2019 16:59
@nikomatsakis

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@csmoe csmoe force-pushed the doc_async_reexport branch from 73f5bac to 9ffb1ce Compare September 19, 2019 18:39
nikomatsakis
nikomatsakis requested changes Sep 20, 2019
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

This looks great, thanks @csmoe!

One question: can we adjust the query to be fn asyncness() instead of fn is_async_fn()?

src/librustdoc/clean/inline.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
nikomatsakis
nikomatsakis requested changes Sep 20, 2019
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Oh, one other thing:

We don't have any test for this! Can you add a test?

Here is an example of such a test for async-fn (crate-local only, obviously):

https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/async-fn.rs

and this is an example testing the cross-crate case specifically:

https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/cross-crate-links.rs

but maybe @QuietMisdreavus can give some better examples of just the tests we want in this case.

(This is for the bug around async fn across crates, @QuietMisdreavus)

@JohnCSimon JohnCSimon added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 21, 2019
@csmoe csmoe force-pushed the doc_async_reexport branch from bf04111 to 726fe3b Compare September 21, 2019 08:34
@csmoe
Copy link
Member Author

csmoe commented Sep 21, 2019

@nikomatsakis I added the tests into https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/inline_cross/impl_trait.rs as it covers bare function and impl re-exports.

@csmoe csmoe added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 23, 2019
nikomatsakis
nikomatsakis requested changes Sep 23, 2019
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

@csmoe great, thanks! I left a few more (quite small) nits.

r=me once fixed

src/librustc/ty/mod.rs Outdated Show resolved Hide resolved
src/librustc/ty/mod.rs Outdated Show resolved Hide resolved
src/librustc_metadata/decoder.rs Outdated Show resolved Hide resolved
@csmoe csmoe force-pushed the doc_async_reexport branch 2 times, most recently from a1587c5 to 04ffef1 Compare September 23, 2019 17:21
@rust-highfive

This comment has been minimized.

Sign in to view
@csmoe csmoe force-pushed the doc_async_reexport branch from 04ffef1 to a744fd0 Compare September 24, 2019 06:18
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 24, 2019

📌 Commit a744fd0 has been approved by nikomatsakis

@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 24, 2019
@nikomatsakis nikomatsakis added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 24, 2019
@nikomatsakis
Copy link
Contributor

I'm assuming this will not make the 1.39 beta cutoff. If so, I think we should consider backporting it, as it resolves a "quality" issue for async await. Basically, the rustdoc for library facades that make use of async fn looks bad (it doesn't show that they are async fn). I wouldn't ordinarily nominate such a thing for backport, I don't think, but since async fn is a high priority feature, we can expect a lot of eyes on it, so I think it might be worth considering.

@Centril
Copy link
Contributor

Centril commented Sep 24, 2019

Yep, this won't make it to the cutoff as @Mark-Simulacrum is branching master to beta in an hour or so. Backporting this seems rather safe so it is reasonable I'd say.

Centril added a commit to Centril/rust that referenced this pull request Sep 25, 2019
@Centril
Rollup merge of rust-lang#64599 - csmoe:doc_async_reexport, r=nikomat…
34067ee 
…sakis

Rustdoc render async function re-export

Closes rust-lang#63710
r? @nikomatsakis
@Centril Centril mentioned this pull request Sep 25, 2019
Merged
bors added a commit that referenced this pull request Sep 25, 2019
@bors
Auto merge of #64754 - Centril:rollup-iwtb5xd, r=Centril
b7820b2 
Rollup of 7 pull requests

Successful merges:

 - #64324 (rustc: Fix mixing crates with different `share_generics`)
 - #64428 (Error explanation e0524)
 - #64481 (A more explanatory thread local storage panic message)
 - #64599 (Rustdoc render async function re-export)
 - #64743 (Update cargo)
 - #64746 (Remove blanket silencing of "type annotation needed" errors)
 - #64753 (Don't emit explain with json short messages.)

Failed merges:

r? @ghost
@bors bors merged commit a744fd0 into rust-lang:master Sep 25, 2019
@pnkfelix
Copy link
Member

discussed at T-compiler meeting. Approved for beta backport.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Sep 26, 2019
Mark-Simulacrum pushed a commit to Mark-Simulacrum/rust that referenced this pull request Sep 26, 2019
@csmoe @Mark-Simulacrum
Backport rust-lang#64599 to beta: rustdoc render async function re-ex…
2ff6c65 
…port
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Sep 26, 2019
Merged
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 26, 2019
bors added a commit that referenced this pull request Sep 27, 2019
@bors
Auto merge of #64814 - Mark-Simulacrum:beta-next, r=Mark-Simulacrum
5752b63 
[beta] switch to stable bootstrap and rollup backports

Contains the following:

* Account for the Zero sub-pattern case. #64748
* relnotes: make compatibility section more sterile and fix rustc version #64742
* Rustdoc render async function re-export #64599
* Update cargo #64773
* switches us to stable bootstrap (not dev-static) (no PR)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tesuji tesuji tesuji left review comments

@nikomatsakis nikomatsakis nikomatsakis requested changes

Assignees

@nikomatsakis nikomatsakis

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rustdoc renders re-exported async fns incorrectly
9 participants
@csmoe @Centril @nikomatsakis @rust-highfive @bors @pnkfelix @tesuji @JohnCSimon @Mark-Simulacrum