munificent / craftinginterpreters

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

Parser: `term` and `factor` are a level off. #115

Open rkirsling opened 7 years ago

rkirsling commented 7 years ago

The words term and factor are being misused in Lox's parser.

A polynomial is a summation of terms which are each a product of factors. These are correctly used in the Wikipedia article on recursive descent parsing, but Lox is a level off.

I'm not certain what the best fix isβ€”if you put them in the right place, primary would lose its spot and you'd need a new word for the operands of a comparison.

FWIW, the V8 asm.js parser (which, in doing away with precedence, looks strikingly similar to Lox's) evidently calls these AdditiveExpression and MultiplicativeExpression.


P.S. Thanks a bunch for writing this book. It's super empowering to be able to jump into the repo of an arbitrary language / VM and find its lexer and parser "readable". :smile:

munificent commented 7 years ago

Ack, you're right! That took a while to fix. Had to redraw an illustration and everything.

Thanks a bunch for writing this book. It's super empowering to be able to jump into the repo of an arbitrary language / VM and find its lexer and parser "readable".

You're welcome! That's exactly the goal. 😁

mkrauskopf commented 1 year ago

Is this fix going to be reflected in the books? The Kindle version from Amazon (I've bought) and the online version (chapter 5) still contain the old Lox grammar with confusing term and factor usage.

Since the book is famous now πŸ˜‰, it spreads the incorrect grammar terminology among the readers (usually beginners I guess) who try to understand grammar and the parsing process. It would be great if the books can be updated with the new grammar.

Or is the "books update" tracked elsewhere?

Thanks for the approachable, superb book. πŸš€

munificent commented 1 year ago

Oops, looks like I just missed those table headers when I fixed it everywhere else.

Since I published the print edition, I haven't been making any more updates to it. I probably will at some point, but it's quite a hassle to keep track of the changes, fix the typesetting, etc. So, for now at least, I've just been leaving issues open in the repo to tracker changes that should be done at some point.

HuM4NoiD commented 1 year ago

I am following the html version of the tutorial here, it still has the rules:

term           β†’ factor ( ( "-" | "+" ) factor )* ;
factor         β†’ unary ( ( "/" | "*" ) unary )* ;

I looked at the code and it is following the same rules.

I have started following the series, Do you recommend sticking with the series or proceed with the actual definition? I'll try being braver and change the production rules a little bit.

Again, thanks for breaking down the entry barrier for building languages.