Error handling: Implement the |expected| function

The |expected| function allows users to report regular match failures inside actions. If the |expected| function is called, and the reported match failure turns out to be the cause of a parse error, the error message reported by the parser will be in the usual "Expected ... but found ..." format with the description specified in the |expected| call used as part of the message. Implements part of pegjs#198. Speed impact ------------ Before: 1146.82 kB/s After: 1031.25 kB/s Difference: -10.08% Size impact ----------- Before: 950817 b After: 973269 b Difference: 2.36% (Measured by /tools/impact with Node.js v0.6.18 on x86_64 GNU/Linux.)
for-GET · Apr 17, 2014 · 8c368cf · 8c368cf
1 parent 007a554
commit 8c368cf
Show file tree

Hide file tree

Showing 9 changed files with 354 additions and 146 deletions.
diff --git a/Makefile b/Makefile
@@ -9,6 +9,7 @@ MODULES = utils                                 \
           grammar-error                         \
           parser                                \
           compiler/opcodes                      \
+          compiler/flags                        \
           compiler/passes/generate-bytecode     \
           compiler/passes/generate-javascript   \
           compiler/passes/remove-proxy-rules    \

diff --git a/README.md b/README.md
@@ -376,6 +376,11 @@ expression as its arguments. The action should return some JavaScript value
 using the `return` statement. This value is considered match result of the
 preceding expression.
 
+To indicate a match failure, the code inside the action can invoke the
+`expected` function. It takes one parameter — a description of what was expected
+at the current position. This description will be used as part of a message of
+the exception thrown if the match failure leads to an parse error.
+
 The code inside the action can access all variables and functions defined in the
 initializer at the beginning of the grammar. Curly braces in the action code
 must be balanced.

diff --git a/lib/compiler/flags.js b/lib/compiler/flags.js
@@ -0,0 +1,6 @@
+/* Bytecode instruction flags. */
+module.exports = {
+  DONT_CHECK_FAILS: 0,
+  CHECK_FAILS:      1
+};
+
diff --git a/lib/compiler/passes/generate-bytecode.js b/lib/compiler/passes/generate-bytecode.js
@@ -1,5 +1,6 @@
 var utils = require("../../utils"),
