Misleading errors when constant definitions incomplete
pushfoo opened this issue · 4 comments
Octo build used: Current web IDE build
Browser used: Chrome 86.0.4240.111 (Official Build) (64-bit), Windows
I haven't tried this using the nodejs cli.
The assembler sometimes gives incorrect or misleading errors when a constant definition isn't fully completed. Beginners will get confused by this, so it's worth clarifying the errors.
Two simple examples are listed below, but I think are still some strange and more severe instances of this problem that I still need to create succinct examples for.
Example 1
The code below produces line 13: Undefined name ':'. instead of a clearer message indicating that a definition wasn't fully closed.
: filler_label_1
v2 := v2
return
:macro NOOP {
v0 := v0
}
# bad definition
:const UNDEFINED_CONST
# this is only an example to take up space
: filler_label_2
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
return
: main # actual game implementation would go here
hires
: game_loop
filler_label_1
filler_label_2
jump game_loop
Example 2
In this version, the example returns line 7: Undefined name ':macro'.
: filler_label_1
v2 := v2
return
:const UNDEFINED_CONST
:macro NOOP {
v0 := v0
}
# this is only an example to take up space
: filler_label_2
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
return
: main # actual game implementation would go here
hires
: game_loop
filler_label_1
filler_label_2
jump game_loop
v0 := key
With the updated compiler, your examples would produce error messages like:
Line 13: Expected a constant name, but found the keyword ":macro". Missing a token?
The current changes seem to be an increase in clarity without increasing compiler complexity. I'd be in favor of even clearer messages that point out mistakes in more depth, but i can understand if you want to keep the code as simple as possible.
Also, the code below produces an error of line 26: Undefined name 'EXAMPLE_CONSTANT'. It's true that this is an undefined constant, but I'm not sure why the compiler doesn't flag the earlier malformed constant definition. It might be worthwhile to block the usage of reserved keywords as label names.
: const EXAMPLE_CONSTANT 5
:macro NOOP {
v0 := v0
}
: filler_label # doesn't do anything
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
NOOP
return
: main
filler_label
v1 := EXAMPLE_CONSTANT
return
const isn't a reserved keyword; :const is. The name "const" is an entirely valid label.
It would be a large breaking change to reserve all suffixes of reserved keywords, and I don't think it would do much to prevent mistakes.
Good point on backward compatibility. Thank you, I think your fixes close the scope of the original ticket. I'll think about the constant issues a little more and file another ticket proposing a fix that doesn't break compatibility with unusual label names.