"Syntax error" message is way too obtuse

[ISSOtm]

issotm@link-panda tmp.UjOR1nmUvy% rgbasm -V
rgbasm v0.3.8-66-ge33e6e2
issotm@link-panda tmp.UjOR1nmUvy% cat test.asm   
if A
endc
issotm@link-panda tmp.UjOR1nmUvy% rgbasm test.asm
ERROR: test.asm(1):
    syntax error
error: Assembly aborted (1 errors)!

[ISSOtm]

I have tried:

• Putting it in a macro
• Simplifying the condition
• Adding indentation
• Doing it in an INCLUDEd file
• Defining a SECTION before
• Putting some code before it
• Putting a label before it
• Putting a global label before it
• Using caps

None of that works.

[ISSOtm]

issotm@link-panda tmp.ptNgXXBbUR% rgbasm -V
rgbasm v0.3.8-66-ge33e6e2
issotm@link-panda tmp.ptNgXXBbUR% cat a.asm
if !def(A)
	WARN "Not defined"
else
	WARN "Defined"
endc
issotm@link-panda tmp.ptNgXXBbUR% rgbasm a.asm
ERROR: a.asm(1):
    syntax error
error: Assembly aborted (1 errors)!
1 issotm@link-panda tmp.ptNgXXBbUR% cat b.asm
if !def(__FILE__)
	WARN "Not defined"
else
	WARN "Defined"
endc
issotm@link-panda tmp.ptNgXXBbUR% rgbasm b.asm
warning: b.asm(3):
    Defined

The issue appears to stem from the variable not being defined. def() should prevent that, however.

I don't understand how pokered manages to compile with this bug, however.

[ISSOtm]

Looks like the error was a conflicting variable name (register a). The error message was beyond obtuse though.

[AntonioND]

Yeah, the parser is less than ideal. For example, register c and the condition c are the same thing internally.

[AntonioND]

https://github.com/rednex/rgbds/blob/e33e6e24132dea1490e3039a6c85194d2b6a7a74/src/asm/asmy.y#L2161-L2165

[meithecatte]

Perhaps the handler for syntax error could be modified so that the current line is printed, together with a row of ^ under the token that doesn't match any rule?

[dbrotz]

This is the Bison documentation on error reporting.

https://www.gnu.org/software/bison/manual/html_node/Error-Reporting.html

If we put %define parse.error verbose in "asmy.y", then it will print the following error:
syntax error, unexpected T_TOKEN_A

You can also name the tokens like this:
%token T_TOKEN_A "Reg A"

Then it prints the following:
syntax error, unexpected Reg A

This would make things a little better. We need to ditch yacc if we want really nice errors.

[ISSOtm]

This is worth experimenting with, if only to slightly improve the experience.

Wouldn't ditching yacc require basically a full rewrite? Not that it would be a bad thing, the code base reeks of 90's-era static buffer manipulation and other similar stuff, but that's a really bug project.

[dbrotz]

Yeah. At that point, it might be better to just make a whole new assembler. It would be difficult to significantly improve rgbasm without drastically altering its design.

[ISSOtm]

HGBASM exists, but because it's built on JavaScript it's fairly slow compared to RGBDS (takes several seconds to assemble individual files in my project versus below half a second for RGBDS).

I wanted to port it to C or C++, but sadly I don't understand how it works, so I dropped that idea.

[AntonioND]

Yeah. At that point, it might be better to just make a whole new assembler. It would be difficult to significantly improve rgbasm without drastically altering its design.

Yes, that's pretty much what I noticed ages ago... It's too much work, though.

[dbrotz]

I like the idea of a new assembler, but I feel that any assembler that tries to be compatible with RGBDS will have to have hacky code to deal with the nature of EQUS and its textual replacement-based macros. Those are admittedly common features in macro assemblers. I don't know of any that have cleaner ways of doing macros.

[ISSOtm]

As far as I know it has a few hacks to stay compatible, but is overall cleaner and more flexible.

[dbrotz]

I took a look at it. It's a lot higher level than RGBDS and I suspect it would be slower than RGBDS even if you rewrote it in C++. I wouldn't even try to write something similar in C because it would be a lot more code in a lower-level language.

It does look a lot cleaner, but it seems to me that it has some incompatibilities just from looking at its code. I would need to run it to confirm.

[ISSOtm]

Yes, it has some incompatibilities, since it doesn't try to emulate all quirks of the parser and custom lexer. It's more of a "99% compatibility" target, if you want.

[ISSOtm]

You can also name the tokens like this:
%token T_TOKEN_A "Reg A"

Then it prints the following:
syntax error, unexpected Reg A

This would make things a little better. We need to ditch yacc if we want really nice errors.

We may actually have to ditch POSIX yacc to get better and readable syntax errors:

src/asm/asmy.y:604.20-25: warning: POSIX Yacc does not support string literals [-Wyacc]
  604 | %token	T_POP_ENDM "ENDM"
      |

[ISSOtm]

Perhaps the handler for syntax error could be modified so that the current line is printed, together with a row of ^ under the token that doesn't match any rule?

Took another look at this, and we have a problem in the form of guessing where the line starts, printing the current line, and figuring out on which column the error occurred. I think the lexer would be the place to implement this in, but I'm worried in that there's a bunch of places where newlines could be read.

[hlorenzi]

Just to throw out an idea (but it's not a solution to the problem at hand): I've been building a generic assembler which primarily lets you define the syntax for a machine's instructions, and has some other nice features, like pretty error messages and math expression support everywhere.

I've been following RGBDS for quite some time now, and unfortunately there's a bit of a long way before my assembler could become some kind of a replacement, but I nonetheless would like to know if there would be any interest in it!

[ISSOtm]

Separately from this issue if anything, the discussion is not about replacing this code base, but improving it. Besides,

generic assembler

sounds a lot like ASMotor.

[ISSOtm]

This, plus other QoL changes, sound to me like enough of a reason to switch to Bison instead of POSIX yacc. (Bison can print named symbols, and can also report where in the syntax it was).
However, I'd like the opinion of the FreeBSD guys (cc @AntonioND @bentley), since as far as I'm aware, "yacc" on all (mainstream, at least) Linux distributions is really Bison with a compatibility flag.

[AntonioND]

I'm not a BSD guy, I personally just use Debian at home. 😆

In general I have mixed feelings about dropping support for one tool. The way I see things, if switching tools means that the code can be clearer or easier to maintain, I'm generally for it, even if it means adding an extra explicit dependency. I guess that even BSD can install bison.

[Rangi42]

Bison 3.6 introduced support for %define parse.error custom, which lets you define yyreport_syntax_error yourself. That would enable reporting the unexpected token and a list of possible expected ones, with custom names/descriptions instead of their yysymbol_name source-code identifiers.

[Rangi42]

Actually, even %define parse.error verbose with custom token names (e.g. %token T_MODE_AF "register af") would provide decent syntax error messages.

[ISSOtm]

I know, I tried, but it explodes with various colors. And I don't understand why yet.