Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

Exit parser instead of raising error #77

Merged
merged 1 commit into from Jul 28, 2020
Merged

Conversation

chenglou
Copy link
Member

@chenglou chenglou commented Jul 28, 2020

Exit parser instead of raising error

See #68. The problem we have is the 2nd and 3rd screenshots, with a superfluous and nonexistent location reported.

This happens because BS/OCaml's way of reporting an error is to catch e.g. a Location.Error exception. The reporting displays the location, then move on. This is problematic for our syntax, because we report more than one error (i.e. we have more than one location to show).

The theoretical solution is to refactor the BS/OCaml reporting pipeline to accept a new exception that allows more than one error. But this shakes things up too much, so our temporary solution is to print all the errors on the syntax side, then simply exit the process. No exception raised means the BS side also doesn't catch anything and tries to display that nonsensical location line.

Caveats

  • (Not implemented yet) We need to reproduce the display logic inside the syntax code. We'll use the same display logic as BS' super-errors for now, and assume that most folks using BS also have super-errors turned on (which is the default). The result is that when we run bsb, a syntax error display and a compiler error display are visually consistent, even though they're produced by 2 different pieces of code. The look will be inconsistent if user has super-errors disabled; a type error (from BS) would look rustic, while a syntax error (from here) still looks super. Technically the compiler can invoke the syntax api and pass a flag to make it display the old ocaml error format, but nah.
  • Exiting a process is a hard control flow. The compiler can't even "catch" it. This is problematic if the compiler wants to keep operating on the file after syntax errors. Right now there's no such thing as "catching a syntax error and recover and keep analyzing the file", but we'll likely need this in the future when in the editor, the compiler operates on the recovered syntax tree in order to provide best-effort type hint, autocomplete, etc. Can't have the syntax exit the compiler process there =).
  • The online playground, compiled through jsoo, naturally doesn't understand "exiting a process". We'll need some patching here.

Anyway, temporary, rather stable solution. After other things settle a bit, we'll come back to this and properly fix it.

@chenglou
Copy link
Member Author

cc @IwanKaramazow @bobzhang @ryyppy

@ryyppy
Copy link
Member

ryyppy commented Jul 28, 2020

I think this will actually not interfere with the JSOO implementation since we are overriding the Napkin_driver functionality and using bits of the Napkin_diagnostics module directly

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants