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.

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

fix the doc-comment-decoration-trimming edge-case rustdoc ICE #47210

Conversation

zackmdavis
Copy link
Member

This horizontal_trim function strips the leading whitespace from
doc-comments that have a left-asterisk-margin:

  /**
   * You know what I mean—
   *
   * comments like this!
   */

The index of the column of asterisks is i, and if trimming is deemed
possible, we slice each line from i+1 to the end of the line. But if, in
particular, i was 0 and there was an empty line (as in the example
given in the reporting issue), we ended up panicking trying to slice an
empty string from 0+1 (== 1).

Let's tighten our check to say that we can't trim when i is even the same
as the length of the line, not just when it's greater. (Any such cases
would panic trying to slice line from line.len()+1.)

Resolves #47197.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@zackmdavis
Copy link
Member Author

r? @QuietMisdreavus

@QuietMisdreavus
Copy link
Member

Can you add a test for this? Something like this:

src/test/rustdoc/issue-47197.rs:

// copyright header

// libsyntax ICE when trimming the blank line in the middle of that comment

// @has issue_47197/fn.x.html
/**
* a

* b
*/
pub fn x() { }

@shepmaster shepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 6, 2018
This `horizontal_trim` function strips the leading whitespace from
doc-comments that have a left-asterisk-margin:

  /**
   * You know what I mean—
   *
   * comments like this!
   */

The index of the column of asterisks is `i`, and if trimming is deemed
possible, we slice each line from `i+1` to the end of the line. But if, in
particular, `i` was 0 _and_ there was an empty line (as in the example
given in the reporting issue), we ended up panicking trying to slice an
empty string from 0+1 (== 1).

Let's tighten our check to say that we can't trim when `i` is even the same
as the length of the line, not just when it's greater. (Any such cases
would panic trying to slice `line` from `line.len()+1`.)

Resolves rust-lang#47197.
@zackmdavis zackmdavis force-pushed the the_3rd_of_2_hardest_problems_in_computer_science branch from b6bde34 to 3cfea33 Compare January 6, 2018 19:18
@zackmdavis
Copy link
Member Author

(updated)

@QuietMisdreavus
Copy link
Member

@bors r+ rollup

Thanks!

@bors
Copy link
Contributor

bors commented Jan 6, 2018

📌 Commit 3cfea33 has been approved by QuietMisdreavus

@kennytm kennytm 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 8, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Jan 8, 2018
…blems_in_computer_science, r=QuietMisdreavus

fix the doc-comment-decoration-trimming edge-case rustdoc ICE

This `horizontal_trim` function strips the leading whitespace from
doc-comments that have a left-asterisk-margin:

```
  /**
   * You know what I mean—
   *
   * comments like this!
   */
```

The index of the column of asterisks is `i`, and if trimming is deemed
possible, we slice each line from `i+1` to the end of the line. But if, in
particular, `i` was 0 _and_ there was an empty line (as in the example
given in the reporting issue), we ended up panicking trying to slice an
empty string from 0+1 (== 1).

Let's tighten our check to say that we can't trim when `i` is even the same
as the length of the line, not just when it's greater. (Any such cases
would panic trying to slice `line` from `line.len()+1`.)

Resolves rust-lang#47197.
bors added a commit that referenced this pull request Jan 9, 2018
Rollup of 10 pull requests

- Successful merges: #47210, #47233, #47246, #47254, #47256, #47258, #47259, #47263, #47270, #47272
- Failed merges: #47248
@bors bors merged commit 3cfea33 into rust-lang:master Jan 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants