From eb4badab2476dd6980f9edc92dad9a192ccc6542 Mon Sep 17 00:00:00 2001 From: David Majda Date: Sun, 24 Jun 2012 16:55:30 +0200 Subject: [PATCH] Refactor named rules AST representation PEG.js grammar rules are represented by |rule| nodes in the AST. Until now, all such nodes had a |displayName| property which was either |null| or stored rule's human-readable name. This commit gets rid of the |displayName| property and starts representing rules with a human-readable name using a new |named| node (a child of the |rule| node). This change simplifies code generation code a bit as tests for |displayName| can be removed (see changes in generate-code.js). It also separates different concerns from each other nicely. --- spec/compiler/passes/compute-params.spec.js | 7 +++++ .../compiler/passes/compute-var-names.spec.js | 11 ++++++++ .../passes/remove-proxy-rules.spec.js | 7 +++++ .../passes/report-left-recursion.spec.js | 4 +++ .../passes/report-missing-rules.spec.js | 4 +++ spec/generated-parser.spec.js | 15 +++++----- spec/parser.spec.js | 28 +++++++++++-------- src/compiler/passes/compute-params.js | 4 +++ src/compiler/passes/compute-var-names.js | 2 ++ src/compiler/passes/generate-code.js | 19 +++++++------ src/compiler/passes/remove-proxy-rules.js | 1 + src/compiler/passes/report-left-recursion.js | 1 + src/compiler/passes/report-missing-rules.js | 1 + src/parser.js | 9 ++++-- src/parser.pegjs | 9 ++++-- 15 files changed, 91 insertions(+), 31 deletions(-) diff --git a/spec/compiler/passes/compute-params.spec.js b/spec/compiler/passes/compute-params.spec.js index 5903702..4355836 100644 --- a/spec/compiler/passes/compute-params.spec.js +++ b/spec/compiler/passes/compute-params.spec.js @@ -41,6 +41,13 @@ describe("compiler pass |computeParams|", function() { }); describe("recursive walk", function() { + it("computes params for a named", function() { + expect(pass).toChangeAST( + 'start "start" = a:"a" { }', + innerExpressionDetails({ params: { a: "result0" } }) + ); + }); + it("computes params for a choice", function() { expect(pass).toChangeAST( 'start = a:"a" { } / "b" / "c"', diff --git a/spec/compiler/passes/compute-var-names.spec.js b/spec/compiler/passes/compute-var-names.spec.js index 57eff42..036d5b6 100644 --- a/spec/compiler/passes/compute-var-names.spec.js +++ b/spec/compiler/passes/compute-var-names.spec.js @@ -22,6 +22,17 @@ describe("compiler pass |computeVarNames|", function() { function ruleDetails(details) { return { rules: [details] }; } + it("computes variable names for a named", function() { + expect(pass).toChangeAST('start "start" = &"a"', ruleDetails({ + resultVars: ["result0"], + posVars: ["pos0"], + expression: { + resultVar: "result0", + expression: { resultVar: "result0", posVar: "pos0" } + } + })); + }); + it("computes variable names for a choice", function() { expect(pass).toChangeAST('start = &"a" / &"b" / &"c"', ruleDetails({ resultVars: ["result0"], diff --git a/spec/compiler/passes/remove-proxy-rules.spec.js b/spec/compiler/passes/remove-proxy-rules.spec.js index 8b9dd80..6cde306 100644 --- a/spec/compiler/passes/remove-proxy-rules.spec.js +++ b/spec/compiler/passes/remove-proxy-rules.spec.js @@ -23,6 +23,13 @@ describe("compiler pass |removeProxyRules|", function() { }); }); + it("removes proxy rule from a named", function() { + expect(pass).toChangeAST( + proxyGrammar('start "start" = proxy'), + simpleDetails + ); + }); + it("removes proxy rule from a choice", function() { expect(pass).toChangeAST( proxyGrammar('start = proxy / "a" / "b"'), diff --git a/spec/compiler/passes/report-left-recursion.spec.js b/spec/compiler/passes/report-left-recursion.spec.js index 79c6dce..1155b02 100644 --- a/spec/compiler/passes/report-left-recursion.spec.js +++ b/spec/compiler/passes/report-left-recursion.spec.js @@ -42,6 +42,10 @@ describe("compiler pass |reportLeftRecursion|", function() { expect(pass).toReportLeftRecursionIn('start = start'); }); + it("reports left recursion inside a named", function() { + expect(pass).toReportLeftRecursionIn('start "start" = start'); + }); + it("reports left recursion inside a choice", function() { expect(pass).toReportLeftRecursionIn('start = start / "a" / "b"'); expect(pass).toReportLeftRecursionIn('start = "a" / "b" / start'); diff --git a/spec/compiler/passes/report-missing-rules.spec.js b/spec/compiler/passes/report-missing-rules.spec.js index 2852d32..edd221d 100644 --- a/spec/compiler/passes/report-missing-rules.spec.js +++ b/spec/compiler/passes/report-missing-rules.spec.js @@ -42,6 +42,10 @@ describe("compiler pass |reportMissingRules|", function() { expect(pass).toReportMissingRuleIn('start = missing'); }); + it("reports missing rule referenced from a named", function() { + expect(pass).toReportMissingRuleIn('start "start" = missing'); + }); + it("reports missing rule referenced from a choice", function() { expect(pass).toReportMissingRuleIn('start = missing / "a" / "b"'); expect(pass).toReportMissingRuleIn('start = "a" / "b" / missing'); diff --git a/spec/generated-parser.spec.js b/spec/generated-parser.spec.js index c5309a3..3ac038f 100644 --- a/spec/generated-parser.spec.js +++ b/spec/generated-parser.spec.js @@ -171,17 +171,18 @@ describe("generated parser", function() { expect(parser).toParse("ac", 2); }); } + }); - it("does not overwrite expected string on failure when not named", function() { - var parser = PEG.buildParser('start = [0-9]', options); + describe("named matching", function() { + var parser = PEG.buildParser('start "start" = "a"'); - expect(parser).toFailToParse("a", { expected: ["[0-9]"] }); + it("delegates to the expression", function() { + expect(parser).toParse("a", "a"); + expect(parser).toFailToParse("b"); }); - it("overwrites expected string on failure when named", function() { - var parser = PEG.buildParser('start "digit" = [0-9]', options); - - expect(parser).toFailToParse("a", { expected: ["digit"] }); + it("overwrites expected string on failure", function() { + expect(parser).toFailToParse("b", { expected: ["start"] }); }); }); diff --git a/spec/parser.spec.js b/spec/parser.spec.js index 1d8ebca..701bf47 100644 --- a/spec/parser.spec.js +++ b/spec/parser.spec.js @@ -23,18 +23,16 @@ describe("PEG.js grammar parser", function() { }; function oneRuleGrammar(expression) { - var initializer = arguments.length > 1 ? arguments[1] : null, - displayName = arguments.length > 2 ? arguments[2] : null; + var initializer = arguments.length > 1 ? arguments[1] : null; return { type: "grammar", initializer: initializer, rules: [ { - type: "rule", - name: "start", - displayName: displayName, - expression: expression + type: "rule", + name: "start", + expression: expression } ], startRule: "start" @@ -159,9 +157,9 @@ describe("PEG.js grammar parser", function() { /* Canonical grammar is "a = \"abcd\"; b = \"efgh\"; c = \"ijkl\";". */ it("parses grammar", function() { - var ruleA = { type: "rule", name: "a", displayName: null, expression: literalAbcd }, - ruleB = { type: "rule", name: "b", displayName: null, expression: literalEfgh }, - ruleC = { type: "rule", name: "c", displayName: null, expression: literalIjkl }; + var ruleA = { type: "rule", name: "a", expression: literalAbcd }, + ruleB = { type: "rule", name: "b", expression: literalEfgh }, + ruleC = { type: "rule", name: "c", expression: literalIjkl }; expect('a = "abcd"').toParseAs({ type: "grammar", @@ -200,7 +198,11 @@ describe("PEG.js grammar parser", function() { oneRuleGrammar(choiceOfLiterals) ); expect('start "start rule" = "abcd" / "efgh" / "ijkl"').toParseAs( - oneRuleGrammar(choiceOfLiterals, null, "start rule") + oneRuleGrammar({ + type: "named", + name: "start rule", + expression: choiceOfLiterals + }) ); expect('start = "abcd" / "efgh" / "ijkl";').toParseAs( oneRuleGrammar(choiceOfLiterals) @@ -369,7 +371,11 @@ describe("PEG.js grammar parser", function() { /* Canonical string is "\"abcd\"". */ it("parses string", function() { - var grammar = oneRuleGrammar(literalAbcd, null, "abcd"); + var grammar = oneRuleGrammar({ + type: "named", + name: "abcd", + expression: literalAbcd + }); expect('start "abcd" = "abcd"' ).toParseAs(grammar); expect('start \'abcd\' = "abcd"').toParseAs(grammar); diff --git a/src/compiler/passes/compute-params.js b/src/compiler/passes/compute-params.js index 7c48ecd..bedcc4e 100644 --- a/src/compiler/passes/compute-params.js +++ b/src/compiler/passes/compute-params.js @@ -42,6 +42,10 @@ PEG.compiler.passes.computeParams = function(ast) { }, rule: computeForScopedExpression, + named: + function(node) { + compute(node.expression); + }, choice: function(node) { diff --git a/src/compiler/passes/compute-var-names.js b/src/compiler/passes/compute-var-names.js index d7f1613..fd7b9df 100644 --- a/src/compiler/passes/compute-var-names.js +++ b/src/compiler/passes/compute-var-names.js @@ -64,6 +64,8 @@ PEG.compiler.passes.computeVarNames = function(ast) { node.posVars = map(range(depth.pos), posVar); }, + named: computeFromExpression({ result: 0, pos: 0 }), + choice: function(node, index) { var depths = map(node.alternatives, function(alternative) { diff --git a/src/compiler/passes/generate-code.js b/src/compiler/passes/generate-code.js index c1a7158..694d957 100644 --- a/src/compiler/passes/generate-code.js +++ b/src/compiler/passes/generate-code.js @@ -565,16 +565,7 @@ PEG.compiler.passes.generateCode = function(ast, options) { ' var #{node.posVars.join(", ")};', ' #end', ' ', - ' #if node.displayName !== null', - ' reportFailures++;', - ' #end', ' #block emit(node.expression)', - ' #if node.displayName !== null', - ' reportFailures--;', - ' if (reportFailures === 0 && #{node.resultVar} === null) {', - ' matchFailed(#{string(node.displayName)});', - ' }', - ' #end', ' #if options.cache', ' ', ' cache[cacheKey] = {', @@ -585,6 +576,14 @@ PEG.compiler.passes.generateCode = function(ast, options) { ' return #{node.resultVar};', '}' ], + named: [ + 'reportFailures++;', + '#block emit(node.expression)', + 'reportFailures--;', + 'if (reportFailures === 0 && #{node.resultVar} === null) {', + ' matchFailed(#{string(node.name)});', + '}' + ], choice: [ '#block emit(alternative)', '#block nextAlternativesCode' @@ -815,6 +814,8 @@ PEG.compiler.passes.generateCode = function(ast, options) { * variables. */ + named: emitSimple("named"), + choice: function(node) { var code, nextAlternativesCode; diff --git a/src/compiler/passes/remove-proxy-rules.js b/src/compiler/passes/remove-proxy-rules.js index 862118f..cafdef2 100644 --- a/src/compiler/passes/remove-proxy-rules.js +++ b/src/compiler/passes/remove-proxy-rules.js @@ -24,6 +24,7 @@ PEG.compiler.passes.removeProxyRules = function(ast) { var replace = buildNodeVisitor({ grammar: replaceInSubnodes("rules"), rule: replaceInExpression, + named: replaceInExpression, choice: replaceInSubnodes("alternatives"), sequence: replaceInSubnodes("elements"), labeled: replaceInExpression, diff --git a/src/compiler/passes/report-left-recursion.js b/src/compiler/passes/report-left-recursion.js index 8c46b22..ae89e5d 100644 --- a/src/compiler/passes/report-left-recursion.js +++ b/src/compiler/passes/report-left-recursion.js @@ -22,6 +22,7 @@ PEG.compiler.passes.reportLeftRecursion = function(ast) { check(node.expression, appliedRules.concat(node.name)); }, + named: checkExpression, choice: checkSubnodes("alternatives"), sequence: diff --git a/src/compiler/passes/report-missing-rules.js b/src/compiler/passes/report-missing-rules.js index d0a37ae..62b5e26 100644 --- a/src/compiler/passes/report-missing-rules.js +++ b/src/compiler/passes/report-missing-rules.js @@ -11,6 +11,7 @@ PEG.compiler.passes.reportMissingRules = function(ast) { var check = buildNodeVisitor({ grammar: checkSubnodes("rules"), rule: checkExpression, + named: checkExpression, choice: checkSubnodes("alternatives"), sequence: checkSubnodes("elements"), labeled: checkExpression, diff --git a/src/parser.js b/src/parser.js index b87aab8..ab6411f 100644 --- a/src/parser.js +++ b/src/parser.js @@ -277,8 +277,13 @@ PEG.parser = (function(){ return { type: "rule", name: name, - displayName: displayName !== "" ? displayName : null, - expression: expression + expression: displayName !== "" + ? { + type: "named", + name: displayName, + expression: expression + } + : expression }; })(pos0, result0[0], result0[1], result0[3]); } diff --git a/src/parser.pegjs b/src/parser.pegjs index a0958a9..2476278 100644 --- a/src/parser.pegjs +++ b/src/parser.pegjs @@ -21,8 +21,13 @@ rule return { type: "rule", name: name, - displayName: displayName !== "" ? displayName : null, - expression: expression + expression: displayName !== "" + ? { + type: "named", + name: displayName, + expression: expression + } + : expression }; }