Haskell/SQL comment parsing failure

[masaeedu]

Hello there. I have delta configured as my git pager, but occasionally I see error messages indicating the executable panics and crashes:

➜  automonad.hs git:(master) git show
thread 'main' panicked at 'byte index 2 is out of bounds of ``', src/libcore/str/mod.rs:2051:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

I don't really know how to go about debugging this, although I did run it with RUST_BACKTRACE=full and collected the following output:

➜  automonad.hs git:(master) git show
thread 'main' panicked at 'byte index 2 is out of bounds of ``', src/libcore/str/mod.rs:2051:9
stack backtrace:
   0:     0x55d8a60c1784 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::he6f982049149276c
   1:     0x55d8a60e55ad - core::fmt::write::h0a4f9eb1997ae8a7
   2:     0x55d8a60bfe35 - std::io::Write::write_fmt::h0ef9dc02b782df17
   3:     0x55d8a60c2cde - std::panicking::default_hook::{{closure}}::h20d0965c2f49660f
   4:     0x55d8a60c29f0 - std::panicking::default_hook::h35815be434d41dd3
   5:     0x55d8a60c337b - std::panicking::rust_panic_with_hook::hb153f0c7d5f10b68
   6:     0x55d8a60c2f1e - std::panicking::continue_panic_fmt::hc6930181fc8baf87
   7:     0x55d8a60c2e06 - rust_begin_unwind
   8:     0x55d8a60e51ae - core::panicking::panic_fmt::h86f58df9a8c5a71a
   9:     0x55d8a60edbb7 - core::str::slice_error_fail::h7d12d290e1b1892c
  10:     0x55d8a5f95752 - core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::RangeFrom<usize>>::index::{{closure}}::he8834dc4d8b68910
  11:     0x55d8a5f999db - delta::parse::get_file_path_from_file_meta_line::h50d8fc928cbb6dd3
  12:     0x55d8a5f7b234 - delta::delta::delta::h8aaa8e731fed5c22
  13:     0x55d8a5f88638 - delta::main::hfc852ff77c949988
  14:     0x55d8a5f894f1 - std::rt::lang_start::{{closure}}::h2a13986616b17724
  15:     0x55d8a60c2da3 - std::panicking::try::do_call::hf407ca0e1a546429
  16:     0x55d8a60d21da - __rust_maybe_catch_panic
  17:     0x55d8a60c8b79 - std::rt::lang_start_internal::h71b7dac5eb32cc57
  18:     0x55d8a5f894d2 - main
  19:     0x7f07b52a0b8e - __libc_start_main
  20:     0x55d8a5f7122a - _start
  21:                0x0 - <unknown>

The code does make heavy use of unicode symbols, I don't know if that might have anything to do with it?

I should mention that I only see this message after I exit the pager. There doesn't seem to be any noticeable issue when it comes to how the program is executing (maybe I'm not looking carefully enough), but I thought I'd let you know.

[masaeedu]

Ah, actually I did notice a problem just now. The diff is truncated.

[masaeedu]

In case you want to try to reproduce this, please clone https://github.com/masaeedu/automonad.hs, check out commit number d481eaa8a249c6daecb05a97e8af1b926b0c02be and try running git show after configuring core.pager in your .gitconfig to delta --dark.

[dandavison]

Thanks! Right, so to narrow it down slightly more I can reproduce with

git show d481eaa -- src/Category/Coproduct.hs

Looks like delta has a bug affecting languages like Haskell and SQL that use -- for comments. I.e. this line in the parser is being triggered when a Haskell comment (--) has been removed (-):

delta/src/delta.rs

Line 89 in 88e967c

} else if (line.starts_with("--- ") || line.starts_with("rename from "))

Hm, how to fix this. That state machine parser is a total mess unfortunately -- the worst part of the codebase.

[dandavison]

What it's trying to do of course is parse stuff like this:

diff --git a/src/CT/Entailment.hs b/src/CT/Entailment.hs
new file mode 100644
index 0000000..3c541e5
--- /dev/null
+++ b/src/CT/Entailment.hs
@@ -0,0 +1,15 @@
+module CT.Entailment where

[masaeedu]

@dandavison Ah, I see. That is quite a conundrum. Not that making a huge change like this is an easy proposition, but does Rust have any parser combinator libraries?

[masaeedu]

Alternatively, is it possible for me to provide the diff to delta in a more structured representation so that it doesn't have to parse things itself?

[dandavison]

I've pushed a quick fix to master. If you are able to build delta (cargo build) from the git repo it would be great to hear whether everything appears to be working for you. The Windows Travis build is broken which is making me loathe to make a release currently.

does Rust have any parser combinator libraries?

Yes, it does. When I started writing delta I was too new to rust to try to introduce major abstractions (I'd never have got it to compile :) ). But I think that a proper parser abstraction would be a really helpful improvement to delta.

is it possible for me to provide the diff to delta in a more structured representation so that it doesn't have to parse things itself?

That's an interesting idea. Not currently. I guess the thing is that unified/git diff is the parseable standard for representing diffs and the issue is that delta should parse it without problems!

[dandavison]

I'm making a release: https://github.com/dandavison/delta/releases/tag/0.0.16

Homebrew bump pending: Homebrew/homebrew-core#49877

[masaeedu]

@dandavison Thanks for the quick turnaround! I'm sorry, I actually don't have any Rust stuff set up locally, I just obtained the binary package through nixpkgs, but I'll try to bump the version in there and pull later on.

[dandavison]

brew upgrade delta will give 0.0.16 which should be free of this bug.