Skip to content

Do not ICE when synthesizing spans falling inside unicode chars #63508

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 3 commits into from
Aug 14, 2019
Merged

Do not ICE when synthesizing spans falling inside unicode chars #63508

merged 3 commits into from
Aug 14, 2019

Conversation

estebank
Copy link
Contributor

Fix #61226.

@rust-highfive
Copy link
Contributor

r? @eddyb

(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 Aug 13, 2019
@estebank estebank added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 13, 2019
@eddyb
Copy link
Member

eddyb commented Aug 13, 2019

r? @petrochenkov or @matklad

@rust-highfive rust-highfive assigned petrochenkov and unassigned eddyb Aug 13, 2019
@pietroalbini
Copy link
Member

This is beta-nominated for 1.38 right?

matklad
matklad reviewed Aug 13, 2019
View reviewed changes
@estebank
Copy link
Contributor Author

@pietroalbini correct

petrochenkov
petrochenkov reviewed Aug 13, 2019
View reviewed changes
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2019
@ehuss
Copy link
Contributor

ehuss commented Aug 13, 2019

I'm curious, why not fix SourceMap::next_point to not create invalid spans?

@estebank
Copy link
Contributor Author

@ehuss That is something we should also do, but given that next_point an such are only used in best effort suggestions for malformed code, I am ok with this mild limitation in order to get rid of the ICE. Ideally this same suggestion could be accomplished without synthesizing spans and those would always be correct.

@ehuss
Copy link
Contributor

ehuss commented Aug 13, 2019

Ah, makes sense, thanks!

@petrochenkov
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 13, 2019

📌 Commit 84e202e has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 13, 2019
Centril added a commit to Centril/rust that referenced this pull request Aug 14, 2019
@Centril
Rollup merge of rust-lang#63508 - estebank:compromice, r=petrochenkov
43f4990 
Do not ICE when synthesizing spans falling inside unicode chars

Fix rust-lang#61226.
@Centril Centril mentioned this pull request Aug 14, 2019
Merged
bors added a commit that referenced this pull request Aug 14, 2019
@bors
Auto merge of #63544 - Centril:rollup-qhqfcov, r=Centril
98282c0 
Rollup of 17 pull requests

Successful merges:

 - #62760 (Deduplicate error messages in `librsctc_mir`)
 - #62849 (typeck: Prohibit RPIT types that inherit lifetimes)
 - #63383 (`async fn` lifetime elision tests)
 - #63421 (Implement Clone, Display for ascii::EscapeDefault)
 - #63459 (syntax: account for CVarArgs being in the argument list.)
 - #63475 (Bring back suggestion for splitting `<-` into `< -`)
 - #63485 (ci: move mirrors to their standalone bucket)
 - #63486 (Document `From` trait for `BinaryHeap`)
 - #63488 (improve DiagnosticBuilder docs)
 - #63493 (Remove unneeded comment in src/libcore/hash/mod.rs)
 - #63499 (handle elision in async fn correctly)
 - #63501 (use `ParamName` to track in-scope lifetimes instead of Ident)
 - #63508 (Do not ICE when synthesizing spans falling inside unicode chars)
 - #63511 (ci: add a check for clock drift)
 - #63512 (Provide map_ok and map_err method for Poll<Option<Result<T, E>>>)
 - #63529 (RELEASES.md: ? is one of three Kleene operators)
 - #63530 (Fix typo in error message.)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Aug 14, 2019
@bors
Auto merge of #63544 - Centril:rollup-qhqfcov, r=Centril
c43d03a 
Rollup of 17 pull requests

Successful merges:

 - #62760 (Deduplicate error messages in `librsctc_mir`)
 - #62849 (typeck: Prohibit RPIT types that inherit lifetimes)
 - #63383 (`async fn` lifetime elision tests)
 - #63421 (Implement Clone, Display for ascii::EscapeDefault)
 - #63459 (syntax: account for CVarArgs being in the argument list.)
 - #63475 (Bring back suggestion for splitting `<-` into `< -`)
 - #63485 (ci: move mirrors to their standalone bucket)
 - #63486 (Document `From` trait for `BinaryHeap`)
 - #63488 (improve DiagnosticBuilder docs)
 - #63493 (Remove unneeded comment in src/libcore/hash/mod.rs)
 - #63499 (handle elision in async fn correctly)
 - #63501 (use `ParamName` to track in-scope lifetimes instead of Ident)
 - #63508 (Do not ICE when synthesizing spans falling inside unicode chars)
 - #63511 (ci: add a check for clock drift)
 - #63512 (Provide map_ok and map_err method for Poll<Option<Result<T, E>>>)
 - #63529 (RELEASES.md: ? is one of three Kleene operators)
 - #63530 (Fix typo in error message.)

Failed merges:

r? @ghost
@bors
Copy link
Collaborator

bors commented Aug 14, 2019

☔ The latest upstream changes (presumably #63544) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 14, 2019
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 14, 2019
@bors bors merged commit 84e202e into rust-lang:master Aug 14, 2019
@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Aug 15, 2019
@estebank estebank mentioned this pull request Aug 15, 2019
Closed
@pietroalbini pietroalbini mentioned this pull request Aug 21, 2019
Merged
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 21, 2019
bors added a commit that referenced this pull request Aug 21, 2019
@bors
Auto merge of #63771 - pietroalbini:beta-rollup, r=pietroalbini
641586c 
[beta] Rollup backports

Cherry-picked:

*  RELEASES.md: ? is one of three Kleene operators #63529
*  Do not ICE when synthesizing spans falling inside unicode chars #63508
*  ci: move mirrors to their standalone bucket #63485

r? @ghost
@goffrie goffrie mentioned this pull request Oct 3, 2019
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@matklad matklad matklad left review comments

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

… in comment cause unexpected rustc panic
9 participants
@estebank @rust-highfive @eddyb @pietroalbini @ehuss @petrochenkov @bors @matklad @nikomatsakis