munificent / craftinginterpreters

Repository for the book "Crafting Interpreters"
http://www.craftinginterpreters.com/
Other
9.05k stars 1.06k forks source link

clox not being able to concat several new strings in one line (end of ch 20) #1143

Closed brachetti closed 1 year ago

brachetti commented 1 year ago

At the end of chapter 20, we store strings in the vm's internal hash table. This works great, except when we want to concatenate multiple new strings in one line, which leads to a segmentation fault. As far as I can tell that is due to the last string not having been committed to the table before.

So this code leads to an error state:

print "a" + "b";
print "a" + "b" + "c"; // no seg fault, as we already know the string "ab"
print "x" + "y" + "z";

This is the output from Lox with debug information

$ ./build/clox test2.lox
== code ==
0000:    1 OP_CONSTANT         0 'a'
0002:      OP_CONSTANT         1 'b'
0004:      OP_ADD
0005:      OP_PRINT
0006:    2 OP_CONSTANT         2 'a'
0008:      OP_CONSTANT         3 'b'
0010:      OP_ADD
0011:      OP_CONSTANT         4 'c'
0013:      OP_ADD
0014:      OP_PRINT
0015:    3 OP_CONSTANT         5 'x'
0017:      OP_CONSTANT         6 'y'
0019:      OP_ADD
0020:      OP_CONSTANT         7 'z'
0022:      OP_ADD
0023:      OP_PRINT
0024:    5 OP_RETURN

0000:    1 OP_CONSTANT         0 'a'
           [ a ]
0002:      OP_CONSTANT         1 'b'
           [ a ][ b ]
0004:      OP_ADD
zsh: segmentation fault  ./build/clox test2.lox

For reference, this is my current working state

Disclaimer: There's a decent chance that I made a mistake, in which case I would be grateful for pointers. I already spent an ungodly amount of time on this 💯

brachetti commented 1 year ago

I found the problem, sorry for bothering 🥲

FWIW, I mistyped a sizeof calculation in the ALLOCATE(type, count) macro (true instead of type), here's the fix

I only found the issue due to a hint in CLion actually, saying there's something fishy going on in a sizeof check