Stream: rustdoc

Topic: Issue 79497


view this post on Zulip CraftSpider (Dec 20 2020 at 02:21):

For #79497, I know where the error is, but I'm not sure the best path to fix it. The actual error seems to be because refutable pattern checking isn't done yet, so an argument can have an invalid type. How should rustdoc handle that though? Panic is wrong, but I'm not sure how to change a Clean<Argument> impl into something that can fail

view this post on Zulip Joshua Nelson (Dec 20 2020 at 03:34):

@Rune Tynan I would make sure this is actually related to being refutable

view this post on Zulip Joshua Nelson (Dec 20 2020 at 03:34):

I suspect it would give the same error on 0_u8..=255

view this post on Zulip CraftSpider (Dec 20 2020 at 03:48):

Right, true. Which, if that's valid, then the solution is just to teach rustdoc to handle ranges, and refutability doesn't come into it.

view this post on Zulip CraftSpider (Dec 20 2020 at 03:54):

Honestly, I forgot/didn't know irrefutable ranges were valid function patterns, it's not something that tends to be used in 'normal' code. Gotta remember that function args really are basically just Pattern: Type

view this post on Zulip Joshua Nelson (Dec 20 2020 at 03:57):

well, I'm not sure they are actually valid :laughing:

view this post on Zulip Joshua Nelson (Dec 20 2020 at 03:57):

but at least this should be an error and not an ICE

view this post on Zulip Joshua Nelson (Dec 20 2020 at 03:57):

I would double check by running rustc

view this post on Zulip CraftSpider (Dec 20 2020 at 04:01):

I tried a fn foo(0u8..=255: u8) {}, and it did compile

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:05):

nice, and did rustdoc panic?

view this post on Zulip CraftSpider (Dec 20 2020 at 04:08):

One minute and I'll tell you, though if it doesn't I'll be surprised :P

view this post on Zulip CraftSpider (Dec 20 2020 at 04:09):

Yep, it does panic

view this post on Zulip CraftSpider (Dec 20 2020 at 04:10):

From the same line, same message. A range can be irrefutable, but name_from_pat seems to have assumed it would never see it

view this post on Zulip CraftSpider (Dec 20 2020 at 04:11):

The most technically correct solution I can think of is to actually render out the range. On the other hand, that would require work to render Expr nodes (Unless there's already a util function somewhere, I'll look for one). On the other hand, it's unlikely this is a very desired use-case, so could probably do something like with Lit, where it warns and returns a dummy value (Could make it just like "[range]" or "..=" or "()" with little effort)

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:15):

I think currently it just shows __arg0 if it can't come up with a reasonable name

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:15):

do you have a link to the code that's panicking?

view this post on Zulip CraftSpider (Dec 20 2020 at 04:16):

https://github.com/rust-lang/rust/blob/master/src/librustdoc/clean/utils.rs#L416

view this post on Zulip CraftSpider (Dec 20 2020 at 04:17):

Called from https://github.com/rust-lang/rust/blob/master/src/librustdoc/clean/mod.rs#L956, cleaning Arguments, called by cleaning a function definition

view this post on Zulip Noah Lev (Dec 20 2020 at 04:17):

Joshua Nelson said:

I think currently it just shows __arg0 if it can't come up with a reasonable name

cc #76517

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:19):

ok it looks like this is handled by pretty-printing the pattern

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:19):

with custom code because of course it is :face_palm:

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:20):

so I think I would pretty-print the range and return that as the name

view this post on Zulip CraftSpider (Dec 20 2020 at 04:20):

Alright. Is that a thing already in rustc I presume?

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:20):

:shrug: I'm not the one to ask, sorry

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:21):

maybe someone in #t-compiler/help will know

view this post on Zulip CraftSpider (Dec 20 2020 at 04:24):

Looking myself, just found rustc_hir_pretty, which looks to be the desired thing

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:24):

when you finish fixing this bug, it would be nice to fix the rest of that function to use hir_pretty too

view this post on Zulip Joshua Nelson (Dec 20 2020 at 04:24):

but no need to do both at once


Last updated: Oct 11 2021 at 22:34 UTC