Skip to content

Fix SourceMap::start_point #81860

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 18, 2021
Merged

Fix SourceMap::start_point #81860

merged 1 commit into from
Feb 18, 2021

Conversation

osa1
Copy link
Contributor

@osa1 osa1 commented Feb 7, 2021

start_point needs to return the first character's span, but it would
previously call find_width_of_character_at_span which returns the span
of the last character. The implementation is now fixed.

Other changes:

  • Docs for start_point, end_point, find_width_of_character_at_span
    updated

  • Minor simplification in find_width_of_character_at_span code

Fixes #81800

@osa1
Fix SourceMap::start_point
7e94641 
`start_point` needs to return the *first* character's span, but it would
previously call `find_width_of_character_at_span` which returns the span
of the *last* character. The implementation is now fixed.

Other changes:

- Docs for start_point, end_point, find_width_of_character_at_span
  updated

- Minor simplification in find_width_of_character_at_span code

Fixes #81800
@rust-highfive
Copy link
Contributor

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 7, 2021
@osa1
Copy link
Contributor Author

osa1 commented Feb 7, 2021

I forgot to mention in the commit message: the reason why this doesn't fail more often is most of the time first and last characters in the span of start_point's argument have the same size. So it's fine if we use the last char's size for the first char's size. The problem only happens when they have different sizes, as in the regression test I added.

@osa1
Copy link
Contributor Author

osa1 commented Feb 15, 2021

Ping @estebank

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 15, 2021

📌 Commit 7e94641 has been approved by estebank

@bors 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 Feb 15, 2021
@osa1
Copy link
Contributor Author

osa1 commented Feb 16, 2021

Thanks!

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 16, 2021
@Dylan-DPC
Rollup merge of rust-lang#81860 - osa1:issue81800, r=estebank
cc408a4 
Fix SourceMap::start_point

`start_point` needs to return the *first* character's span, but it would
previously call `find_width_of_character_at_span` which returns the span
of the *last* character. The implementation is now fixed.

Other changes:

- Docs for start_point, end_point, find_width_of_character_at_span
  updated

- Minor simplification in find_width_of_character_at_span code

Fixes rust-lang#81800
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Feb 16, 2021
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 16, 2021
@Dylan-DPC
Rollup merge of rust-lang#81860 - osa1:issue81800, r=estebank
a03cda1 
Fix SourceMap::start_point

`start_point` needs to return the *first* character's span, but it would
previously call `find_width_of_character_at_span` which returns the span
of the *last* character. The implementation is now fixed.

Other changes:

- Docs for start_point, end_point, find_width_of_character_at_span
  updated

- Minor simplification in find_width_of_character_at_span code

Fixes rust-lang#81800
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Feb 16, 2021
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 17, 2021
@Dylan-DPC
Rollup merge of rust-lang#81860 - osa1:issue81800, r=estebank
06bb4e7 
Fix SourceMap::start_point

`start_point` needs to return the *first* character's span, but it would
previously call `find_width_of_character_at_span` which returns the span
of the *last* character. The implementation is now fixed.

Other changes:

- Docs for start_point, end_point, find_width_of_character_at_span
  updated

- Minor simplification in find_width_of_character_at_span code

Fixes rust-lang#81800
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Feb 17, 2021
Closed
@bors
Copy link
Collaborator

bors commented Feb 17, 2021

⌛ Testing commit 7e94641 with merge 7b3053763333134c00035285cc6dc8b4acfae5d8...

@bors
Copy link
Collaborator

bors commented Feb 17, 2021

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 17, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@osa1
Copy link
Contributor Author

osa1 commented Feb 17, 2021

@estebank I think this needs an approval again?

@estebank
Copy link
Contributor

@bors retry

@bors 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 Feb 17, 2021
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Feb 17, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2021
@bors
Auto merge of rust-lang#82241 - Dylan-DPC:rollup-munmzg5, r=Dylan-DPC
cbf666d 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#77728 (Expose force_quotes on Windows.)
 - rust-lang#80572 (Add a `Result::into_ok_or_err` method to extract a `T` from `Result<T, T>`)
 - rust-lang#81860 (Fix SourceMap::start_point)
 - rust-lang#81869 (Simplify pattern grammar, improve or-pattern diagnostics)
 - rust-lang#81898 (Fix debug information for function arguments of type &str or slice.)
 - rust-lang#81972 (Placeholder lifetime error cleanup)
 - rust-lang#82007 (Implement reborrow for closure captures)
 - rust-lang#82021 (Spell out nested Self type in lint message)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d223250 into rust-lang:master Feb 18, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 18, 2021
@osa1 osa1 deleted the issue81800 branch February 18, 2021 04:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@estebank estebank

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.52.0
Development

Successfully merging this pull request may close these issues.

Thread 'rustc' panicked at 'assertion failed: bpos.to_u32() >= mbc.pos.to_u32() + mbc.bytes as u32', compiler/rustc_span/src/lib.rs:1512:17
6 participants
@osa1 @rust-highfive @estebank @bors @rust-log-analyzer @rustbot