Skip to content

getRightmostFailures() returns 0 failures when all rules are described #544

@pdubroy

Description

@pdubroy

I definitely believe there is an outright bug with error reporting for rules with descriptions. Consider the following simple variant of the first test in packages/ohm-js/test/test-errors.js:

test('described match failure', t => {
  const g = ohm.grammar('G { start (an abcd) = "a" "b" "c" "d" }');
  const e = g.match('ab');
  t.is(e.failed(), true);
  t.is(e.getRightmostFailurePosition(), 0);
  const f = e.getRightmostFailures();
  t.is(f.length, 1)
});

This test fails, at the last t.is(f.length, 1) check; the actual length is 0. So the rightmost failure position must in fact be 0, which makes sense, since ohm tries to match the rule-with-description start at position 0, and that match fails. So there should be a failure at position 0 (which I would think would turn into a message like "Expected an abcd") but in fact there is no failure.

However, if in MatchResult.getRightmostFailures, we force it to forget the memo table like so:

  getRightmostFailures() {
    if (!this._rightmostFailures) {
      this.matcher._resetMemoTable(); // ADDED
      this.matcher.setInput(this.input);
      const matchResultWithFailures = this.matcher._match(this.startExpr, {
        tracing: false,
        positionToRecordFailures: this.getRightmostFailurePosition(),
      });
      this._rightmostFailures = matchResultWithFailures.getRightmostFailures();
    }
    return this._rightmostFailures;
  }

then the "described match failure" test passes (!) and indeed the one Failure object has text: 'an abcd'.

I am not advocating forgetting the memo table to extract the failures, but here's what's happening: the call g.match('ab') is setting the rightmostFailurePosition to 0, and since we didn't know that and so have positionToRecordFailures undefined for that match, the _rightmostFailures is coming back undefined. So to extract those failures, getRightmostFailures is naturally setting 0 as the positionToRecordFailures. However, in the re-match (when the memo table is not forgotten), the rightmostFailurePosition is coming out to 2 (the position where the "c" in the rule fails to match) and there never is a failure at position 0, and so again no failures are recorded.

So there appears to be a bug when re-matching with the memo table, in the sense that match failures are no longer handled identically. My presumption is that with the memo table, the parse is supposed to proceed absolutely identically as when the parse is started without a memo table, just faster, including failing in an identical way.

Would you agree that the behavior as detailed here constitutes a bug? If so, should I file a separate specific report, or just leave this concern here as part of this issue? I'm inclined to move it to a separate issue, because fixing this specific concern won't have any effect on the originally posted concern, which was that naming a rule may end up hiding that it was actually able to partially match up to a much later point in the input than other alternatives at the position where it started, including the alternative of itself succeeding on a prefix of that partial match (potentially leading to a misleading failure message). But I wanted to ask first so as not to clutter the issue list if you prefer it's just handled here.

Originally posted by @gwhitney in #302

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions