REPL segfaulting in a few situations about ark HOT 7 CLOSED

The last three commits should only fix the VM behaviour on empty blocks:

• () and {} as programs don't generate segfaults
• ((fun () (print "hello"))) should print hello (yay, anonymous functions are back)
• (print (if true () ())) should print nil
• (print (fun () ())) should print Function @ 1 instead of only its page address, 1

Feel free to take a look at it (I can have forgotten edge cases) or even to try to fix the REPL!

from ark.

I've tried creating a buffer that holds all the code that's been entered, and then passing it to a compiler to check for errors (this feels a little bit redundant though due to the new state.doString(...) function).

I've been messing around with saving the locals in the REPL, and then assigning the old locals to the new VM's locals. I have a few more things to try. I've been spending my time playing the the State and VM to get a better idea of how they work (it's all been very fun and interesting though).

I'll open up a new issue about the REPL variable/import persistence once I have tried a few more things.

from ark.

• The conditional statement to check if line == "(quit)" is redundant since it has balanced parentheses and will be taken care of in the next conditional where the condition for the parentheses is checked.
  @SuperFola, is it a good idea to quit as soon as we see a (quit) without letting the VM know (by appending HALT to the bytecode)?

• The parser doesn't handle a few cases. The parse method checks if the first token is a '(' or a ShortHand (not sure what this is). If it is neither of the to it assumes it's an atom (Number or String, etc.).
  The implementation should be modified to check for cases such as )(. Such cases must throw a ParseError.

• @FrenchMasterSword, in both cases (1 & 2), the lexer strips it down to a string of 0 length. The issue is probably with the VM, which raises a segfault when there are no instructions. Inputs such as {} or {#asdjf\n} compile to nothing.
  @SuperFola, will HALTing by default at the end solve this?

from ark.

• the next condition you are talking about is breaking the for loop, not the while loop, and I think we need to keep the parens around quit just for uniformisation ; that's a lisp like After all. Sending halt to the vm isn't useful because it's only halting the current instance of the VM (btw every code page is ended n'y an HALT, but the empty code page wasn't ; I've implemented a fix locally)

• A shorthand is ' expanding into (quote ...). We should be able to parse things like (print (foo)(egg)) which is a correct program and have a )( in it. Or maybe you mean that we should throw an error when we meet a first token being a ), then a (?

• As said in the previous bullet point, we're already halting the vm on an empty code page, the problem was that I told thé compiler to write something like : code page size 0, append HALT. Thus the vm is segfaulting because it doesn't care if the cp is of size 0, it's waiting for at least 1 instruction, and we put one it wasn't waiting for, thus the function handling tables & cp filling reaches an UB ; fixed this specific thing locally, will push asap.

I couldn't reproduce the segfault with the comment block on Windows, I'll try ASAP on Linux.

If you have any idea why this is segfaulting on a comment, I would be glad to hear it! IMO it can be related to the both the parser and the compiler.

I think the test L26 in Repl.cpp should as well strip blank characters to see if the line is really empty, thanks for your report @FrenchMasterSword !

from ark.

Hi @rstefanic, since you wanted to work more on the REPL, would you like to work on this issue?

from ark.

Yeah sure. After reading through this thread, the only work for the REPL that I can pick out here is to check if the line entered is all whitespace or starts with a #, and in those cases, we'd just not pass it to the state.

Is that what you're looking to add? I can definitely add that (or something else that I've missed).

I'm actually still working on REPL persistence and I've found it to be a bit trickier than I had first imagined.

While I can save the m_locals of the VM, there are still problems when the code is compiled, because (1.) new code passed to the state is stored on a new page so I have to manually increment the PP (not a big deal) and (2.) every time you compile some code, the resulting bytecode will reference the ids from the bytecode that was just compiled before hand, but the compiler is unaware of them.

As an example, if I compile the string (print "Hello!") in the REPL, the constant "Hello!" will be given an id of 1, and the resulting bytecode will say "LOAD_CONSTANT ID: 1" to the print function.

Then afterwards if I compile the string (print "Goodbye!"), the constant "Goodbye!" will be stored with an id of 2, but the compiler doesn't know that because it's the only constant it's seen. So the compiler will give it an id of 1, and the newly compiled bytecode for this second statement will still say "LOAD_CONSTANT ID: 1", which will then result in printing "Hello".

I hope this makes sense. I've been spending a lot of my time with this just playing with the compiler, state, and VM trying to understand how things are loaded and in what order. If you have any tips (or corrections), I'm definitely open to them!

from ark.

Is that what you're looking to add?

Yes, definitely!

hen afterwards if I compile the string (print "Goodbye!"), the constant "Goodbye!" will be stored with an id of 2, but the compiler doesn't know that because it's the only constant it's seen. So the compiler will give it an id of 1, and the newly compiled bytecode for this second statement will still say "LOAD_CONSTANT ID: 1", which will then result in printing "Hello".

What if we have a buffer for all the code entered, recompile it everytime and update it if the compiler has 0 error? After each execution we save the ip (we should always get back to PP=0 when a program has finished running), and relaunch the VM with the old state updated to have a new locals map (just put FFI::undefined for the new symbol ids), starting at ip-1? (-1 because the last ip is the one where we have a HALT)

from ark.