rustdoc: Render HRTB correctly for bare functions by camelid · Pull Request #79991 · rust-lang/rust

camelid · 2020-12-13T00:19:51Z

The angle brackets were not rendered, so code like this:

some_func: for<'a> fn(val: &'a i32) -> i32

would be rendered as:

some_func: fn'a(val: &'a i32) -> i32

However, rendering with angle brackets is still invalid syntax:

some_func: fn<'a>(val: &'a i32) -> i32

so now it renders correctly as:

some_func: for<'a> fn(val: &'a i32) -> i32

However, note that this code:

some_trait: dyn for<'a> Trait<'a>

will still render as:

some_trait: dyn Trait<'a>

which is not invalid syntax, but is still unclear. Unfortunately I think
it's hard to fix that case because there isn't enough information in the
rustdoc::clean::Type that this code operates on. Perhaps that case can
be fixed in a later PR.

r? @jyn514

src/test/rustdoc/for-lifetime.rs

src/test/rustdoc/fn-type.rs

jyn514 · 2020-12-13T00:29:52Z

some_func: fn<'a>(val: &'a i32) -> i32

This is invalid syntax:

error: expected `(`, found `<`
 --> src/main.rs:1:30
  |
1 | fn higher_order(some_func: fn<'a>(val: &'a i32) -> &i32) {
  |                              ^ expected `(`

Is it simple to display for<'a> fn instead? If so, we should do that.

camelid · 2020-12-13T00:36:20Z

Yes, I was planning on opening a follow-up PR; I figured we should do it in two stages so the rendered syntax is at least plausible. I can do it in this PR instead if you think that's better though.

jyn514 · 2020-12-13T00:54:14Z

Yes, I think it would be better to fix it here if possible.

camelid · 2020-12-13T03:51:32Z

I fixed it for bare functions, but I think it will be harder to fix for dyn Traits. See the commit for more: cf5a620.

src/librustdoc/html/format.rs

src/librustdoc/html/render/mod.rs

src/librustdoc/html/format.rs

src/test/rustdoc/for-lifetime.rs

jyn514 · 2020-12-13T14:11:40Z

I fixed it for bare functions, but I think it will be harder to fix for dyn Traits. See the commit for more: cf5a620.

Ok, that seems reasonable. The new output for traits is at least vaguely right so this is definitely a step in the right direction 😆

jyn514 · 2020-12-23T06:42:33Z

@camelid would this help with #78482 ?

camelid · 2020-12-23T18:57:44Z

@camelid would this help with #78482 ?

@jyn514 No, I don't think so. This just fixes the rendering on bare functions: the HRT bounds aren't rendered at all on trait bounds or trait objects.

camelid · 2021-01-11T03:06:09Z

This PR is blocked on me figuring out why the @has checks are failing. Maybe @GuillaumeGomez knows? :P

These are the htmldocck errors: #79991 (comment)

GuillaumeGomez · 2021-01-11T10:00:44Z

Can you provide the full diff provided by the tidy tool when running the tests too please? (You might need to install it, to update your nightly rustdoc and to rebase this PR though haha).

camelid · 2021-01-17T19:33:18Z

Rebased.

camelid · 2021-01-19T02:03:58Z

I fixed the test! It was a silly mistake on my part:

There were two issues:

I was using the wrong filename: foo/index.html instead of
foo/struct.Foo.html
I was using the wrong syntax (I had changed it after creating the
test): fn<'a> instead of for<'a> fn

The first one was the primary issue, though, because I couldn't
understand why the text I was matching against was the same as the text
in the @has check but the test was failing.

Glad I was able to figure it out!

camelid · 2021-01-19T02:09:56Z

Should be ready for review! (Let me know before you r+ and I can squash.)

GuillaumeGomez · 2021-01-19T09:52:40Z

Looks good to me. Just waiting for @jyn514 now. ;)

bors · 2021-01-27T13:35:40Z

