Optimize silent fails: remove checks that always false (#399)

* Optimization: do not generate unreachable calls |peg$fail| and constants for it (local to the rule).
* Optimization: do not generate unreachable calls |peg$fail| and constants for it (non-local to rule).
master
Mingun 6 years ago committed by Futago-za Ryuu
parent a7a0a0d9ac
commit 42ec335d13

@ -1,5 +1,6 @@
"use strict";
const calcReportFailures = require( "./passes/calc-report-failures" );
const generateBytecode = require( "./passes/generate-bytecode" );
const generateJS = require( "./passes/generate-js" );
const removeProxyRules = require( "./passes/remove-proxy-rules" );
@ -56,6 +57,7 @@ const compiler = {
removeProxyRules: removeProxyRules
},
generate: {
calcReportFailures: calcReportFailures,
generateBytecode: generateBytecode,
generateJS: generateJS
}

@ -0,0 +1,68 @@
"use strict";
const asts = require( "../asts" );
const visitor = require( "../visitor" );
// Determines if rule always used in disabled report failure context,
// that means, that any failures, reported within it, are never will be
// visible, so the no need to report it.
function calcReportFailures( ast, options ) {
// By default, not report failures for rules...
ast.rules.forEach( rule => {
rule.reportFailures = false;
} );
// ...but report for start rules, because in that context report failures
// always enabled
const changedRules = options.allowedStartRules.map( name => {
const rule = asts.findRule( ast, name );
rule.reportFailures = true;
return rule;
} );
const calc = visitor.build( {
rule( node ) {
calc( node.expression );
},
// Because all rules already by default marked as not report any failures
// just break AST traversing when we need mark all referenced rules from
// this sub-AST as always not report anything (of course if it not be
// already marked as report failures).
named() {},
rule_ref( node ) {
const rule = asts.findRule( ast, node.name );
// This function only called when rule can report failures. If so, we
// need recalculate all rules that referenced from it. But do not do
// this twice - if rule is already marked, it was in `changedRules`.
if ( ! rule.reportFailures ) {
rule.reportFailures = true;
changedRules.push( rule );
}
}
} );
while ( changedRules.length > 0 ) {
calc( changedRules.pop() );
}
}
module.exports = calcReportFailures;

@ -269,7 +269,8 @@ function generateBytecode( ast ) {
generate( expression, {
sp: context.sp + 1,
env: cloneEnv( context.env ),
action: null
action: null,
reportFailures: context.reportFailures
} ),
[ op.EXPECT_NS_END, negative ? 1 : 0 ],
buildCondition(
@ -327,23 +328,32 @@ function generateBytecode( ast ) {
node.bytecode = generate( node.expression, {
sp: -1, // stack pointer
env: { }, // mapping of label names to stack positions
action: null // action nodes pass themselves to children here
action: null, // action nodes pass themselves to children here
reportFailures: node.reportFailures // if `false`, suppress generation of EXPECT opcodes
} );
},
named( node, context ) {
const nameIndex = addConst(
// Do not generate unused constant, if no need it
const nameIndex = context.reportFailures ? addConst(
`peg$otherExpectation("${ js.stringEscape( node.name ) }")`
);
) : null;
const expressionCode = generate( node.expression, {
sp: context.sp,
env: context.env,
action: context.action,
reportFailures: false
} );
return buildSequence(
// No need to disable report failures if it already disabled
return context.reportFailures ? buildSequence(
[ op.EXPECT, nameIndex ],
[ op.SILENT_FAILS_ON ],
generate( node.expression, context ),
expressionCode,
[ op.SILENT_FAILS_OFF ]
);
) : expressionCode;
},
@ -355,7 +365,8 @@ function generateBytecode( ast ) {
generate( alternatives[ 0 ], {
sp: context.sp,
env: cloneEnv( context.env ),
action: null
action: null,
reportFailures: context.reportFailures
} ),
alternatives.length < 2
? []
@ -382,7 +393,8 @@ function generateBytecode( ast ) {
const expressionCode = generate( node.expression, {
sp: context.sp + ( emitCall ? 1 : 0 ),
env: env,
action: node
action: node,
reportFailures: context.reportFailures
} );
const functionIndex = addFunctionConst( Object.keys( env ), node.code );
@ -416,14 +428,16 @@ function generateBytecode( ast ) {
generate( elements[ 0 ], {
sp: context.sp,
env: context.env,
action: null
action: null,
reportFailures: context.reportFailures
} ),
buildCondition(
[ op.IF_NOT_ERROR ],
buildElementsCode( elements.slice( 1 ), {
sp: context.sp + 1,
env: context.env,
action: context.action
action: context.action,
reportFailures: context.reportFailures
} ),
buildSequence(
processedCount > 1 ? [ op.POP_N, processedCount ] : [ op.POP ],
@ -460,7 +474,8 @@ function generateBytecode( ast ) {
buildElementsCode( node.elements, {
sp: context.sp + 1,
env: context.env,
action: context.action
action: context.action,
reportFailures: context.reportFailures
} )
);
@ -475,7 +490,8 @@ function generateBytecode( ast ) {
return generate( node.expression, {
sp: context.sp,
env: env,
action: null
action: null,
reportFailures: context.reportFailures
} );
},
@ -487,7 +503,8 @@ function generateBytecode( ast ) {
generate( node.expression, {
sp: context.sp + 1,
env: cloneEnv( context.env ),
action: null
action: null,
reportFailures: context.reportFailures
} ),
buildCondition(
[ op.IF_NOT_ERROR ],
@ -516,7 +533,8 @@ function generateBytecode( ast ) {
generate( node.expression, {
sp: context.sp,
env: cloneEnv( context.env ),
action: null
action: null,
reportFailures: context.reportFailures
} ),
buildCondition(
[ op.IF_ERROR ],
@ -532,7 +550,8 @@ function generateBytecode( ast ) {
const expressionCode = generate( node.expression, {
sp: context.sp + 1,
env: cloneEnv( context.env ),
action: null
action: null,
reportFailures: context.reportFailures
} );
return buildSequence(
@ -549,7 +568,8 @@ function generateBytecode( ast ) {
const expressionCode = generate( node.expression, {
sp: context.sp + 1,
env: cloneEnv( context.env ),
action: null
action: null,
reportFailures: context.reportFailures
} );
return buildSequence(
@ -569,7 +589,8 @@ function generateBytecode( ast ) {
return generate( node.expression, {
sp: context.sp,
env: cloneEnv( context.env ),
action: null
action: null,
reportFailures: context.reportFailures
} );
},
@ -592,25 +613,26 @@ function generateBytecode( ast ) {
},
literal( node ) {
literal( node, context ) {
if ( node.value.length > 0 ) {
const stringIndex = addConst( `"${ js.stringEscape(
node.ignoreCase ? node.value.toLowerCase() : node.value
) }"` );
const expectedIndex = addConst(
// Do not generate unused constant, if no need it
const expectedIndex = context.reportFailures ? addConst(
"peg$literalExpectation("
+ `"${ js.stringEscape( node.value ) }", `
+ node.ignoreCase
+ ")"
);
) : null;
// For case-sensitive strings the value must match the beginning of the
// remaining input exactly. As a result, we can use |ACCEPT_STRING| and
// save one |substr| call that would be needed if we used |ACCEPT_N|.
return buildSequence(
[ op.EXPECT, expectedIndex ],
context.reportFailures ? [ op.EXPECT, expectedIndex ] : [],
buildCondition(
node.ignoreCase
? [ op.MATCH_STRING_IC, stringIndex ]
@ -629,7 +651,7 @@ function generateBytecode( ast ) {
},
class( node ) {
class( node, context ) {
const regexp = "/^["
+ ( node.inverted ? "^" : "" )
@ -656,16 +678,17 @@ function generateBytecode( ast ) {
+ "]";
const regexpIndex = addConst( regexp );
const expectedIndex = addConst(
// Do not generate unused constant, if no need it
const expectedIndex = context.reportFailures ? addConst(
"peg$classExpectation("
+ parts + ", "
+ node.inverted + ", "
+ node.ignoreCase
+ ")"
);
) : null;
return buildSequence(
[ op.EXPECT, expectedIndex ],
context.reportFailures ? [ op.EXPECT, expectedIndex ] : [],
buildCondition(
[ op.MATCH_REGEXP, regexpIndex ],
[ op.ACCEPT_N, 1 ],
@ -675,12 +698,15 @@ function generateBytecode( ast ) {
},
any() {
any( node, context ) {
const expectedIndex = addConst( "peg$anyExpectation()" );
// Do not generate unused constant, if no need it
const expectedIndex = context.reportFailures
? addConst( "peg$anyExpectation()" )
: null;
return buildSequence(
[ op.EXPECT, expectedIndex ],
context.reportFailures ? [ op.EXPECT, expectedIndex ] : [],
buildCondition(
[ op.MATCH_ANY ],
[ op.ACCEPT_N, 1 ],

File diff suppressed because one or more lines are too long

@ -94,6 +94,10 @@ declare namespace peg {
bytecode?: number[];
// Added by calc-report-failures pass
reportFailures?: boolean;
}
interface Named extends INode {
@ -340,6 +344,7 @@ declare namespace peg {
namespace transform {
function removeProxyRules( ast: Grammar, options: ICompilerPassOptions ): void;
function calcReportFailures( ast: Grammar, options: ICompilerPassOptions ): void;
}

@ -75,14 +75,30 @@ describe( "compiler pass |generateBytecode|", function () {
describe( "for named", function () {
const grammar = "start 'start' = 'a'";
const grammar1 = "start 'start' = .";
const grammar2 = "start 'start' = 'a'";
const grammar3 = "start 'start' = [a]";
describe( "when |reportFailures=true|", function () {
it( "generates correct bytecode", function () {
expect( pass ).to.changeAST( grammar, bytecodeDetails( [
expect( pass ).to.changeAST( grammar1, bytecodeDetails( [
23, 0, // EXPECT <0>
28, // SILENT_FAILS_ON
17, 2, 1, 21, 1, 3, // <expression>
29 // SILENT_FAILS_OFF
] ) );
expect( pass ).to.changeAST( grammar2, bytecodeDetails( [
23, 0, // EXPECT <0>
28, // SILENT_FAILS_ON
23, 2, 18, 1, 2, 1, 22, 1, 3, // <expression>
18, 1, 2, 1, 22, 1, 3, // <expression>
29 // SILENT_FAILS_OFF
] ) );
expect( pass ).to.changeAST( grammar3, bytecodeDetails( [
23, 0, // EXPECT <0>
28, // SILENT_FAILS_ON
20, 1, 2, 1, 21, 1, 3, // <expression>
29 // SILENT_FAILS_OFF
] ) );
@ -90,16 +106,54 @@ describe( "compiler pass |generateBytecode|", function () {
it( "defines correct constants", function () {
expect( pass ).to.changeAST( grammar, constsDetails( [
expect( pass ).to.changeAST( grammar1, constsDetails( [
"peg$otherExpectation(\"start\")"
] ) );
expect( pass ).to.changeAST( grammar2, constsDetails( [
"peg$otherExpectation(\"start\")",
"\"a\"",
"peg$literalExpectation(\"a\", false)"
"\"a\""
] ) );
expect( pass ).to.changeAST( grammar3, constsDetails( [
"peg$otherExpectation(\"start\")",
"/^[a]/"
] ) );
} );
} );
describe( "when |reportFailures=false|", function () {
it( "generates correct bytecode", function () {
expect( pass ).to.changeAST( grammar1, bytecodeDetails( [
17, 2, 1, 21, 1, 3, // <expression>
] ), {}, { reportFailures: false } );
expect( pass ).to.changeAST( grammar2, bytecodeDetails( [
18, 0, 2, 1, 22, 0, 3, // <expression>
] ), {}, { reportFailures: false } );
expect( pass ).to.changeAST( grammar3, bytecodeDetails( [
20, 0, 2, 1, 21, 1, 3, // <expression>
] ), {}, { reportFailures: false } );
} );
it( "defines correct constants", function () {
expect( pass ).to.changeAST( grammar1, constsDetails( [] ), {}, { reportFailures: false } );
expect( pass ).to.changeAST( grammar2, constsDetails( [
"\"a\""
] ), {}, { reportFailures: false } );
expect( pass ).to.changeAST( grammar3, constsDetails( [
"/^[a]/"
] ), {}, { reportFailures: false } );
} );
} );
} );
describe( "for choice", function () {
it( "generates correct bytecode", function () {
@ -452,14 +506,25 @@ describe( "compiler pass |generateBytecode|", function () {
describe( "for group", function () {
const grammar = "start = ('a')";
it( "generates correct bytecode", function () {
expect( pass ).to.changeAST( "start = ('a')", bytecodeDetails( [
expect( pass ).to.changeAST( grammar, bytecodeDetails( [
23, 1, 18, 0, 2, 1, 22, 0, 3 // <expression>
] ) );
} );
it( "defines correct constants", function () {
expect( pass ).to.changeAST( grammar, constsDetails( [
"\"a\"",
"peg$literalExpectation(\"a\", false)"
] ) );
} );
} );
describe( "for semantic_and", function () {
@ -662,6 +727,8 @@ describe( "compiler pass |generateBytecode|", function () {
describe( "for literal", function () {
describe( "when |reportFailures=true|", function () {
describe( "empty", function () {
const grammar = "start = ''";
@ -736,8 +803,84 @@ describe( "compiler pass |generateBytecode|", function () {
} );
describe( "when |reportFailures=false|", function () {
describe( "empty", function () {
const grammar = "start = ''";
it( "generates correct bytecode", function () {
expect( pass ).to.changeAST( grammar, bytecodeDetails( [
0, 0 // PUSH
] ), {}, { reportFailures: false } );
} );
it( "defines correct constants", function () {
expect( pass ).to.changeAST( grammar, constsDetails( [ "\"\"" ] ), {}, { reportFailures: false } );
} );
} );
describe( "non-empty case-sensitive", function () {
const grammar = "start = 'a'";
it( "generates correct bytecode", function () {
expect( pass ).to.changeAST( grammar, bytecodeDetails( [
18, 0, 2, 1, // MATCH_STRING <0>
22, 0, // * ACCEPT_STRING
3 // * PUSH_FAILED
] ), {}, { reportFailures: false } );
} );
it( "defines correct constants", function () {
expect( pass ).to.changeAST( grammar, constsDetails( [
"\"a\""
] ), {}, { reportFailures: false } );
} );
} );
describe( "non-empty case-insensitive", function () {
const grammar = "start = 'A'i";
it( "generates correct bytecode", function () {
expect( pass ).to.changeAST( grammar, bytecodeDetails( [
19, 0, 2, 1, // MATCH_STRING_IC <0>
21, 1, // * ACCEPT_N
3 // * PUSH_FAILED
] ), {}, { reportFailures: false } );
} );
it( "defines correct constants", function () {
expect( pass ).to.changeAST( grammar, constsDetails( [
"\"a\""
] ), {}, { reportFailures: false } );
} );
} );
} );
} );
describe( "for class", function () {
describe( "when |reportFailures=true|", function () {
it( "generates correct bytecode", function () {
expect( pass ).to.changeAST( "start = [a]", bytecodeDetails( [
@ -803,8 +946,74 @@ describe( "compiler pass |generateBytecode|", function () {
} );
describe( "when |reportFailures=false|", function () {
it( "generates correct bytecode", function () {
expect( pass ).to.changeAST( "start = [a]", bytecodeDetails( [
20, 0, 2, 1, // MATCH_REGEXP <0>
21, 1, // * ACCEPT_N
3 // * PUSH_FAILED
] ), {}, { reportFailures: false } );
} );
describe( "non-inverted case-sensitive", function () {
it( "defines correct constants", function () {
expect( pass ).to.changeAST( "start = [a]", constsDetails( [
"/^[a]/"
] ), {}, { reportFailures: false } );
} );
} );
describe( "inverted case-sensitive", function () {
it( "defines correct constants", function () {
expect( pass ).to.changeAST( "start = [^a]", constsDetails( [
"/^[^a]/"
] ), {}, { reportFailures: false } );
} );
} );
describe( "non-inverted case-insensitive", function () {
it( "defines correct constants", function () {
expect( pass ).to.changeAST( "start = [a]i", constsDetails( [
"/^[a]/i"
] ), {}, { reportFailures: false } );
} );
} );
describe( "complex", function () {
it( "defines correct constants", function () {
expect( pass ).to.changeAST( "start = [ab-def-hij-l]", constsDetails( [
"/^[ab-def-hij-l]/"
] ), {}, { reportFailures: false } );
} );
} );
} );
} );
describe( "for any", function () {
describe( "when |reportFailures=true|", function () {
const grammar = "start = .";
it( "generates bytecode", function () {
@ -829,4 +1038,33 @@ describe( "compiler pass |generateBytecode|", function () {
} );
describe( "when |reportFailures=false|", function () {
const grammar = "start = .";
it( "generates bytecode", function () {
expect( pass ).to.changeAST( grammar, bytecodeDetails( [
17, 2, 1, // MATCH_ANY
21, 1, // * ACCEPT_N
3 // * PUSH_FAILED
] ), {}, { reportFailures: false } );
} );
it( "defines correct constants", function () {
expect( pass ).to.changeAST(
grammar,
constsDetails( [] ),
{},
{ reportFailures: false }
);
} );
} );
} );
} );

@ -6,9 +6,10 @@ module.exports = function ( chai, utils ) {
const Assertion = chai.Assertion;
Assertion.addMethod( "changeAST", function ( grammar, props, options ) {
Assertion.addMethod( "changeAST", function ( grammar, props, options, additionalRuleProps ) {
options = typeof options !== "undefined" ? options : {};
additionalRuleProps = typeof additionalRuleProps !== "undefined" ? additionalRuleProps : { reportFailures: true };
function matchProps( value, props ) {
@ -63,11 +64,13 @@ module.exports = function ( chai, utils ) {
}
ast.rules = ast.rules.map( rule => Object.assign( rule, additionalRuleProps ) );
utils.flag( this, "object" )( ast, options );
this.assert(
matchProps( ast, props ),
"expected #{this} to change the AST to match #{exp}",
"expected #{this} to change the AST to match #{exp} but #{act} was produced",
"expected #{this} to not change the AST to match #{exp}",
props,
ast

Loading…
Cancel
Save