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

Enable testing of grammars with Antlr4ng #4019

Merged
merged 6 commits into from Mar 25, 2024
Merged

Conversation

kaby76
Copy link
Contributor

@kaby76 kaby76 commented Mar 17, 2024

This PR updates the grammars and build to test using the Typescript tool and runtime Antlr4ng for certain Antlr4 grammars. The target Antlr4ng is added to the grammar's desc.xml where the target works.

Antlr4ng is @mike-lischke 's updated TypeScript target, improving on many issues with the Antlr4 TypeScript target version 4.13.1. The reason it is added is because there will likely not be any further releases of Antlr4. In fact, for grammars that have TypeScript target-specific support code, Antlr 4.13.1 TypeScript runtime does not work with newer versions of tsc/Node because the generated parser and lexer cannot run (it cannot find the compiled base class files). The only hope is to use Antlr4ng.

This PR made the following changes.

  • Installed Node 21.7.1 for all OSes for the CI workflow. This is required to run on Mac (dyld[15293]: Library not loaded: '/usr/local/opt/icu4c/lib/libicui18n.73.dylib'). In addition, I saw a performance gain with the newer node, but I don't see the gain here in the Github Actions.
  • Added Antlr4ng templates for trgen. These templates mirror what is done with the Antlr 4.13.1 TypeScript templates, except that here we need to use the Antlr4ng-specific tool and runtime. I have the package.json use specifically version 3.0.4 of the runtime and 2.0.0 of the tool. Note, the package-lock.json file is NOT checked in because: (1) it is generated from a specification: package.json; (2) when I build a TS application, the package-lock.json gets updated. I do NOT want to check this change as a side-effect from a build. There have been many "answers" on SO that insist to check in package-lock.json. I emphatically reject this idea! Package.json is a specification; package-lock.json is an implementation. Fortunately, in the world of C#, there is no such stupid concept with .csproj's because every one knows you don't check in generated files.
  • Updated the CSharp targets with net8.0. I already updated the Dotnet tool chain with version 8 here. This PR completes the switchover by modifying the trgen templates to use net8.
  • Fixed a problem with "clean.sh" in templates. The Antlr .jar is used to tell what files are generated and what should be removed. Note, this script is never used in testing. The change is not perfect because I hardwire the name of the Antlr tool, so I will still need to figure out a more permanent solution.
  • Added in Antlr4ng into <targets> sections in desc.xml. This tells trgen that the grammar works for Antlr4ng. Otherwise, the grammar DOES NOT work for the target. No split grammars with a base class are working yet. The assumption is that it should work. If not, we can ask Mike to make a fix and cut an immediate release. Mike is a much more responsive partner in fixing bugs.

@kaby76 kaby76 marked this pull request as ready for review March 18, 2024 06:34
@teverett
Copy link
Member

@kaby76 thanks!

@teverett teverett merged commit 4a54822 into antlr:master Mar 25, 2024
33 checks passed
kaby76 added a commit to kaby76/grammars-v4 that referenced this pull request Mar 31, 2024
This is exactly why I added antlr#4019 but it looks like I missed removing the target from this grammar.
teverett pushed a commit that referenced this pull request Apr 1, 2024
* Fix for #4035 -- add Antlr4ng port for cpp grammar.

* The TypeScript target does not work.

This is exactly why I added #4019 but it looks like I missed removing the target from this grammar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants