Export a parser for Types and refactor a bit parser
#1227
Conversation
There was a problem hiding this comment.
This feels like it's coupling us quite tightly to larlpop implementation details. I'm not if that's a problem but it's worth pondering.
There was a problem hiding this comment.
Well, maybe that feeling only comes from all the underscores 😅 It seems the only real internal we're coupling to here is grammar::__ToTriple.
There was a problem hiding this comment.
I wouldn't say it does, because LalrpopParser ist in fact mostly a facade for Lalrpop-generated parsers. But everything else in the Nickel codebase should use the ErrorTolerantParser trait, which is mostly independent from Lalrpop (that's not entirely true, because of error type and so forth, but I think it's hardly avoidable). Maybe I should make this trait private? I wonder if it's possible to depend on private trait for a public trait. Let me try.
There was a problem hiding this comment.
I think 1c31f53 decouples the trait implements a bit, in a cleaner way, although it's a tad more verbose (we had to repeat the interface of parse_tolerant and parse_strict).
There was a problem hiding this comment.
I think a private trait makes sense here; even though the coupling objection was a bit vague in the first place. 👍
This PR has been done as part of fixing #1187, and split as a stand-alone change. For some reason I needed to parse types. This PR exposes the rule for parsing type in the LALRPOP grammar.
I also noticed that the code already present had duplication, and that I was going to add one more copy of the
parse_term_tolerantandparse_termmethods. Those methods are now implemented once and for all for any parser generated by Lalrpop, and have been renamed to be more consistent with the rest of the codebase.