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

regression: ICE - byte index is not a char boundary #111485

Closed
Mark-Simulacrum opened this issue May 11, 2023 · 9 comments
Closed

regression: ICE - byte index is not a char boundary #111485

Mark-Simulacrum opened this issue May 11, 2023 · 9 comments
Assignees
Labels
I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented May 11, 2023

https://crater-reports.s3.amazonaws.com/beta-1.70-2/beta-2023-05-08/reg/aiken-1.0.4-alpha/log.txt

[INFO] [stderr] thread 'rustc' panicked at 'byte index 42492 is not a char boundary; it is inside '━' (bytes 42490..42493) of `use super::Type;
[INFO] [stderr] use crate::{
[INFO] [stderr]     ast::{Annotation, BinOp, CallArg, Span, UntypedPattern},
[INFO] [stderr]     expr::{self, UntypedExpr},
[INFO] [stderr]     format::Formatter,
[INFO] [stderr]     levenshtein,
[INFO] [stderr]     pretty::Documentable,
[INFO] [stderr] };
[INFO] [stderr] use indoc::formatdoc;
[INFO] [stderr] use miette::{Diagnostic, LabeledSpan};
[INFO] [stderr] use `[...]', compiler/rustc_span/src/source_map.rs:1025:14
@Mark-Simulacrum Mark-Simulacrum added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels May 11, 2023
@Mark-Simulacrum Mark-Simulacrum added this to the 1.71.0 milestone May 11, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 11, 2023
@compiler-errors
Copy link
Member

compiler-errors commented May 13, 2023

@Nilstrieb this seems like another indoc ICE like #106191...?

Kinda minimal repro (the bytes matter quite a lot, so e.g., renaming longish_name will cause it to go from ICE -> pass kinda arbitrarily.)

use indoc::formatdoc;

fn main() {
    let x = formatdoc! {
        r#"━━━━━━━━
           │ {a} {d}

           │ {longish_name} {c}

           '{e}'
        "#
        , a = "".to_string()
        , longish_name = "".to_string()
        , c = "".to_string()
        , d = "".to_string()
        , e = "".to_string()
    };
}

@compiler-errors
Copy link
Member

Actually, upon bisection, this seems to have regressed in @m-ou-se's #109664.

searched nightlies: from nightly-2023-01-01 to nightly-2023-05-13
regressed nightly: nightly-2023-03-30
searched commit range: 478cbb4...17c1167
regressed commit: cf32b9d

@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.71.0, 1.70.0 May 13, 2023
@m-ou-se m-ou-se self-assigned this May 14, 2023
@m-ou-se
Copy link
Member

m-ou-se commented May 14, 2023

relevant part of the stack trace from a debug+unoptimized build of rustc:

  25: <rustc_span[a35dddb0941cd0b0]::source_map::SourceMap>::find_width_of_character_at_span
          at rust/compiler/rustc_span/src/source_map.rs:1025:14
  26: <rustc_span[a35dddb0941cd0b0]::source_map::SourceMap>::end_point
          at rust/compiler/rustc_span/src/source_map.rs:931:21
  27: <rustc_mir_build[ddb99fb8276dd8b7]::build::Builder>::schedule_drop
          at rust/compiler/rustc_mir_build/src/build/scope.rs:946:33
  28: <rustc_mir_build[ddb99fb8276dd8b7]::build::Builder>::as_temp_inner
          at rust/compiler/rustc_mir_build/src/build/expr/as_temp.rs:103:21
  29: <rustc_mir_build[ddb99fb8276dd8b7]::build::Builder>::as_temp::{closure#0}
          at rust/compiler/rustc_mir_build/src/build/expr/as_temp.rs:23:36
  30: stacker[7ec4398987cef261]::maybe_grow::<rustc_mir_build[ddb99fb8276dd8b7]::build::BlockAnd<rustc_middle[3c153db6115e4e4e]::mir::Local>, <rustc_mir_build[ddb99f
b8276dd8b7]::build::Builder>::as_temp::{closure#0}>
          at .cargo/registry/src/index.crates.io-6f17d22bba15001f/stacker-0.1.15/src/lib.rs:55:9
  31: rustc_data_structures[169f270e33b59d29]::stack::ensure_sufficient_stack::<rustc_mir_build[ddb99fb8276dd8b7]::build::BlockAnd<rustc_middle[3c153db6115e4e4e]::mi
r::Local>, <rustc_mir_build[ddb99fb8276dd8b7]::build::Builder>::as_temp::{closure#0}>
          at rust/compiler/rustc_data_structures/src/stack.rs:17:5
  32: <rustc_mir_build[ddb99fb8276dd8b7]::build::Builder>::as_temp
          at rust/compiler/rustc_mir_build/src/build/expr/as_temp.rs:23:9

@m-ou-se
Copy link
Member

m-ou-se commented May 14, 2023

Running -Zunpretty=hir-tree on:

fn main() {
    indoc::formatdoc! {
        r#"abcabcabcabcabcabcabcab
           abcde
           abcdefghij
           abc {cc}
        "#,
        cc = "".to_string(),
    };
}

gives a tree that contains

span: src/main.rs:5:5: 5:9 (#9)

even though line 5 is the line with abcdefghij, and columns 5 through 9 are part of the whitespace/indentation on that line.

That span is used for the new_display() call for the {cc} placeholder. It should have referred to the {cc} placeholder itself.

Before #109664, it used the span of the cc = argument instead of the {cc} placeholder.

So, the span of {cc} was already wrong, but #109664 made that more visible by using that span for more things.

In cases where that span was already used, such as for diagnostics/errors (e.g. when you replace {c} by {x} in this snippet), it already triggered the same ICE also before #109664.

@m-ou-se
Copy link
Member

m-ou-se commented May 14, 2023

The reason that that mir code is trying to access the source code underneath the span at all is because it tries to create a span of the } closing brace of a block, and the code for getting a span for the last character of a span checks the actual source code to find out what the byte width of the last character is.

@m-ou-se
Copy link
Member

m-ou-se commented May 14, 2023

So, the short term workaround is to make find_width_of_character_at_span a bit more robust. The proper solution is to fix the spans of the placeholders of format_args!() when the span of the format string is a lie.

@m-ou-se
Copy link
Member

m-ou-se commented May 14, 2023

So, the short term workaround is to make find_width_of_character_at_span a bit more robust.

Doing that in #111560

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 15, 2023
…r=cjgillot

Simplify find_width_of_character_at_span.

This makes `find_width_of_character_at_span` simpler and more robust against bad spans.

Fixes (but does not close, per beta policy) rust-lang#111485
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 15, 2023
…r=cjgillot

Simplify find_width_of_character_at_span.

This makes `find_width_of_character_at_span` simpler and more robust against bad spans.

Fixes (but does not close, per beta policy) rust-lang#111485
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 16, 2023
@Mark-Simulacrum
Copy link
Member Author

Closing as fixed, #111560 is merged and backported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants