icerpc / slicec

The Slice compiler library
Apache License 2.0
13 stars 5 forks source link

Fix The Parser Exiting After a Single Error & Parser Cleanup #579

Closed InsertCreativityHere closed 1 year ago

InsertCreativityHere commented 1 year ago

This PR fixes #559

Previously, there was a single DiagnosticReporter that was shared across the parsing of every file. However, this made it difficult for a parser to know whether errors came from it, or other files.

Now, the parser allocates a Vec<Diagnostic> for each file it parses. Then, at the end of parsing, these diagnostics are forwarded to the single instance of DiagnosticReporter. Since each parser instance has it's own Vec, they can't be affected by other parsers anymore.

This change affected all the parsers, including the attribute 'parser'.

I didn't add a test, as there's no way to reliably test this behavior, since the order we parse files in is un-deterministic.


It also makes the Slice, preprocessor, and comment parsers more consistent, things are declared in the same order, use the same style for grammar rules, have consistent comments, etc.

All the logic is unchanged, except this one warning message which was changed: doc comment tag '{tag}' is invalid -> unknown doc comment tag '{tag}' It was changed to use the same message structure as all our other 'unknown X' messages.

externl commented 1 year ago

I think you can add a test by passing in two stings with two errors and making sure they both are emitted.

InsertCreativityHere commented 1 year ago

I think you can add a test by passing in two stings with two errors and making sure they both are emitted.

I added a test. My issue was that when I originally added a test, I used 2 different syntax errors, so the order of errors was always messed up. I added a new test where both files trigger the exact same error message, so the ordering doesn't matter. See: https://github.com/icerpc/slicec/pull/579/commits/e6bbf8261db614dd577f405235a9db70ff6af7d6