Skip to content

Include trailing commas in wrapped function declarations [RustDoc] #125946

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

Merged
merged 3 commits into from
Jun 4, 2024
Merged

Include trailing commas in wrapped function declarations [RustDoc] #125946

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b7a8f1f
Include trailing commas in functions
Sergi-Ferrez Jun 4, 2024
617e64c
Update code format and tests
Sergi-Ferrez Jun 4, 2024
744dc8c
Use checked_sub
Sergi-Ferrez Jun 4, 2024
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
22 changes: 12 additions & 10 deletions src/librustdoc/html/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1431,20 +1431,16 @@ impl clean::FnDecl {
cx: &Context<'_>,
) -> fmt::Result {
let amp = if f.alternate() { "&" } else { "&amp;" };

write!(f, "(")?;
if let Some(n) = line_wrapping_indent
&& !self.inputs.values.is_empty()
{
write!(f, "\n{}", Indent(n + 4))?;
}

let last_input_index = self.inputs.values.len() - 1;
Copy link
Member

Choose a reason for hiding this comment

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

this crashes in debug builds when self.inputs.values.is_empty() due to negative overflow (it wraps around in release)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, but i can't understand this comment, last_input_index would be -1 if the inputs vector is empty, but since is only used to compare indexes inside the for loop i can't see the problem.
There is something i'm not noticing?

Copy link
Contributor

Choose a reason for hiding this comment

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

last_input_index is usize, so it doesn’t have negative values.

As an alternative to using negatives, maybe it could use checked_sub?

Copy link
Member

@fmease fmease Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

since [it's] only used to compare indexes inside the for loop i can't see the problem

It's not inside a for-loop, therefore it can indeed trigger. Otherwise you'd be right (apart from "-1usize" as mentioned).

for (i, input) in self.inputs.values.iter().enumerate() {
if i > 0 {
match line_wrapping_indent {
None => write!(f, ", ")?,
Some(n) => write!(f, ",\n{}", Indent(n + 4))?,
};
}
if let Some(selfty) = input.to_self() {
match selfty {
clean::SelfValue => {
Expand Down Expand Up @@ -1477,18 +1473,24 @@ impl clean::FnDecl {
write!(f, "{}: ", input.name)?;
input.type_.print(cx).fmt(f)?;
}
match line_wrapping_indent {
None if i == last_input_index => (),
None => write!(f, ", ")?,
Some(_n) if i == last_input_index => write!(f, ",\n")?,
Some(n) => write!(f, ",\n{}", Indent(n + 4))?,
}
}

if self.c_variadic {
match line_wrapping_indent {
None => write!(f, ", ...")?,
Some(n) => write!(f, "\n{}...", Indent(n + 4))?,
None => write!(f, "...")?,
Some(n) => write!(f, "{}...\n", Indent(n + 4))?,
};
}

match line_wrapping_indent {
None => write!(f, ")")?,
Some(n) => write!(f, "\n{})", Indent(n))?,
Some(n) => write!(f, "{})", Indent(n))?,
};

self.print_output(cx).fmt(f)
Expand Down
Loading