bnfc icon indicating copy to clipboard operation
bnfc copied to clipboard

Haskell: structured errors

Open vkpgwt opened this issue 3 years ago • 2 comments

Hello! This is a proposed implementation of structured errors in Haskell (discussed in #423). Currently it's mostly done, but I guess I need to add some tests, and some issues should be discussed.

  • a new option --errors=string|structured. It's implemented for Haskell backend only, but its name is abstract enough to be used in other backends in future. --errors=string is a default.
  • Par.y now exports Failure type and some more types to represent parser errors.
  • Par.y now creates a Failure value and converts it to the old-style error string, if string errors are used. Otherwise it returns Either Failure a instead of Either String a. As I checked manually, the string error format hasn't been changed (maybe we need a test for it?).
  • ErrM.hs is not created with --errors=structured, since I couldn't find a way to modify its content appropriately. And it seems to be for keeping backward compatibility only. Now all error types are exported from Par.y.
  • I added record syntax to Posn type in Lex.x, since Posn is now used to report error position in exported failure types, and users might deal with it. I find it useful to use Posn as it is (absolute position too, not just the line and column numbers). In my project I need to get some context around the error position, which is more convenient to do when we have the absolute position too.

Some issues that might be fixed if needed:

  • [ ] no tests added (but a little test to see if ErrM.hs is omitted). I guess we need some success tests with --errors=structured, and the generated parser tests should be run too - what I have to do manually. Could you please make a hint what tests and where I should add?
  • [ ] no documentation added
  • [ ] I could not check the new option together with --glr, since --glr generates broken code on 2.9.4. This PR should make things even worse, since I added some directives to Par.y which change the lexer signature from the parser's perspective. I found some comments in the source code which mention that GLR is obsolete. Should I fix it?
  • [ ] you proposed adding option --errors=string|structured, but in the end I decided to add --string-errors and --structured-errors for the following reasons:
    • supporting an option with a checked argument would require to do some unwieldy refactoring in Options.hs. In fact, I did it and wasn't happy with the result.
    • it matches the style of other options: --glr, --text-tokens, --string-tokens, but not --tokens=....
  • [ ] In #423 you proposed also text argument for --errors option which is not supported in the PR, since I'm not sure I understand what it should mean. Should it just change the error type to Text, so that Err would be Either Text? Frankly speaking, I don't think it will be helpful or efficient, since error messages are short strings, and a user doing Text.pack err would get the same result with almost the same performance, as compared with Text errors constructed internally in BNFC. But if you think it's worth doing, I can add Text errors support.
  • [ ] rebase and squash fixup commits

vkpgwt avatar Aug 08 '22 09:08 vkpgwt

Hello @andreasabel! Would you mind to look at the PR?

vkpgwt avatar Aug 24 '22 11:08 vkpgwt

@andreasabel Is this blocked on something? Any design decisions are yet to be made?

dpwiz avatar Oct 28 '22 09:10 dpwiz