Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug with named NEVER_MATCH expressions #454

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@ Released: TBD

### Minor Changes

- [#454](https://github.com/peggyjs/peggy/pull/454) Fix a bug with named NEVER_MATCH expressions
- [#453](https://github.com/peggyjs/peggy/pull/453) Make generate-bytecode.js ts-clean
- [#452](https://github.com/peggyjs/peggy/pull/452) Fixes to prepare generate-bytecode.js for ts-check
- [#432](https://github.com/peggyjs/peggy/pull/432) Add peggy.code-workspace
2 changes: 1 addition & 1 deletion docs/js/benchmark-bundle.min.js

Large diffs are not rendered by default.

3 changes: 0 additions & 3 deletions docs/js/examples.js
Original file line number Diff line number Diff line change
@@ -204,7 +204,6 @@ function peg$parse(input, options) {
var peg$e11 = peg$literalExpectation(" ", false);
var peg$e12 = peg$literalExpectation("c", false);
var peg$e13 = peg$classExpectation([["a", "c"]], false, false);
var peg$e14 = peg$otherExpectation("The rest of the input");

var peg$f0 = function(match, rest) { return {match, rest}; };
var peg$f1 = function(match, rest) { return {match, rest}; };
@@ -1488,8 +1487,6 @@ function peg$parse(input, options) {
}
s0 = input.substring(s0, peg$currPos);
peg$silentFails--;
s1 = peg$FAILED;
if (peg$silentFails === 0) { peg$fail(peg$e14); }

return s0;
}
2 changes: 1 addition & 1 deletion docs/js/test-bundle.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/vendor/peggy/peggy.min.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions lib/compiler/passes/generate-bytecode.js
Original file line number Diff line number Diff line change
@@ -715,8 +715,8 @@ function generateBytecode(ast, options) {

named(node, context) {
const match = node.match || 0;
// Expectation not required if node always fail
const nameIndex = (match === NEVER_MATCH)
// Expectation not required if node always match
const nameIndex = (match === ALWAYS_MATCH)
? -1
: addExpectedConst({ type: "rule", value: node.name });

@@ -728,7 +728,7 @@ function generateBytecode(ast, options) {
[op.SILENT_FAILS_ON],
generate(node.expression, context),
[op.SILENT_FAILS_OFF],
buildCondition(match, [op.IF_ERROR], [op.FAIL, nameIndex], [])
buildCondition(-match, [op.IF_ERROR], [op.FAIL, nameIndex], [])
);
Copy link
Member

@Mingun Mingun Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! The bug probably was introduced during rebase while adapt initial PR to the new code after another merged PR.

Actually, the whole buildSequence could be replaced by [op.FAIL, nameIndex] if node always fail and there are no actions or semantic predicates (because they can have side effects), but anyway, it is better to do in a separate pass.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if node always fail and there are no actions or semantic predicates (because they can have side effects)

There are already comments about adding side-effect analysis so we can do that kind of thing...

},

4 changes: 2 additions & 2 deletions test/behavior/generated-parser-behavior.spec.js
Original file line number Diff line number Diff line change
@@ -215,11 +215,11 @@ describe("generated parser behavior", () => {
});
});

it("reports match failure and doesn't records any expectations when expression never match", () => {
it("reports match failure and records an expectation of type \"other\" when expression never match", () => {
const parser = peg.generate("start 'start' = []", options);

expect(parser).to.failToParse("b", {
expected: [],
expected: [{ type:"other", description: "start" }],
});
});