Skip to content

Conversation

@istathar
Copy link
Member

@istathar istathar commented Sep 1, 2025

Until now we have just exited on detecting the first parsing or validation error, but the forthcoming language server will need to know where all the problems are. So the next step in maturing the parser is collecting more than a single error at a time when checking the validity of a document.

The Parser state object is updated to include a Vec that the parser (and subparsers) can contribute to.

Finally, when reporting errors we deduplicate, with more specific messages overriding enclosing general ones. This is an admittedly rare occurrence, as usually fail-fast short-circuiting will cause a single error to propagate, but in the event nesting results in multiple errors at the same site we trim it down to a single message.

@istathar istathar self-assigned this Sep 1, 2025
@istathar istathar marked this pull request as draft September 1, 2025 14:15
@istathar
Copy link
Member Author

istathar commented Sep 1, 2025

This is going to be a big refactoring. Gathering together pieces as they occur. Many subsequent commits to follow.

@istathar
Copy link
Member Author

istathar commented Sep 2, 2025

I've got another branch going to try and simplify this, but this branch works!

$ technique format tests/broken/BadDeclaration.tq 
error: tests/broken/BadDeclaration.tq:1:29 Invalid signature

error: tests/broken/BadDeclaration.tq:3:1 Invalid identifier

Unable to parse input file. Try `technique check tests/broken/BadDeclaration.tq` for details.

@istathar istathar marked this pull request as ready for review September 2, 2025 13:20
@istathar
Copy link
Member Author

istathar commented Sep 3, 2025

The implementation was getting excessively complicated. At one point we had this type going on:

Result<(Result<Procedure<'i>, ParsingError<'i>>, Vec<ParsingError<'i>>), ParingError<'_>>

which is slightly ridiculous. Continued refactoring, changing the parser to return a ParseOutcome which contains the result value and/or the errors collected along the way in there instead of in the Parser state.

That led to types that were still a little noisy, with things like

ParseOutcome<'i, Procedure<'i>>

the reason for the lifetime was that the enclosed Vec or ParsingError<'i> needed that lifetime, but only the InvalidIdentifier variant there actually used it. For small strings like that we can just make an owned String. That gave us

ParseOutcome<Procedure<'i>>

which is way easier to work with and the built up structures are still zero-copy which is really important.

@istathar
Copy link
Member Author

istathar commented Sep 4, 2025

Well, after all that, jettisoned all of this because losing fail-fast via the ? operator was leading to obscene amounts of boilerplate code and unnecessary closures.

@istathar istathar merged commit c3588b9 into technique-lang:main Sep 4, 2025
1 check passed
@istathar istathar deleted the gather-errors branch September 4, 2025 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant