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

Do not ICE on unicode next point #68084

Merged
merged 5 commits into from
Jan 11, 2020
Merged

Do not ICE on unicode next point #68084

merged 5 commits into from
Jan 11, 2020

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jan 10, 2020

Use shrink_to_hi instead of next_point and fix next_point.

Fix #68000, fix #68091, fix #68092.

Use `shrink_to_hi` instead of `next_point`
Fix rust-lang#68000.
@rust-highfive
Copy link
Collaborator

r? @zackmdavis

(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 Jan 10, 2020
@estebank estebank added beta-accepted Accepted for backporting to the compiler in the beta channel. beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed beta-accepted Accepted for backporting to the compiler in the beta channel. labels Jan 10, 2020
@varkor
Copy link
Member

varkor commented Jan 10, 2020

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 10, 2020

📌 Commit fcd850f has been approved by varkor

@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 Jan 10, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 10, 2020
Do not ICE on unicode next point

Use `shrink_to_hi` instead of `next_point`.
Fix rust-lang#68000.
@estebank
Copy link
Contributor Author

estebank commented Jan 10, 2020

@varkor Added some extra changes that should fix all possible ICEs stemming from this: fixed next_point and reduced its usage wherever possible. If you don't mind re-reviewing it should be pretty fast and each commit should be easy to follow.

@@ -710,7 +710,7 @@ impl SourceMap {
pub fn next_point(&self, sp: Span) -> Span {
let start_of_next_point = sp.hi().0;

let width = self.find_width_of_character_at_span(sp, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only necessary change to stop the ICEs in stable and beta.

Copy link
Member

Choose a reason for hiding this comment

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

This isolated change is stable-accepted

@varkor
Copy link
Member

varkor commented Jan 10, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jan 10, 2020

📌 Commit f6e9fd0 has been approved by varkor

Centril added a commit to Centril/rust that referenced this pull request Jan 11, 2020
Do not ICE on unicode next point

Use `shrink_to_hi` instead of `next_point` and fix `next_point`.

Fix rust-lang#68000, fix rust-lang#68091, fix rust-lang#68092.
Centril added a commit to Centril/rust that referenced this pull request Jan 11, 2020
Do not ICE on unicode next point

Use `shrink_to_hi` instead of `next_point` and fix `next_point`.

Fix rust-lang#68000, fix rust-lang#68091, fix rust-lang#68092.
bors added a commit that referenced this pull request Jan 11, 2020
Rollup of 8 pull requests

Successful merges:

 - #67666 (make use of pointer::is_null)
 - #67806 (Extract `rustc_ast_passes`, move gating, & refactor linting)
 - #68043 (Add some missing timers)
 - #68074 (Add `llvm-skip-rebuild` flag to `x.py`)
 - #68079 (Clarify suggestion for E0013)
 - #68084 (Do not ICE on unicode next point)
 - #68102 (Inline some conversion methods around OsStr)
 - #68106 (Fix issue with using `self` module via indirection)

Failed merges:

r? @ghost
@bors bors merged commit f6e9fd0 into rust-lang:master Jan 11, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jan 15, 2020
Add some regression tests

Closes rust-lang#64848 (fixed by rust-lang#67631)
Closes rust-lang#65918 (ICE is hidden by rust-lang#67000, no longer ICE)
Closes rust-lang#66473 (fixed by rust-lang#68084)
Closes rust-lang#67550 (set mir-opt-level to 3)

r? @Centril
@pnkfelix
Copy link
Member

discussed at T-compiler meeting. Accepted for beta-backport.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jan 16, 2020
@pnkfelix
Copy link
Member

aforementioned isolated change is stable-accepted for backport

@pnkfelix pnkfelix added the stable-accepted Accepted for backporting to the compiler in the stable channel. label Jan 16, 2020
@cuviper cuviper mentioned this pull request Jan 17, 2020
bors added a commit that referenced this pull request Jan 19, 2020
Beta backports

- expect `fn` after `const unsafe` / `const extern` #68073
- Do not ICE on unicode next point #68084
- rustdoc: Don't allow `#![feature(...)]` on stable or beta #67989

r? @Mark-Simulacrum
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 23, 2020
@Mark-Simulacrum Mark-Simulacrum removed stable-accepted Accepted for backporting to the compiler in the stable channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Jan 31, 2020
@estebank estebank deleted the ice-68000 branch November 9, 2023 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
7 participants