Issue 2693

[kaby76]

This is a fix for #2693. This is a new PR, replacing #2936 (coincidence that it's similarly named to the issue).

This change adds in code that I ported from the Java runtime for profiling. The problem was that sometimes setting parser.Profile = true; before a parse would cause a null-pointer exception in the C# runtime. This change alters a number of the method signatures in the runtime. Note that in the Java runtime the constructor LookaheadEventInfo() contains parameter predictedAlt that is undocumented in the comments. I added a stub to avoid complaints from Docfx.

The test for this change was not easy to add because the parser constructed from template did not have code to set parser.Profile = true;. To minimize changes, I added method BaseCSharpTest.execParser() to take an additional prarmeter, and subclassed BaseProfileCSharpTest from BaseCSharpTest to generate code for profiling. The code is generated to "BaseProfile....." in /tmp. I noticed the indentation off in the template, so I fixed that. There is now a new test "descriptor", ProfileDescriptors.java, which contains code to try two different cases for profiling, currently only for C#. The "FatProfile" is an example that I experimentally determined by going through all the grammars and examples in antlr/grammars-v4, choosing asm/asm8080 because it was small (the Java9 grammar is large and also requires a split grammar to make it portable to C#). I created a descriptor with the inlined asm8080 grammar, tightened up considerably--people love too much spacing, a separate line per alt, yuck! The output from the driver program is experimentally determined. The runtime no longer crashes, and the output looks fine.

I would like, however, to note that the runtime test suite code tries to do too much under Java. Testing should be under Bash, and a separate program constructed to generate the driver programs from templates. This can then be used outside of Java, outside of the antlr4/ repo. I've already started writing said program, Antlr4BuildTasks/dotnet-antlr, which is platform-independent, and outputs drivers for Java and C# (both Antlr4.Runtime.Standard and Harwell's Antlr4, any runtime version).

[ericvergnaud]

Hi,

thanks for this.

I think we will stick to junit for grammar tests, because the ability to ensure the exact same test (grammar, generation, input output and error) runs in all target languages is paramount, and very convenient to action from IntelliJ.

This only applies to tests that involve code generation. The runtime target toolkit tests do not need to sit with the Java code base, and you are welcome to move the profiler test out, and into the C# code base. Other runtime targets already do this (Swift and Python I think).

[kaby76]

This only applies to tests that involve code generation. The runtime target toolkit tests do not need to sit with the Java code base, and you are welcome to move the profiler test out, and into the C# code base. Other runtime targets already do this (Swift and Python I think).

I don't understand how the profile interpreter does not depend on the code generation of the tool. The profile test requires the Parser, Lexer data structures generated by the Antlr Java Tool for a .g4 example that did crash prior to the fix (asm8080.g4 or Java9.g4). I can't magically create this source code out of thin air. Please explain.

What you are suggesting, I guess, is to reorganize the code in antlr4/runtime/CSharp/. The proposal is to create two new directories: antlr4/runtime/CSharp/src/ (to contains the current source code in antlr4/runtime/CSharp/) and antlr4/runtime/CSharp/test/ sub-directory to contain sub-projects (.csproj) for tests that "do not depend on code generation". I.e.,

CSharp
|
---- src
    |
    - *.cs, .csproj of the C# runtime
|
---- test
    |
    - profile
    |
    ---- Program.cs, Test.csproj, asm8080.g4, input.asm for testing of Profile = true.

In other words, how every other runtime is currently organized. Why wasn't CSharp/ set up this way to begin with??

[ericvergnaud]

The grammar tests serve a very different purpose from the profiler test:

• if a change is required in how the tool generates levers or parsers, then there is a need to test that against all targets
• whereas the profiler is not tool or grammar dependent, the test is target specific (and could rely on a pre-generated lexer and parser, like Python does)

Since I did not write the original code, I can't comment on why the CSharp structure is different from other targets.

If you are willing to invest the effort, I am supportive of aligning it.

[ericvergnaud]

Also suggest you rebase against latest master, such that circle-ci tests can be executed.

[kaby76]

Thanks Eric.

That seems to be a reasonable demarcation between tests that belong in runtime-testsuite/ vs runtime/. I will reorganize the CSharp/ files as per my description, and move the profile test there. Obviously, that will require any references to Antlr4.csproj to also be modified (find . -type f -exec grep 'Antlr4.csproj' -l '{}' ';').

The code for Dart and Swift should be reorganized similarly. After all, I modeled my changes to runtime-testsuite/ after these two runtimes. Note, I won't change the Dart or Swift code because I can't get "mvn install" to work from runtime-testsuite/. I don't know what build environment is required. Even after sudo apt install npm, mvn install still says "mvn install" cannot find npm. Terrible. The instructions for setting up one's environment need to be improved, but perhaps I can figure it out because I now see scripts in https://github.com/antlr/antlr4/tree/master/.circleci/scripts that will point me in the way as they contain sudo installs.

I will be creating an entirely new branch and PR for the changes because you merged it with "master", and I have other changes. (On second thought, I'll just merge from master and continue the changes.)

I don't understand why isn't Circle-CI working.

#!/bin/sh -eo pipefail
# No configuration was found in your project. Please refer to https://circleci.com/docs/2.0/ to get started with your configuration.
# 
# -------
# Warning: This configuration was auto-generated to show you the message above.
# Don't rerun this job. Rerunning will have no effect.
false

Exited with code exit status 1
CircleCI received exit code 1

I guess I have to sign up with Circleci.com and try a "Hello World" before submitting the new PR for Antlr4.

[ericvergnaud]

circle-ci isn't working because your branch was created before it was enabled
so it does not contain a .circle-ci folder
that's why you need to rebase