RedPRL / asai

🩺 A library for compiler diagnostics
https://ocaml.org/p/asai
Apache License 2.0
35 stars 2 forks source link

❌ Explicator crashes when encountering a standalone backslash? #188

Closed kentookura closed 2 weeks ago

kentookura commented 2 weeks ago

The contents of test.tree is a single backslash.

 → error[Reporter.Message.Parse_error]
 ꭍ ○ when parsing file `./trees/test.tree`
Fatal error: exception Invalid_argument("Asai.Explicator.explicate: unexpected newline; use the debug mode")
Raised at Stdlib__Effect.Deep.try_with.(fun) in file "effect.ml", line 101, characters 47-54
Called from Eio_unix__Thread_pool.run in file "lib_eio/unix/thread_pool.ml", line 108, characters 8-13
Re-raised at Eio_unix__Thread_pool.run in file "lib_eio/unix/thread_pool.ml", line 113, characters 4-39
Called from Eio_linux__Sched.run.(fun) in file "lib_eio_linux/sched.ml", line 498, characters 22-90
Re-raised at Eio_linux__Sched.run in file "lib_eio_linux/sched.ml", line 509, characters 22-57
Called from Eio_linux__Sched.with_eventfd in file "lib_eio_linux/sched.ml", line 536, characters 8-18
Re-raised at Eio_linux__Sched.with_eventfd in file "lib_eio_linux/sched.ml", line 541, characters 4-39
Called from Eio_linux__Sched.with_sched in file "lib_eio_linux/sched.ml", lines 570-572, characters 8-109
Re-raised at Eio_linux__Sched.with_sched in file "lib_eio_linux/sched.ml", line 583, characters 8-43
Called from Forester_prelude__Fun_util.let@ in file "lib/prelude/Fun_util.ml" (inlined), line 7, characters 13-19
Called from Dune__exe__Main in file "bin/forester/main.ml", lines 293-295, characters 2-44
favonia commented 2 weeks ago

@kentookura Did you enable the debug mode (by passing ~debug:true to Tty.S.display)?

kentookura commented 2 weeks ago

Aha ❗

→ error[Reporter.Message.Parse_error]
 ꭍ ○ when parsing file `./trees/jms-00EY.tree`
Fatal error: exception Invalid range:
  Range
    ({source=(`File "./trees/jms-00EY.tree"); offset=406; start_of_line=405;
      line_num=9},
     {source=(`File "./trees/jms-00EY.tree"); offset=407; start_of_line=405;
      line_num=9})
  is invalid because its ending position is invalid; its line number is 9 but
  it should have been 10.
kentookura commented 2 weeks ago

For consistency's sake, here is the output for a file that just contains a single backslash again:

❮ forester build         
 → error[Reporter.Message.Parse_error]
 ꭍ ○ when parsing file `./trees/test.tree`
Fatal error: exception Invalid range:
  Range
    ({source=(`File "./trees/test.tree"); offset=1; start_of_line=0;
      line_num=1},
     {source=(`File "./trees/test.tree"); offset=2; start_of_line=0;
      line_num=1})
  is invalid because its ending position is invalid; its line number is 1 but
  it should have been 2.
favonia commented 2 weeks ago

@kentookura While the asai library could still have bugs, the paranoid mode is extremely robust 😉 I guess the line number was not increased after \n? Is that the cause?

kentookura commented 2 weeks ago

I think it is the converse: The file does not contain a newline, but the line number is expected to be 2?

favonia commented 2 weeks ago

@kentookura That sounds quite impossible. Could you either send me the file, show me the output of hexdump -C FILE, or confirm that the size of the file is 1?

kentookura commented 2 weeks ago
❮ hexdump -C trees/test.tree
00000000  5c 0a                                             |\.|
00000002
favonia commented 2 weeks ago

@kentookura The output shows that there are two bytes in the file. The first byte is the slash. The second byte (with the hex code 0a) is the newline. Here is the table of offsets and bytes:

Offset 0 Byte 0 Offset 1 Byte 1 Offset 2
\ begin \n end

The ending position is after the newline character, so it indeed should have the line number 2. The bug is in forester's lexer, not asai.

Maybe I should tag @jonsterling...? 😆

favonia commented 2 weeks ago

It seems that you agreed. Let me close this issue, then 😅

kentookura commented 2 weeks ago

Yes, thanks for your help!

favonia commented 2 weeks ago

@kentookura Hmm however there's a usability problem here. The highlighting will not do anything 🤔 because the range only contains the newline character. As we are adjusting the renderer, what should be the output? Any suggestion?

kentookura commented 2 weeks ago

Ah, we have encountered this before. When a delimiter is not closed, we used to get such an error message. What I have opted to do is to use menhir's incremental API to make a stack of opening delimiters along with their ranges, that gets pushed and popped during parsing. This allows me to reconstruct a bigger range for printing a more informative error.

kentookura commented 2 weeks ago

Although, I am currently refining this code. In a long file, an early unclosed delimiter would create a very large highlighted range. So now I am checking if the distance between the unclosed delimiter and the range that contains the parse error is not too large. Granted, this is a solution for a very specific setup, I don't know if we can extract a useful general design from this.

favonia commented 2 weeks ago

an early unclosed delimiter would create a very large highlighted range.

Hmm... a workaround is to have "..." for the middle part (despite being highlighted)? Currently, asai assumed highlighted text is important and will never omit any part of it. However, this can be changed.

kentookura commented 2 weeks ago

Yes, this is essentially what I am trying to implement.

favonia commented 2 weeks ago

I think you are trying to improve things by having a smaller range. I wonder if it's also helpful for asai to display a super large range more concisely. I think these are two different approaches. 😅

kentookura commented 2 weeks ago

Ah, I had thought that your suggestion was an easy step from what I am trying to do, since I have access to both the beginning and the end of the range I want to print, but you are right: Just because I have the ranges does not mean I can print them in the way that you suggested...

kentookura commented 2 weeks ago

But I think this discussion is out of scope for this issue 😅