☔ The latest upstream changes (presumably #80987) made this pull request unmergeable. Please resolve the merge conflicts.

camelid · 2021-01-28T03:36:38Z

I'm going to squash before I rebase since the conflicts are non-trivial (and it
makes sense to squash anyway).

camelid · 2021-01-28T03:54:21Z

Rebased!

@GuillaumeGomez Given that Joshua is taking a break for a bit, do you want to be
the reviewer for this? If not, I can wait until Joshua gets back, but I wanted
to check with you first :)

The angle brackets were not rendered, so code like this: some_func: for<'a> fn(val: &'a i32) -> i32 would be rendered as: some_func: fn'a(val: &'a i32) -> i32 However, rendering with angle brackets is still invalid syntax: some_func: fn<'a>(val: &'a i32) -> i32 so now it renders correctly as: some_func: for<'a> fn(val: &'a i32) -> i32 ----- However, note that this code: some_trait: dyn for<'a> Trait<'a> will still render as: some_trait: dyn Trait<'a> which is not invalid syntax, but is still unclear. Unfortunately I think it's hard to fix that case because there isn't enough information in the `rustdoc::clean::Type` that this code operates on. Perhaps that case can be fixed in a later PR.

src/librustdoc/html/format.rs

GuillaumeGomez · 2021-01-28T09:36:31Z

Looks good to me, thanks!

@bors: r=GuillaumeGomez,jyn514

bors · 2021-01-28T09:36:32Z

📌 Commit cd8dcee has been approved by GuillaumeGomez,jyn514

…llaumeGomez,jyn514 rustdoc: Render HRTB correctly for bare functions The angle brackets were not rendered, so code like this: some_func: for<'a> fn(val: &'a i32) -> i32 would be rendered as: some_func: fn'a(val: &'a i32) -> i32 However, rendering with angle brackets is still invalid syntax: some_func: fn<'a>(val: &'a i32) -> i32 so now it renders correctly as: some_func: for<'a> fn(val: &'a i32) -> i32 ----- However, note that this code: some_trait: dyn for<'a> Trait<'a> will still render as: some_trait: dyn Trait<'a> which is not invalid syntax, but is still unclear. Unfortunately I think it's hard to fix that case because there isn't enough information in the `rustdoc::clean::Type` that this code operates on. Perhaps that case can be fixed in a later PR. r? `@jyn514`

Rollup of 10 pull requests Successful merges: - rust-lang#79570 (rustc: Stabilize `-Zrun-dsymutil` as `-Csplit-debuginfo`) - rust-lang#79819 (Add `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint) - rust-lang#79991 (rustdoc: Render HRTB correctly for bare functions) - rust-lang#80215 (Use -target when linking binaries for Mac Catalyst) - rust-lang#81158 (Point to span of upvar making closure FnMut) - rust-lang#81176 (Improve safety of `LateContext::qpath_res`) - rust-lang#81287 (Split rustdoc JSON types into separately versioned crate) - rust-lang#81306 (Fuse inner iterator in FlattenCompat and improve related tests) - rust-lang#81333 (clean up some const error reporting around promoteds) - rust-lang#81459 (Fix rustdoc text selection for page titles) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

0xPoe · 2021-04-05T04:06:29Z

@camelid would this help with #78482 ?

@jyn514 No, I don't think so. This just fixes the rendering on bare functions: the HRT bounds aren't rendered at all on trait bounds or trait objects.

@camelid Do you have any suggestions for rendering it?

jyn514 · 2021-04-05T09:40:17Z

@hi-rustin there are some instructions in #78482. Please try to keep discussion in one place, this PR is not super relevant to 78482.

camelid added C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 13, 2020

rust-highfive assigned jyn514 Dec 13, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 13, 2020

camelid added the A-trait-system Area: Trait system label Dec 13, 2020

camelid commented Dec 13, 2020

View reviewed changes

src/test/rustdoc/for-lifetime.rs Outdated Show resolved Hide resolved

camelid commented Dec 13, 2020

View reviewed changes