-    op    = require("../opcodes");
+    op    = require("../opcodes"),
+    flags = require("../flags");
 
 /* Generates bytecode.
  *
@@ -151,9 +152,9 @@ var utils = require("../../utils"),
  *
  *        reportedPos = currPos;
  *
- * [23] CALL f, n, pc, p1, p2, ..., pN
+ * [23] CALL f, c, n, pc, p1, p2, ..., pN
  *
- *        value = consts[f](stack[p1], ..., stack[pN]);
+ *        value = call(consts[f], c, stack[p1], ..., stack[pN]);
  *        stack.pop(n);
  *        stack.push(value);
  *
@@ -206,10 +207,16 @@ module.exports = function(ast, options) {
     return condCode.concat([bodyCode.length], bodyCode);
   }
 
-  function buildCall(functionIndex, delta, env, sp) {
+  function buildCall(functionIndex, checkFails, delta, env, sp) {
     var params = utils.map( utils.values(env), function(p) { return sp - p; });
 
-    return [op.CALL, functionIndex, delta, params.length].concat(params);
+    return [
+      op.CALL,
+      functionIndex,
+      checkFails,
+      delta,
+      params.length
+    ].concat(params);
   }
 
   function buildSimplePredicate(expression, negative, context) {
@@ -248,7 +255,13 @@ module.exports = function(ast, options) {
 
     return buildSequence(
       [op.REPORT_CURR_POS],
-      buildCall(functionIndex, 0, context.env, context.sp),
+      buildCall(
+        functionIndex,
+        flags.DONT_CHECK_FAILS,
+        0,
+        context.env,
+        context.sp
+      ),
       buildCondition(
         [op.IF],
         buildSequence(
@@ -347,7 +360,13 @@ module.exports = function(ast, options) {
               [op.IF_NOT_ERROR],
               buildSequence(
                 [op.REPORT_SAVED_POS, 1],
-                buildCall(functionIndex, 1, env, context.sp + 2)
+                buildCall(
+                  functionIndex,
+                  flags.CHECK_FAILS,
+                  1,
+                  env,
+                  context.sp + 2
+                )
               ),
               []
             ),
@@ -396,6 +415,7 @@ module.exports = function(ast, options) {
               [op.REPORT_SAVED_POS, node.elements.length],
               buildCall(
                 functionIndex,
+                flags.CHECK_FAILS,
                 node.elements.length,
                 context.env,
                 context.sp

diff --git a/lib/compiler/passes/generate-javascript.js b/lib/compiler/passes/generate-javascript.js
@@ -1,5 +1,6 @@
 var utils = require("../../utils"),
-    op    = require("../opcodes");
+    op    = require("../opcodes"),
+    flags = require("../flags");
 
 /* Generates parser JavaScript code. */
 module.exports = function(ast, options) {
@@ -102,7 +103,7 @@ module.exports = function(ast, options) {
     }
 
     function generateCall() {
-      var baseLength       = 4,
+      var baseLength       = 5,
           paramsLengthCode = 'bc[ip + ' + (baseLength - 1) + ']';
 
       return [
@@ -111,11 +112,15 @@ module.exports = function(ast, options) {
         '  params[i] = stack[stack.length - 1 - params[i]];',
         '}',
         '',
-        'stack.splice(',
-        '  stack.length - bc[ip + 2],',
-        '  bc[ip + 2],',
-        '  peg$consts[bc[ip + 1]].apply(null, params)',
-        ');',
+        'if (bc[ip + 2] === ' + flags.CHECK_FAILS + ') {',
+        '  peg$userFail = false;',
+        '}',
+        'result = peg$consts[bc[ip + 1]].apply(null, params);',
+        'if (bc[ip + 2] === ' + flags.CHECK_FAILS + ') {',
+        '  if (peg$userFail) { result = peg$FAILED; }',
+        '}',
+        '',
+        'stack.splice(stack.length - bc[ip + 3], bc[ip + 3], result);',
         '',
         'ip += ' + baseLength + ' + ' + paramsLengthCode + ';',
         'break;'
@@ -140,7 +145,7 @@ module.exports = function(ast, options) {
       '      end   = bc.length,',
       '      ends  = [],',
       '      stack = [],',
-      '      params, i;',
+      '      params, result, i;',
       ''
     ].join('\n'));
 
@@ -272,7 +277,7 @@ module.exports = function(ast, options) {
       '        case ' + op.FAIL + ':',             // FAIL e
       '          stack.push(peg$FAILED);',
       '          if (peg$silentFails === 0) {',
-      '            peg$fail(peg$consts[bc[ip + 1]]);',
+      '            peg$fail(peg$consts[bc[ip + 1]], peg$currPos);',
       '          }',
       '          ip += 2;',
       '          break;',
@@ -419,7 +424,7 @@ module.exports = function(ast, options) {
       }
 
       function compileCall(cond) {
-        var baseLength   = 4,
+        var baseLength   = 5,
             paramsLength = bc[ip + baseLength - 1];
 
         var params = utils.map(
@@ -430,8 +435,11 @@ module.exports = function(ast, options) {
         params.push("'" + rule.name + "'");
 
         var value = c(bc[ip + 1]) + '(' + params.join(', ') + ')';
-        stack.pop(bc[ip + 2]);
+        stack.pop(bc[ip + 3]);
         parts.push(stack.push(value));
+        if (bc[ip + 2] === flags.CHECK_FAILS) {
+          parts.push('if (peg$userFail) { ' + stack.top() + ' = peg$FAILED; }');
+        }
         ip += baseLength + paramsLength;
       }
 
@@ -588,7 +596,7 @@ module.exports = function(ast, options) {
 
           case op.FAIL:             // FAIL e
             parts.push(stack.push('peg$FAILED'));
-            parts.push('if (peg$silentFails === 0) { peg$fail(' + c(bc[ip + 1]) + '); }');
+            parts.push('if (peg$silentFails === 0) { peg$fail(' + c(bc[ip + 1]) + ', peg$currPos); }');
             ip += 2;
             break;
 
@@ -793,6 +801,7 @@ module.exports = function(ast, options) {
     '        peg$maxFailPos       = 0,',
     '        peg$maxFailExpected  = [],',
     '        peg$silentFails      = 0,', // 0 = report failures, > 0 = silence failures
+    '        peg$userFail         = false,',
     ''
   ].join('\n'));
 
@@ -845,6 +854,13 @@ module.exports = function(ast, options) {
     '      return peg$computePosDetails(peg$reportedPos).column;',
     '    }',
     '',
+    '    function expected(description) {',
+    '      if (peg$silentFails === 0) {',
+    '        peg$fail({ type: "other", description: description }, peg$reportedPos);',
+    '      }',
+    '      peg$userFail = true;',
+    '    }',
+    '',
     '    function peg$computePosDetails(pos) {',
     '      function advance(details, startPos, endPos) {',
     '        var p, ch;',
@@ -878,19 +894,19 @@ module.exports = function(ast, options) {
     '      return peg$cachedPosDetails;',
     '    }',
     '',
-    '    function peg$fail(expected) {',
-    '      if (peg$currPos < peg$maxFailPos) { return; }',
+    '    function peg$fail(expected, pos) {',
+    '      if (pos < peg$maxFailPos) { return; }',
     '',
-    '      if (peg$currPos > peg$maxFailPos) {',
-    '        peg$maxFailPos = peg$currPos;',
+    '      if (pos > peg$maxFailPos) {',
+    '        peg$maxFailPos = pos;',
     '        peg$maxFailExpected = [];',
     '      }',
     '',
     '      peg$maxFailExpected.push(expected);',
     '    }',
     '',
     '    function peg$cleanupExpected(expected) {',
-    '      var i = 0;',
+    '      var i = 1;',
     '',
     '      expected.sort(function(a, b) {',
     '        if (a.description < b.description) {',
@@ -902,13 +918,8 @@ module.exports = function(ast, options) {
     '        }',
     '      });',
     '',
-    /*
-     * This works because the bytecode generator guarantees that every
-     * expectation object exists only once, so it's enough to use |===| instead
-     * of deeper structural comparison.
-     */
     '      while (i < expected.length) {',
-    '        if (expected[i - 1] === expected[i]) {',
+    '        if (expected[i - 1].description === expected[i].description) {',
     '          expected.splice(i, 1);',
     '        } else {',
     '          i++;',