Treat address labels as "const" symbols

antis81 commented 3 years ago

This PR serves as a basis to check on symbol parsing states during passes. States are:

~~declared -> Symbol has been registered (with or without a value assigned)~~
accessed -> Symbol is accessed from at least one code location (not necessarily defined)
defined -> Symbol is declared and value has been assigned

Other attributes:

final -> Symbol is marked constant (redefining it leads to error)

antis81 commented 3 years ago

@sasq64 Giving it another try… :) Could you please give a feedback on the latest 2 commits moving this in the right direction? ~~Why would subsequent passes try to redefine a symbol once it is complete?~~

antis81 commented 3 years ago

Ran the existing tests manually via make test -> :ok:

Are you ok with changing the meaning of "is_defined" to "is_declared"?

antis81 commented 3 years ago

@sasq64 Could you please double-check commit b6a5674?

sasq64 commented 3 years ago

Sorry for not answering eariler, Anyway my only problem is that is not clear what "declared" means, since you can't decalare and not define symbols. Isn't it really the same as "accessed" ? But if we need to keep it, I think it should be renamed to exists() or used().

antis81 commented 3 years ago

First the easy stuff. Yeah you're right the endless recursion does not happen -> cleaned up a little by calling "set_sym(name,map)" directly (1ed1b68). Rename to "set_map" may help.

Went through the get/set functions in detail yesterday -> Making it short: Yeah, "accessed" and "defined" should be enough state(s). In current implementation the "is_defined" checks on the "defined" flag in addition to "exists()" -> test 65c02 fails (A != 1 -> which should be in table[0])

Looks like one of the "is_defined" needs to change to "is_accessed"… needs a bit more investigation/debugging.

I like to go back to the original intention creating this issue -> check if a (constant) label had been redefined. This should be the "final" attribute right? This should apply (only?) on "pure" address labels (label without an "=" assignment operator). Ah… code is easier to understand:

!org $1000  ; just to give a section context
i_am_var = 42  ; a "var" label type 
i_am_label:      ; a "final" label type

Shouldn't be overly complex - am I missing something?

antis81 commented 3 years ago

@sasq64 FYI: The tests are ok again ("table" is defined). Seems the "defined" flag is not always set/cleared correctly - but the lists are. Can we remove the "defined" flag completely?

antis81 commented 3 years ago

Hi @sasq64, reached the "works for me" state! The test ./tests/err_labels.asm now fails. ~~Can it give an "ok" result instead aka "death test" or something?~~ All fine now! (pls review - tests contain a "poor man's include guard" now)! :slightly_smiling_face:

builds/debug/tester all
/home/nils/Projects/badass/./tests/65c02.asm
* PARSING
Using cached AST
* PASS 1
* PASS 2
* TESTS (1)
Running code at $1000
Start
*** Test 'start' : 93 cycles, [A=$02 X=$02 Y=$03]
/home/nils/Projects/badass/./tests/banks.asm
* PARSING
Using cached AST
* PASS 1
* TESTS (1)
Running code at $2000
BANK    2
BANK    1
BANK    2
BANK    1
*** Test 'test' : 307 cycles, [A=$01 X=$ff Y=$10]
/home/nils/Projects/badass/./tests/basic_errors.asm
* PARSING
Using cached AST
* PASS 1
ERROR 'Illegal instruction 'xyz'' in /home/nils/Projects/badass/./tests/basic_errors.asm:4
/home/nils/Projects/badass/./tests/copy_test.asm
* PARSING
Using cached AST
* PASS 1
* PASS 2
* TESTS (2)
Running code at $800
*** Test 'my_test' : 193 cycles, [A=$a9 X=$0c Y=$00]
Running code at $1007
*** Test 'dummy' : 5 cycles, [A=$04 X=$00 Y=$00]
* FINAL PASS
$a9
…
... <all tests before tests/err_labels.asm are OK> …
…
/home/nils/Projects/badass/./tests/err_labels.asm                                                                                                                                                                                                                             
* PARSING                                                                                                                                                                                                                                                                     
* PASS 1
ERROR 'already defined label 'same_label'' in /home/nils/Projects/badass/./tests/err_labels.asm:3
===============================================================================                                                      
test cases: 1 | 1 passed
assertions: - none -

sasq64 commented 3 years ago

Testing this now. I am afraid an important case may not be covered by the tests, because labels can move, exampe: lda something,x label: nop

If something < 0 its a different opcode than > ff so label will be moved 1 byte for next pass

sasq64 commented 3 years ago

... but it does seem to work :) Checking some more

sasq64 / bass

Treat address labels as "const" symbols #18