src/test/rustdoc/fn-type.rs Outdated Show resolved Hide resolved

jyn514 changed the title ~~rustdoc: Fix bug when rendering generic bare function~~ rustdoc: Fix bug when rendering function pointers with higher-ranked lifetimes Dec 13, 2020

jyn514 reviewed Dec 13, 2020

View reviewed changes

src/librustdoc/html/format.rs Outdated Show resolved Hide resolved

src/librustdoc/html/render/mod.rs Outdated Show resolved Hide resolved

src/librustdoc/html/format.rs Outdated Show resolved Hide resolved

src/test/rustdoc/for-lifetime.rs Outdated Show resolved Hide resolved

jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2020

camelid force-pushed the rustdoc-for-lifetime branch from cf5a620 to c82b548 Compare December 14, 2020 02:28

This comment has been minimized.

Sign in to view

camelid mentioned this pull request Dec 14, 2020

compiletest: Not all diff installations have --color #80037

Closed

camelid force-pushed the rustdoc-for-lifetime branch from aebf392 to 9c31372 Compare January 17, 2021 19:32

This comment has been minimized.

Sign in to view

camelid 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 Jan 19, 2021

GuillaumeGomez approved these changes Jan 19, 2021

View reviewed changes

camelid force-pushed the rustdoc-for-lifetime branch from b58c71b to 8a389b2 Compare January 28, 2021 03:40

camelid changed the title ~~rustdoc: Fix bug when rendering function pointers with higher-ranked lifetimes~~ rustdoc: Render HRTB correctly for bare functions Jan 28, 2021

camelid force-pushed the rustdoc-for-lifetime branch from 8a389b2 to 65a7ac0 Compare January 28, 2021 03:53

camelid commented Jan 28, 2021

View reviewed changes

src/librustdoc/html/format.rs Outdated Show resolved Hide resolved

camelid force-pushed the rustdoc-for-lifetime branch from 65a7ac0 to cd8dcee Compare January 28, 2021 03:56

GuillaumeGomez approved these changes Jan 28, 2021

View reviewed changes

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 Jan 28, 2021

Dylan-DPC-zz mentioned this pull request Jan 28, 2021

Rollup of 8 pull requests #81475

Closed

camelid assigned GuillaumeGomez Jan 28, 2021

JohnTitor mentioned this pull request Jan 29, 2021

Rollup of 10 pull requests #81493

Merged

bors merged commit 3eac643 into rust-lang:master Jan 29, 2021

rustbot added this to the 1.51.0 milestone Jan 29, 2021

camelid deleted the rustdoc-for-lifetime branch January 29, 2021 04:26

Uh oh!

Conversation

camelid commented Dec 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyn514 commented Dec 13, 2020

Uh oh!

camelid commented Dec 13, 2020

Uh oh!

jyn514 commented Dec 13, 2020

Uh oh!

camelid commented Dec 13, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyn514 commented Dec 13, 2020

Uh oh!

This comment has been minimized.

jyn514 commented Dec 23, 2020

Uh oh!

camelid commented Dec 23, 2020

Uh oh!

camelid commented Jan 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuillaumeGomez commented Jan 11, 2021

Uh oh!

camelid commented Jan 17, 2021

Uh oh!

This comment has been minimized.

camelid commented Jan 19, 2021

Uh oh!

camelid commented Jan 19, 2021

Uh oh!

GuillaumeGomez commented Jan 19, 2021

Uh oh!

bors commented Jan 27, 2021

Uh oh!

camelid commented Jan 28, 2021

Uh oh!

camelid commented Jan 28, 2021

Uh oh!

Uh oh!

GuillaumeGomez commented Jan 28, 2021

Uh oh!

bors commented Jan 28, 2021

Uh oh!

0xPoe commented Apr 5, 2021

Uh oh!

jyn514 commented Apr 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

camelid commented Dec 13, 2020 •

edited

Loading

camelid commented Jan 11, 2021 •

edited

Loading