Java Backend: Add line number support.

[guardbotmk3]

Use complex symbol for automatic context with syntax errors.

[guardbotmk3]

Consider this a first pass. If there's something that doesn't match the project's style, or you'd like to see re-worked, let me know.

[andreasabel]

Thanks for the PR and your contribution to BNFC.

I took your PR as opportunity to put out some contribution guidelines, see README at https://github.com/BNFC/bnfc, section Contribute.

I am a bit confused about the title Java Backend: Add line number support. Looking at the existing code, there seems already to be line number support for the java backend. Maybe you meant column number?

• Please add tests to the https://github.com/BNFC/bnfc/tree/master/testing infrastructure that document the new behavior.
• Please document your changes in the code and the changelog.
• From the style guide: Document function arguments, avoid Bool.

Looking forward to the updated PR!

[guardbotmk3]

I am a bit confused about the title Java Backend: Add line number support. Looking at the existing code, there seems already to be line number support for the java backend. Maybe you meant column number?

No. The line number support currently extends only as far the lexer. The symbols created for the parser have no line information, so syntax errors have no context info to find them.

ComplexSymbol is necessary because the standard JLex Symbol class only keeps track of character position, oddly enough.

Expect an eventual followup to record the line info in the generated abstract syntax tree.

[guardbotmk3]

Added tests to the testsuite, and verified they pass for jlex and jflex, both with and without line numbers.

[andreasabel]

Thanks for the quick response!

Will you update this PR or make a new one, with the changes you outlined above?

[guardbotmk3]

It's already updated.

Edit: I realized you may have meant adding line info the Abstract Syntax Tree. I'll do that in a separate PR, but the other changes are already in this one. On Dec 13, 2017 12:20 AM, "Andreas Abel" notifications@github.com wrote:

Thanks for the quick response!

Will you update this PR or make a new one, with the changes you outlined above?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BNFC/bnfc/pull/217#issuecomment-351317281, or mute the thread https://github.com/notifications/unsubscribe-auth/AgRziCtUftUy-R9wtQ5fK-4MBIk0MAeJks5s_4jjgaJpZM4Q-IuU .

[andreasabel]

Thanks for the update. Almost there, can we make the title a bit more precise? "Java Backend: Add line number support." suggests that there was no line number support even though the help text says

Special options for the Java backend
  -l                        Add and set line_number field for all syntax classes

The new text should make clear

• what did not work about the existing -l option
• what works now
• what does not work yet.

Please write this up in a paragraph or so and put it into Java.hs, and then update the title for more precision.

Thanks for your contribution so far!

[guardbotmk3]

I added the additional documentation to the commit message. Java.hs didn't seem like the right place for it. The short answer as to what didn't work is "any of it". If you look at the existing code, nothing in the Java Backend even checks Options.linenumbers. The commit also clarifies what is working post commit, and what still needs to be done.

[andreasabel]

Thanks for the good work!

[andreasabel]

Er, the testsuite does not pass. I am getting errors like:

C4/Yylex.java:299: error: incompatible types: int cannot be converted to String
    return new ComplexSymbolFactory.Location(yyline+1, yycolumn+1, yychar);
                                                   ^
C4/Yylex.java:303: error: cannot find symbol
  right.move(0, yylength(), yylength());
       ^

According to http://czt.sourceforge.net/dev/java-cup-runtime/apidocs/java_cup/runtime/ComplexSymbolFactory.Location.html there is no Location constructor of the type you use.

Could be that your patch requires a different version of cup?

ubuntu xenial comes with version 0.11a.

apt list -v cup
cup/xenial,xenial,now 0.11a+20060608-6 all [installed]
  LALR parser generator for Java(tm)

What is the minimal version of cup you require?

[andreasabel]

It seems that with the latest version of java-cup, there are no problems with the testsuite.

How can we ensure that or test whether the necessary version is installed?

[guardbotmk3]

With the move in there, it requires a fairly new cup, 2015 or so. Without it, any cup 0.11b will do.

I can also adjust the code to work with 0.11a. I have that working already. I'll create a new PR that does that.