Location Parsing Bug

[rgrinberg]

I have the following stack trace:

/usr/local/home/nroberts/.opam/ocaml-base-compiler/bin/dune diagnostics
Error: exception Invalid_argument("Bytes.create")
Raised by primitive operation at Stdlib__Bytes.make in file "[bytes.ml](http://bytes.ml/)", line
  42, characters 10-18
Called from Stdlib__String.make in file "[string.ml](http://string.ml/)" (inlined), line 41,
  characters 2-12
Called from Stdune__loc.pp_file_excerpt.pp_file_excerpt in file
  "otherlibs/stdune/src/[loc.ml](http://loc.ml/)", line 72, characters 52-73
Called from Stdune__result.bind in file "otherlibs/stdune/src/[result.ml](http://result.ml/)"
  (inlined), line 33, characters 12-15
Called from Stdune__result.O.(>>=) in file "otherlibs/stdune/src/[result.ml](http://result.ml/)"
  (inlined), line 68, characters 20-29
Called from Stdune__loc.pp_file_excerpt in file
  "otherlibs/stdune/src/[loc.ml](http://loc.ml/)", line 117, characters 6-158
Called from Stdune__loc.pp in file "otherlibs/stdune/src/[loc.ml](http://loc.ml/)", line 140,
  characters 5-72
Called from Main__diagnostics.format_diagnostic in file "bin/[diagnostics.ml](http://diagnostics.ml/)",
  line 25, characters 67-77
Called from Main__diagnostics.exec.(fun) in file "bin/[diagnostics.ml](http://diagnostics.ml/)", line
  50, characters 55-78
Called from Stdlib__List.iter in file "[list.ml](http://list.ml/)", line 110, characters 12-15
Called from Fiber__core.O.(>>|).(fun) in file "otherlibs/fiber/src/[core.ml](http://core.ml/)",
  line 250, characters 36-41
Called from Fiber__scheduler.exec in file "otherlibs/fiber/src/[scheduler.ml](http://scheduler.ml/)",
  line 73, characters 8-11

I must not crash.  Uncertainty is the mind-killer. Exceptions are the
little-death that brings total obliteration.  I will fully express my cases.
Execution will pass over me and through me.  And when it has gone past, I
will unwind the stack along its path.  Where the cases are handled there will
be nothing.  Only I will remain.

And that the commit from which this error was triggered is: ocaml-flambda/flambda-backend@7ae1088

[nojb]

The exception is raised by

dune/otherlibs/stdune/src/loc.ml

Line 72 in 3647393

++ pp_left_pad (stop_c + padding_width + 3) (String.make len '^')

when len < 0. If the start and stop positions are not guaranteed to be coherent (ie the byte offset of start to be at most that of stop), then this looks like an actual bug?

[rgrinberg]

Indeed. Although I'd argue that we shouldn't crash on invalid locations regardless.

[ncik-roberts]

Indeed, this looks like a bug on a local branch of the OCaml compiler exactly as @nojb described. The start position exceeds the end position.

File "ocaml/typing/typecore.ml", lines 3640-3678, characters 17-11:
Error: This expression has type unit but an expression was expected of type
      'a -> 'b

I don't have evidence to suggest this compiler bug is present in the upstream OCaml compiler. But I would guess that, if you're developing a feature on a branch of the OCaml compiler, it's likely enough that you'll write a bug where the compiler starts producing invalid locations.

[Alizter]

What should we do with an invalid location? Warn the user? Or just ignore it and try our best?

[ncik-roberts]

Upon some more investigation, I take back what I said about the compiler producing an invalid location span. I think the compiler is producing a location span across multiple lines where the start's column is greater than the end's column. (So, a valid location span, but perhaps not one that dune is currently expecting.)

Here's a smaller reproduction using OCaml 4.14.1.

(* lib/tst.ml *)
type t = A | B

let f () () = function
  | A -> ()

$ dune build --passive-watch-mode
$ dune rpc build _build/install/default/bin/tst

This displays:

File "lib/tst.ml", lines 3-4, characters 14-11:
3 | ..............function
4 |   | A -> ()
Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
Here is an example of a case that is not matched:
B

Note how the location span is "lines 3-4, characters 14-11".

Then running dune diagnostics will show:

Error: exception Invalid_argument("Bytes.create")
Raised by primitive operation at Stdlib__Bytes.make in file "bytes.ml", line
  42, characters 10-18
Called from Stdlib__String.make in file "string.ml" (inlined), line 41,
  characters 2-12
Called from Stdune__loc.pp_file_excerpt.pp_file_excerpt in file
  "otherlibs/stdune/src/loc.ml", line 72, characters 52-73
Called from Stdune__result.bind in file "otherlibs/stdune/src/result.ml"
  (inlined), line 33, characters 12-15
Called from Stdune__result.O.(>>=) in file "otherlibs/stdune/src/result.ml"
  (inlined), line 68, characters 20-29
Called from Stdune__loc.pp_file_excerpt in file
  "otherlibs/stdune/src/loc.ml", line 117, characters 6-158
Called from Stdune__loc.pp in file "otherlibs/stdune/src/loc.ml", line 140,
  characters 5-72
Called from Main__diagnostics.format_diagnostic in file "bin/diagnostics.ml",
  line 25, characters 67-77
Called from Main__diagnostics.exec.(fun) in file "bin/diagnostics.ml", line
  50, characters 55-78
Called from Stdlib__List.iter in file "list.ml", line 110, characters 12-15
Called from Fiber__core.O.(>>|).(fun) in file "otherlibs/fiber/src/core.ml",
  line 250, characters 36-41
Called from Fiber__scheduler.exec in file "otherlibs/fiber/src/scheduler.ml",
  line 73, characters 8-11

I must not crash.  Uncertainty is the mind-killer. Exceptions are the
little-death that brings total obliteration.  I will fully express my cases. 
Execution will pass over me and through me.  And when it has gone past, I
will unwind the stack along its path.  Where the cases are handled there will
be nothing.  Only I will remain.