lark
lark copied to clipboard
lark-parser
Load files once
The changes implemented during the discussion of #992, and a basic implementation of %include (typo in commit message :-/). Should I also implement more complex load once stuff in this PR?
@MegaIng say i have 2 grammars A and B that both include a grammar C. now if i include grammars A and B in a grammar D, i get conflict errors as rules defined in grammar C are declared twice. is that wanted? in fact (not exclusive to %include), why prohibit defining a rule more than once, instead of using the last definition?
@ornariece Without a major rewrite of GrammarLoader, that is not avoidable. I will take %include out of this PR, and start on a rework. The rest of this PR works.
The logic is that you can manually specify %extend or %override which might both be correct behavior for a repeated definition.
HOW? I completely reverted those changes (they aren't even in my local files). I am not sure where git got those changes from...
@MegaIng (currently tweaking my grammars to use %include)
%extend can't be used on a rule that got defined through an %include statement (ie. not through direct definition or %import).
the error that is raised is lark.exceptions.GrammarError: Can't extend rule name as it wasn't defined before
@ornariece That is part of the reason why I removed this implementation of %include.
@MegaIng I'm probably missing something, but why not just make it use the same implementation of %import but just with an empty namespace?
@erezsh That leads exactly to the problem @ornariece describes, since imports (and then includes) are added before anything in the grammar is done, meaning that an included grammar could not extend symbols in the main grammar. Also, that would still create a full GrammarLoader instance for each included grammar, meaning that no actual import de duplication would happen.
an included grammar could not extend symbols in the main grammar
I think that's the desirable behavior.
Perhaps these symbols should be refactored into a shared.lark, from which they can be extended.
I think that's the desirable behavior.
I disagree. Including a grammar should behave no different then just pasting the entire content at that place.
@erezsh That is the plan, but that is interdependent of this PR. I will create one for those changes.
Sorry I'm only getting to this now. Any chance you can fix the PR for the recent changes?
It might even need a few more changes for allowing abnf and such? (maybe not. I didn't look that deeply yet)