-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
basic: merge eval_ast into EVAL #707
base: master
Are you sure you want to change the base?
Conversation
@asarhaddon Sounds like a good plan. I've been quite impressed by your rapid progress so far. But I wasn't sure if you were going to tackle basic. The combination of early BASIC's lack of features along with garbage collection makes it pretty daunting TBH. |
b5366f8
to
972d561
Compare
The last commit fails on step8 for both variants, with the same message: "file not found". |
Let me try this locally and see if I can find a clue to what's happening. |
@asarhaddon Somewhere along the way the filename string (for the command line argument file to load ) that is passed to DO_READ_FILE gets corrupted:
|
Thanks. I was forgetting that MAL itself read files. I intend to backport the separate evaluation of the function and the arguments from step8 downto step2. They are only required by macros, but it is good to debug them step by step. This one will probably explode far before step6. |
That makes a lot of sense, especially for basic (and probably wasm too) where the memory is managed. Getting similar code paths in place as early as possible is good so that problems with reference counting and memory management can be caught earlier where it's simpler to debug. |
4bf7fc1
to
b98925a
Compare
I am giving up for now. Here are the remaining blockers.
I have updated the Dockerfile in order to try some local tests. The qb64 part seems completely out of date. This is not a prerequisite for this merge request. Ironically, |
This is a prerequisite for merging macroexpand into EVAL.
@asarhaddon I did a little searching and found that FreeBASIC (freebasic.net) has qbasic support ( I might be able to look at why those are failing in the next couple of weeks but won't be able to touch it again for a few days at least. If you do the following at the REPL you can trigger errors that are pretty clearly a GC/reference counting issue:
The above starts happening with the f/args separation (and it's the same with cbm or qbasic). |
I intend to successively push and test small commits until the macroexpand function can be removed.