`…` in comment cause unexpected rustc panic

[dduan]

When I used the character … in comment, combined with some (invalid) code preceding it, the compiler panicked.

I tried this code:

struct X {}
fn f() {
    vec![X]; //…
}

I expected to see this happen: rustc compiles this code.

Instead, this happened: rustc panicked with the following output

thread 'rustc' panicked at 'byte index 38 is not a char boundary; it is inside '…' (bytes 36..39) of `struct X {}
fn f() {
    vec![X]; //…
}
`', src/libcore/str/mod.rs:2027:5
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.35.0 (3c235d560 2019-05-20) running on x86_64-apple-darwin

Meta

rustc --version --verbose:

rustc 1.35.0 (3c235d560 2019-05-20)
binary: rustc
commit-hash: 3c235d5600393dfe6c36eeed34042efad8d4f26e
commit-date: 2019-05-20
host: x86_64-apple-darwin
release: 1.35.0
LLVM version: 8.0

Backtrace:

stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::_print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::continue_panic_fmt
   7: rust_begin_unwind
   8: core::panicking::panic_fmt
   9: core::str::slice_error_fail
  10: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::Range<usize>>::index::{{closure}}
  11: syntax::source_map::SourceMap::span_to_snippet
  12: rustc_resolve::error_reporting::<impl rustc_resolve::Resolver>::smart_resolve_report_errors
  13: rustc_resolve::Resolver::smart_resolve_path_fragment::{{closure}}
  14: rustc_resolve::Resolver::smart_resolve_path_fragment
  15: rustc_resolve::Resolver::smart_resolve_path
  16: rustc_resolve::Resolver::resolve_expr
  17: syntax::visit::walk_expr
  18: rustc_resolve::Resolver::resolve_expr
  19: rustc_resolve::Resolver::resolve_expr
  20: rustc_resolve::Resolver::resolve_expr
  21: <rustc_resolve::Resolver as syntax::visit::Visitor>::visit_block
  22: <rustc_resolve::Resolver as syntax::visit::Visitor>::visit_fn
  23: syntax::visit::walk_item
  24: rustc_resolve::Resolver::resolve_item
  25: rustc_resolve::Resolver::resolve_crate
  26: rustc::util::common::time
  27: rustc_interface::passes::configure_and_expand_inner
  28: rustc_interface::passes::configure_and_expand::{{closure}}
  29: rustc_data_structures::box_region::PinnedGenerator<I,A,R>::new
  30: rustc_interface::passes::configure_and_expand
  31: rustc_interface::queries::Query<T>::compute
  32: rustc_interface::queries::Query<T>::compute
  33: rustc_interface::queries::Query<T>::compute
  34: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::prepare_outputs
  35: rustc_interface::interface::run_compiler_in_existing_thread_pool
  36: std::thread::local::LocalKey<T>::with
  37: scoped_tls::ScopedKey<T>::set
  38: syntax::with_globals

[Centril]

Regressed in 1.34.0. Used to be:

error[E0423]: expected value, found struct `X`
 --> <source>:3:10
  |
3 |     vec![X]; //…
  |          ^
  |          |
  |          did you mean `X { /* fields */ }`?
  |          help: a function with a similar name exists: `f`

error: aborting due to previous error

[Centril]

cc @petrochenkov @estebank

[oli-obk]

Miminized:

struct X {}
fn f() {
    X; //…
}

Probably some span handcrafting?

[petrochenkov]

Probably some span handcrafting?

+1

[ehuss]

I was idly curious, so I took a brief look at this. The diagnostic that triggers this was introduced in #57725, but I don't think it is responsible for the error (it's just stepping through characters looking for a closing brace). There is some strange math in SourceMap::next_point (introduced in #47420). It oscillates between zero-length and 1+ length spans as it is stepping over characters. When it hits a multi-byte character, it returns an invalid span. Example:

//\xe2\x80\xa6

Starting with a span of (0,1), using next_point would result in (1,1), then (1,2), then (2,2), then (2,4). That last one slices into the middle of the multi-byte character (because of width - 1, where width is 3), causing a panic.

I'm not familiar enough with this stuff to say how it should behave. It seems like next_point is being used for two different purposes (getting the next zero-length span, and getting the "next" character), and it's not clear to me what the behavior should be since it is used in a variety of places. I don't think it can be changed to always return zero-length spans, and it can't be changed to always return non-zero length spans. Perhaps there should be two different functions?

[pnkfelix]

triage: P-medium, removing nomination; the ICE to me appears to include enough information for a user to identify the offending character and work-around the problem (namely by getting rid of the character).

I do agree with @ehuss that there should probably be two distinct functions corresponding to the two distinct purposes that they identified for the current callers of SourceMap::next_point.