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

Wrong caller location for overloaded index operator #114388

Closed
ebds-rs opened this issue Aug 2, 2023 · 4 comments · Fixed by #114434
Closed

Wrong caller location for overloaded index operator #114388

ebds-rs opened this issue Aug 2, 2023 · 4 comments · Fixed by #114434
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ebds-rs
Copy link

ebds-rs commented Aug 2, 2023

Code

for i in 0..=255 {
    for j in 0..=255 {
        for k in 0..=255 {
            &std::str::from_utf8(&[i, j, k])
                .unwrap_or("XXX")
                .to_uppercase()[..3];
        }
    }
}

Meta

rustc --version --verbose:

rustc 1.71.0 (8ede3aae2 2023-07-12)
binary: rustc
commit-hash: 8ede3aae28fe6e4d52b38157d7bfe0d3bceef225
commit-date: 2023-07-12
host: x86_64-unknown-linux-gnu
release: 1.71.0
LLVM version: 16.0.5

Error output

failures:

---- tests::it_works stdout ----
thread 'tests::it_works' panicked at 'byte index 3 is out of bounds of `I`', src/lib.rs:15:22
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Backtrace

failures:

---- tests::it_works stdout ----
thread 'tests::it_works' panicked at 'byte index 3 is out of bounds of `I`', src/lib.rs:15:22
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
   2: core::str::slice_error_fail_rt
   3: core::str::slice_error_fail
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/str/mod.rs:87:9
   4: core::str::traits::<impl core::slice::index::SliceIndex<str> for core::ops::range::RangeTo<usize>>::index
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/str/traits.rs:309:21
   5: core::str::traits::<impl core::ops::index::Index<I> for str>::index
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/str/traits.rs:61:15
   6: <alloc::string::String as core::ops::index::Index<core::ops::range::RangeTo<usize>>>::index
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/alloc/src/string.rs:2358:10
   7: ice_oob::tests::it_works
             at ./src/lib.rs:15:22
   8: ice_oob::tests::it_works::{{closure}}
             at ./src/lib.rs:11:19
   9: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
  10: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@ebds-rs ebds-rs added C-bug Category: This is a bug. 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. labels Aug 2, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 2, 2023
@ebds-rs
Copy link
Author

ebds-rs commented Aug 2, 2023

Not sure if this qualifies as ICE, but its is not clear that indexing into a unicode string that contains a multibyte code point is causing this error.

Maybe it is just a doc/error message issue?

@Noratrieb Noratrieb removed I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 3, 2023
@Noratrieb
Copy link
Member

Noratrieb commented Aug 3, 2023

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3e5b5f1a22f9b0074d4c5529e2eb6926

The panic message is pointing at the &std::str::from_utf8(&[i, j, k]) line, which is weird because the panic is happening two lines below.

A simplified example using track_caller produces the correct line number: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=21c9fb246423ca6ffa3256421965a05f

@Noratrieb
Copy link
Member

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01ffb8128a4175fe2c014db8ce4e8786

This looks like an issue with the indexing operator.

@Noratrieb Noratrieb changed the title ICE: byte index N is out of bounds of `` Wrong line number in panicking message when indexing operator panics Aug 3, 2023
@Noratrieb Noratrieb self-assigned this Aug 3, 2023
@Noratrieb Noratrieb added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 3, 2023
@Noratrieb Noratrieb changed the title Wrong line number in panicking message when indexing operator panics Wrong caller location for overloaded index operator Aug 3, 2023
@ebds-rs
Copy link
Author

ebds-rs commented Aug 3, 2023

This looks like an issue with the indexing operator.

Right, its because the str::from_utf8 method reduces the length of the byte sequence. Even if it doesn't, if you index into the middle of a UTF-8 codepoint (multibyte), you get an error.

I solved this in my specific use-case by converting back using str.as_bytes(), then checking length, indexing, and matching against the resulting sequence with b"XXX", etc.

Just a subtle issue with the str matching, since the error message isn't exactly clear, and you have to remember about UTF-8 codepoints sometimes being multibyte.

@bors bors closed this as completed in 99e4127 Aug 4, 2023
flip1995 pushed a commit to flip1995/rust that referenced this issue Aug 11, 2023
Improve spans for indexing expressions

fixes rust-lang#114388

Indexing is similar to method calls in having an arbitrary left-hand-side and then something on the right, which is the main part of the expression. Method calls already have a span for that right part, but indexing does not. This means that long method chains that use indexing have really bad spans, especially when the indexing panics and that span in coverted into a panic location.

This does the same thing as method calls for the AST and HIR, storing an extra span which is then put into the `fn_span` field in THIR.

r? compiler-errors
calebcartwright pushed a commit to calebcartwright/rust that referenced this issue Oct 23, 2023
Improve spans for indexing expressions

fixes rust-lang#114388

Indexing is similar to method calls in having an arbitrary left-hand-side and then something on the right, which is the main part of the expression. Method calls already have a span for that right part, but indexing does not. This means that long method chains that use indexing have really bad spans, especially when the indexing panics and that span in coverted into a panic location.

This does the same thing as method calls for the AST and HIR, storing an extra span which is then put into the `fn_span` field in THIR.

r? compiler-errors
calebcartwright pushed a commit to calebcartwright/rust that referenced this issue Jun 22, 2024
Improve spans for indexing expressions

fixes rust-lang#114388

Indexing is similar to method calls in having an arbitrary left-hand-side and then something on the right, which is the main part of the expression. Method calls already have a span for that right part, but indexing does not. This means that long method chains that use indexing have really bad spans, especially when the indexing panics and that span in coverted into a panic location.

This does the same thing as method calls for the AST and HIR, storing an extra span which is then put into the `fn_span` field in THIR.

r? compiler-errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants