feat: add support for templates #337
Conversation
quadristan
force-pushed
the
templates
branch
from
88b61f3 to
0398f52
Compare
|
This is the issue in the original pegjs repo: pegjs/pegjs#45 |
I have looked at the issue. |
There was a problem hiding this comment.
Introducing templates is a big task and thanks that you are working on it! I left some comments.
I think, that firstly we should decide, how the templates should be implemented. Should we instantiate rules (then we should think about recursion) or use high-order functions to implement templates? We also could implement both variants.
Another question to discuss is where th template passes should be inserted in current compilation pipeline? It seems valuable to work with templates after some basic checks, like duplicated or undefined rules, but other checks should be done after recognizing templates, like checking for infinite recursion.
| // Javascript does not let us have variables names such as Map<Key,Value> | ||
| // Instead, it accepts MapᐊKeyͺValueᐅ | ||
| // See https://es5.github.io/x7.html#x7.6 | ||
| const name = `${template.name}ᐊ${templateArgs.map(a => a.name).join("ͺ")}ᐅ`; |
There was a problem hiding this comment.
Such a replacement should be done in the generate-js pass
| if (node.templateArgs) { | ||
| const targetRule = asts.findRule(ast, targetName); | ||
| if (!targetRule) { | ||
| throw new GrammarError(`Rule "${targetName}" is not defined`, node.location); |
There was a problem hiding this comment.
You should be prefer to use session.error() instead of direct throwing GrammarError, because that will allow you to collect many errors at once. You should be prepared, that session.error() will not throw.
| const targetRule = asts.findRule(ast, node.name); | ||
| if (!targetRule) { | ||
| throw new GrammarError(`Rule "${node.name}" is not defined`, node.location); | ||
| } |
There was a problem hiding this comment.
It is better to run template instantiation after checking for existence of all rules.
| GenericsArgumentsDeclaration | ||
| = "<" __ | ||
| head:IdentifierName __ | ||
| tail:("," __ @IdentifierName __ )* | ||
| ">" { |
There was a problem hiding this comment.
We now have a repeated expression, so it would be good to rewrite using it. I personally also love, if dangling comma would be supported, because this simplifies refactorings and auto-generation.
The location of each parameters is also worth to be saved, so we can report:
- unused parameters
- incorrect recursion via parameters
| templateArgs | ||
| = "<" __ | ||
| head:RuleReferenceExpression __ | ||
| tail:("," __ @RuleReferenceExpression)* | ||
| ">" { | ||
| return [head,...tail] | ||
| } | ||
|
|
There was a problem hiding this comment.
It would be worth to allow instantiate with any expression. At least, literals could be widely use
| templates: [ | ||
| instantiateTemplates, | ||
| removeNonInstantiatedTemplates, | ||
| ], |
There was a problem hiding this comment.
It would be better to work with templates after doing some basic checks, like undefined or duplicating rules
|
|
||
| while (rulesToRemove.length) { | ||
| const name = rulesToRemove.pop(); | ||
| session.info("Removing dangling generic rule", name); |
There was a problem hiding this comment.
If we use word "template" then it should be used consistently.
Co-authored-by: Mingun <Alexander_Sergey@mail.ru>
Add support for template rules
📘 Glossary
templatetogenericbut feel free to make me change my mindℹ️ Description
A template is a rule that is given parameters.
can be turned into a template
and then used
Example of full templated grammar is in the MR.
💨 Fast fast !
This MR takes performances in mind.$O(N)$ fashion.
Without templated rules, it will only do a quick pass on existing rules_ref , in a
I compared parsing the example
javascript.pegjsin main / in my branch, nothing changes, really.With templated rules, it becomes a bit slower. The complexity in time/space is of the number templates instantiations times the complexity of parsing said rules.
⚙️ Generated code
Generated code must follow javascript standard.
There is no way to write
But instead we can write, according to this
MapᐊKeyͺValueᐅLet me know if that is not acceptable for generated code. I could add IDs on top of symbols, but i believe it will slow down performances for not such gain
💖 Personal motivation
My personal motivation to pass this is to be able to create a grammar for https://github.com/structurizr/dsl/blob/master/docs/language-reference.md . There is a lot of repetition amongst blocks, so i want to use templates to do it
👓 Scope of this PR
The way templates are implemented have room for improvements, i may open additional MRs depending on my bandwith:
expressionsinstead ofrule_refsas template arguments