Skip to content

Report the syntax error instead of dying on the half-built tree (#813) - #824

Merged
Rafael-SOWNet merged 1 commit into
masterfrom
fix/provided-comma-parser
Aug 8, 2026
Merged

Rafael-SOWNet merged 1 commit into
masterfrom
fix/provided-comma-parser

Conversation

@Rafael-SOWNet

Copy link
Copy Markdown
Member

Closes #813.

What was wrong

ANTLR reports a syntax error to the listener and then recovers, carrying on with a rule context whose value was never assigned. The grammar's action for provided runs anyway:

_localctx.value = _localctx.value.Provided(_localctx.pred.value);   // AngouriMathParser.cs:1549

so the parse came down with a raw NullReferenceException before ParseSilent ever read the error already sitting in writer.errors.

That last part is the whole fix. The error was detected the entire time; the parser just died before anyone looked at it.

try { parser.Parse(); }
catch (Exception) when (writer.errors.Count > 0)
{
    return new Failure<ReasonWhyParsingFailed>(writer.errors[0]);
}

Reading the recorded error is the honest answer — the input really was invalid, and a caller is owed an exception under AngouriMathBaseException, as Docs/Usage/Exceptions.md promises. The guard is what makes this safe rather than a blanket swallow: where the parser throws with nothing recorded against the input, the exception is ours and it keeps propagating. A bug of ours cannot be laundered into a syntax error.

Measured

Each of these was a NullReferenceException on 61de353b:

input now
(1 provided x > 0, 2) UnhandledParseException: line 1:17 no viable alternative
(a provided b, c) line 1:13
(1 provided x > 0, 2, 3) line 1:17
(1, 2 provided x > 0) line 1:2
(1 provided x > 0, 2 provided x < 0) line 1:17

The last two are wider than the issue reported: a provided after the comma crashes too, so the trigger is a provided anywhere in a parenthesised comma list, not only one followed by a comma.

A correction to the issue

#813 says the piecewise forms from #326 "do not parse either". That is wrong, and I wrote it. All three documented shapes parse:

piecewise(1 provided x > 0, 2)                     ->  piecewise(1 provided (x > 0), 2 provided True)
piecewise(1 provided x > 0, 2 provided x < 0)      ->  parses
piecewise(1 provided x > 0, 2 provided x < 0, 3)   ->  piecewise(..., 3 provided True)

Only the capitalised Piecewise( fails, as an unknown name followed by a bracket — which is #733, not this. I tested with the capitalisation used in #326's prose and drew the wrong conclusion from it.

So the grammar does have a rule for the comma list of provided; what has no rule is that list without a piecewise in front of it, and that is exactly what the crash was hiding. The tests now pin the working forms so the fix cannot be mistaken for refusing the construct.

Tests

5828 passing, 0 failed, 14 skipped. F# 130 passing.

Three groups added to FromStringTest: the five crashing inputs with their error positions; the provided and piecewise forms that must keep parsing; and a broader check that a malformed input is refused by this library's own exception rather than a framework one — which is the property the issue is actually about, and which nothing was asserting.

Not in scope

What the piecewise syntax should be is #326, still open and marked Opinions wanted. Fixing the crash did not need that settled, and this does not settle it.

🤖 Generated with Claude Code

ANTLR reports a syntax error to the listener and then recovers, carrying on with
a rule context whose value was never assigned. The grammar's action for provided
runs anyway and dereferences it, so the parse came down with a raw
NullReferenceException before ParseSilent ever read the error already sitting in
writer.errors.

The error was detected the whole time. Reading it is the honest answer: the input
really was invalid, and a caller is owed an exception under AngouriMathBaseException
as Docs/Usage/Exceptions.md promises -- not whatever the half-built tree threw.

Guarded on an error having been reported, so this cannot swallow a bug of ours:
where the parser throws with nothing recorded against the input, the exception is
ours and it keeps propagating.

Measured on this build; each of these was a NullReferenceException:

  (1 provided x > 0, 2)                  line 1:17 no viable alternative
  (a provided b, c)                      line 1:13
  (1 provided x > 0, 2, 3)               line 1:17
  (1, 2 provided x > 0)                  line 1:2
  (1 provided x > 0, 2 provided x < 0)   line 1:17

The last two are wider than the issue reported: a provided *after* the comma
crashes too, so the trigger is a provided anywhere in a parenthesised comma list.

The issue also said the piecewise forms from #326 do not parse. That is wrong and
the tests now pin the opposite: piecewise(1 provided x > 0, 2 provided x < 0, 3)
parses, and so do the other two documented shapes. Only the capitalised
Piecewise( fails, as an unknown name followed by a bracket, which is #733. So the
comma list has a rule after all -- what has none is the list without a piecewise
in front of it, and that is what the crash was hiding.

Tests: 5828 passing, 0 failed, 14 skipped; F# 130.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Rafael-SOWNet
Rafael-SOWNet merged commit e8dab66 into master Aug 8, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A 'provided' inside parentheses followed by a comma throws NullReferenceException from the parser

1 participant