lcompilers / lpython

Python compiler
https://lpython.org/
Other
1.5k stars 157 forks source link

Improving prefix-string parsing (fixes #2339) #2511

Closed mrdaybird closed 6 months ago

mrdaybird commented 7 months ago

Fixes #2339 Modifies how prefix-strings is parsed and tokenized.

  1. Tokenize prefixes ( 'r', 'b' etc) as Soft keyword KW_STR_PREFIX
  2. Update grammar to check for KW_STR_PREFIX followed by STRING

Now the compiler gives a syntax error when encountering a identifier(not a prefix) followed by a string literal.

certik commented 7 months ago

Thanks! This PR looks good.

Can you write a test that failed (or gave a misleading error message) in master, and now works (gives a good error message)?

If it is just a better error message, you can put it into tests/error.

mrdaybird commented 6 months ago

Added the tests. But to clarify, should I use the 'ast' flag or the 'ast_new' flag for the tests? are they even relevant for testing errors?

certik commented 6 months ago

This looks great, thanks for fixing it and adding tests. I left some minor comments to fix.

But to clarify, should I use the 'ast' flag or the 'ast_new' flag for the tests? are they even relevant for testing errors?

Use ast to test syntax errors. Use asr to test semantic errors.

certik commented 6 months ago

You have to update reference tests via ./run_tests -u.

mrdaybird commented 6 months ago

Thanks I noticed a little late. :sweat_smile: Should I rebase as well?

certik commented 6 months ago

Either polish the commits to a few logical commits and rebase, or just leave it and I'll squash and merge it.

mrdaybird commented 6 months ago

I think you can just squash and merge it. I think I would probably mess things up with the rebase.