asc-community / asc-community/AngouriMath

Should the parser stay ANTLR-generated? (intent of #565, needs-design)

Open
#898 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
831
Forks
79
Avg merge
3h 23m
Merged PRs (30d)
309

Description

Splitting the intent of [#565](https://github.com/asc-community/AngouriMath/pull/565) out of the branch, which is being closed: it proposed replacing the ANTLR-generated parser with one built on [Yoakke](https://github.com/LanguageDev/Yoakke), and the question it raises is live even though the branch is not.

## What the branch did

Deleted the whole of `Core/Antlr` — the grammar, the four generated files, the `.interp`/`.tokens`
artefacts and the bundled `antlr-4.8-complete.jar` — and replaced it with a hand-assembled parser:
**+740 / −7185 across 26 files**.

## Why ANTLR is worth replacing

Not on parser-theory grounds. On the day-to-day cost of it:

- **The generated files are committed**, so a grammar change is a two-step ritual — edit
`AngouriMath.g`, run `antlr_rerun.bat`, then run a post-processing pass whose only job is to rewrite
`public` to `internal` on the generated classes. `Docs/Contributing/ImproveParser.md` documents it,
and the documentation has to warn you to regenerate the *unmodified* grammar first and check the diff
is empty, so that a toolchain version difference is not mistaken for your change.
- **It needs a JDK** to change the grammar at all, in a repository that otherwise needs only the .NET
SDK.
- **A 1 MB jar is in the source tree.**
- **The error messages are ANTLR's**, which is why `MissingOperatorParseException` and friends exist to
translate them, and why `Docs/Usage/Syntax.md` had to be written by hand as a separate statement of
what the parser accepts.

## Why it is not obviously worth doing

- **The parser works, and its contract is now tested.** `StringizeRoundTripTest` holds printing to being
parsing's inverse across every node type, and 2.0 fixed several node shapes that did not round-trip.
A rewrite starts that guarantee from zero.
- **The grammar is the specification.** `Syntax.md` describes it, but `AngouriMath.g` is the thing that
decides, and it is 480-odd lines of readable declarative rules. A hand-written recursive-descent
parser is more code and less obviously equivalent to a reader.
- **Yoakke is itself a dependency**, and a small one — it would be trading a build-time dependency on a
jar for a run-time dependency on a young library, which for a package with 313k downloads is a
different kind of risk rather than less risk.
- **Nothing about the parser is currently a bug.** The open parse issues are about what the grammar
*says* — precedence, notation — not about how it is produced.

## What would make this decidable

1. **Is the round trip preserved?** `StringizeRoundTripTest` over every node type is the acceptance, and
it did not exist when #565 was written.
2. **Are the error messages better?** The reason to hand-write a parser is control over failure; if the
replacement's messages are no better than the translated ANTLR ones, the main benefit is only the
build simplification.
3. **What does it cost at run time?** Parsing is on the hot path for `FromString`, which caches by
string precisely because it is not free.
4. **Does `Syntax.md` stay true, and can it be generated from the new parser** rather than maintained
beside it?

I have no recommendation. The four costs above are real and so are the four objections, and this is a
maintainer's call about what the library wants to own. Filed so that the reasoning survives the branch
rather than being rediscovered in another four years.

Contributor guide

Open the contributing guide

Research direction

Start by reading Core/Antlr, AngouriMath.g, Docs/Contributing/ImproveParser.md, and StringizeRoundTripTest to understand the current parser and its acceptance criteria. Any proposed replacement should be evaluated for round-trip preservation, error messages, runtime cost, and whether Docs/Usage/Syntax.md remains accurate; the issue is done when maintainers have a decision supported by those checks.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
compilers
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.