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

Add support for range params to rustdoc #80220

Closed
wants to merge 2 commits into from
Closed

Add support for range params to rustdoc #80220

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3812b9f
Add range support to rustdoc, and ensure it compiles
CraftSpider Dec 20, 2020
04ccdf1
Add display test for range args
CraftSpider Dec 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/rustc_hir_pretty/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ pub fn enum_def_to_string(
to_string(NO_ANN, |s| s.print_enum_def(enum_definition, generics, name, span, visibility))
}

pub fn pat_to_string(pat: &hir::Pat<'_>) -> String {
to_string(NO_ANN, |s| s.print_pat(pat))
}

impl<'a> State<'a> {
pub fn cbox(&mut self, u: usize) {
self.s.cbox(u);
Expand Down
5 changes: 1 addition & 4 deletions src/librustdoc/clean/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,10 +413,7 @@ crate fn name_from_pat(p: &hir::Pat<'_>) -> Symbol {
);
return Symbol::intern("()");
}
PatKind::Range(..) => panic!(
"tried to get argument name from PatKind::Range, \
which is not allowed in function arguments"
),
PatKind::Range(..) => rustc_hir_pretty::pat_to_string(p),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to degrade to _ here than pull HIR pretty-printing, especially given that this case is improbable in practice and only full ranges can be accepted in this position without errors.
This is also true for the PatKind::Lit case above (also warn!ing in that case is not rustdoc's job).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Degrade as in print out _ instead of the actual range, I'm assuming you mean?
I used the pretty-print primarily due to @jyn514's advice that the function should actually, in future work, shift to using it for more of the cases, and rely less on re-implementing logic for it. And I don't think there's any harm to printing the true value, as this case is rare and thus it's not like it should hurt average performance. So if anything, I think the Lit case should also fall back to pretty-print as well.

Copy link
Contributor

@petrochenkov petrochenkov Dec 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The general plan for HIR pretty-printing is to either remove or privatize it, some work in this direction was already done in #70344 so it's rarely used now.

rustdoc's goal on the other hand is to print human-readable function parameter names, not to print HIR as precisely as possible.
Literals or ranges don't contain parameter names, so they don't need to be printed.

I'll leave this to @jyn514 to decide.

Copy link
Member

Choose a reason for hiding this comment

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

The main goal here is to reduce rustc_hir's dependencies and its size such that it can start and finish earlier, thereby working towards #65031.

Rustdoc is built after all rustc crates are built, so it doesn't affect this goal one way or another.

The general plan for HIR pretty-printing is to either remove or privatize it

Can you expand on this? I haven't seen plans to remove it altogether - will it be replaced with something? In this particular case a wildcard seems fine, but I'd like to eventually get rid of this whole function and use param_to_string instead. Would it be better to use that here directly instead of adding more functions to rustc_hir_pretty?

PatKind::Slice(ref begin, ref mid, ref end) => {
let begin = begin.iter().map(|p| name_from_pat(&**p).to_string());
let mid = mid.as_ref().map(|p| format!("..{}", name_from_pat(&**p))).into_iter();
Expand Down
3 changes: 3 additions & 0 deletions src/test/rustdoc-ui/range-pattern.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// check-pass

fn func(0u8..=255: u8) {}
jyn514 marked this conversation as resolved.
Show resolved Hide resolved