munificent / craftinginterpreters

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

Mistake in the implementation of the logical operator in the Jlox interpreter #788

Closed nocturn9x closed 4 years ago

nocturn9x commented 4 years ago

Hi Bob, I've been reading your amazing book for a couple of days already, and I'm reimplementing lox in Python (I plan to make my own language in Nim for performance after that) and I'm amazed by the quality of your book.

Unfortunately I found a weird mistake that was probably a typo of some sort, but at this page, in the section that implements visitLogicalExpr you return the left operand no matter if it is itself truthy or not. Initially I thought I didn't understand the code properly, but when I tried with my repl the error was evident: print 2 or nil; worked, it printed 2, but print nil or "yes"; actually returned None (Python equivalent to Java's null). So I tried to replace the line after if (!isTruthy(left)) with return evaluate(expr.right) and so far it seems to behave properly both with logical ANDs and ORs. Am I missing something here that I didn't notice? If yes I'd be pleased to find out! I'm 17 after all ;)

Have a great day!

~ Mattia Giambirtone aka nocturn9x

P.S.: I didn't actually change the Java implementation, as I made my own one in Python by carefully translating your code. It might be as well an issue with my interpretater, but since now the results have been identical to the examples and I just felt that opening an issue might have been the right thing to do

Axnyff commented 4 years ago

Hi, I've stumbled into your issue while looking for an issue on my own. It seems like the bug you're describing has been fixed since your post, print nil or "yes"; now correctly returns "yes"

nocturn9x commented 4 years ago

Great!