-
-
Notifications
You must be signed in to change notification settings - Fork 747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix lexing of regex at the start of a if/loop #2529
Fix lexing of regex at the start of a if/loop #2529
Conversation
This only really appears in code (espruino/BangleApps#3498) which is minified to something like: ```javascript var b=E.getPowerUsage(), c=E.getBattery(), a=0; for(var d in b.device) /^(LCD|LED)/.test(d) || (a += b.device[d]); ``` ... resulting in: ``` Uncaught SyntaxError: Got '^' expected EOF at line 1 col 109 in widbattpwr.wid.js ...attery(),a=0;for(var d in b.device)/^(LCD|LED)/.test(d)||(a+=b.device[... ^ at line 1 col 110 in widbattpwr.wid.js ...ttery(),a=0;for(var d in b.device)/^(LCD|LED)/.test(d)||(a+=b.device[d... ^ ``` So while the code runs fine normally, installing it via the proper route minifies it and leads to it being unable to run. --- This is the case for both `if` statements and loops: ```javascript >for(;0;)/a/; Uncaught SyntaxError: Got ';' expected EOF at line 1 col 9 for(;0;)/a/; ^ at line 1 col 12 for(;0;)/a/; ^ >for(x in [])/a/; Uncaught SyntaxError: Got ';' expected EOF at line 1 col 13 for(x in [])/a/; ^ at line 1 col 16 for(x in [])/a/; ^ ``` while & if statements: ```javascript >while(0)/a/; Uncaught SyntaxError: Got ';' expected EOF at line 1 col 9 while(0)/a/; ^ at line 1 col 12 while(0)/a/; ^ >if(0)/a/; Uncaught SyntaxError: Got '/' expected EOF at line 1 col 6 if(0)/a/; ^ >if(0)3; =undefined ``` The issue is that we don't lex regex immediately after a close-paren, so the fix is quite straightforward.
Excellent - thanks for tracking this down! Nice easy fix, but potentially a hard one to find :) I'll update the parser in Espruinotools too |
Thank you! |
I suspect this has broken something. The clock face doesn't show - it's just blank. Clicking the hw button fastloads the launcher ( Fastloading
|
Adding on, I verified this one works: https://www.espruino.com/binaries/travis/584b5886906327d0cae5bb7e7301490731fcfa58/ But this next one breaks: https://www.espruino.com/binaries/travis/28b41fd99bf8dabb841be7a3f926a0be866aaeeb/ The commit history: |
And that is why we didn't have |
because of espruino/Espruino#2529 (comment) This reverts commit 05918f8.
Thanks for testing and reporting this @thyttan! Did it actually work for you on a real watch @bobrippling ? It looks like none of the desktop test cases found any issues with this, but I have now just added one |
Thanks for the quick find! I didn't find an issue but I only gave it a cursory check that the interpreter could handle regex. So to fix this I think we'll need to change how things are done - it's a large change so I'll run it by here to see what you both think first. I think we should stop lexing a regex and instead decide whether we have one based on the parsing state, so we're not dependant on a look-behind at the previous token. My reasoning here is at a parse level it's relatively trivial to tell if we're going to have a regex or not: x = a /
// ^ when the parser is here, we have a bianry-/, so it's obvious it's a division
x = /
// ^ when the parser is here, we have a unary-/, so it's either a regex or a comment.
// the lexer takes care of comments, so we know it's a regex,
// so we now step through this with the usual regex parsing code This would mean removing What do you think? |
I'm afraid this won't work, at least not without crippling performance - Espruino uses bracket counting to skip over areas of code that it doesn't want to execute. Either when parsing functions,
So in your case in order to parse the function we have to be able to run over the entire parse tree each time we looked over any code, because otherwise I see two options here really:
|
Dang, I had wondered what the rationale was behind the lookbehind code. I think the lexer state machine could be good, I'll take a look but it might be a while - I'm away for a bit with it being summer! |
This only really appears in code (espruino/BangleApps#3498) which is minified to something like:
... resulting in:
So while the code runs fine normally, installing it via the proper route minifies it and leads to it being unable to run.
This is the case for both
if
statements and loops:while & if statements:
The issue is that we don't lex regex immediately after a close-paren, so the fix is quite straightforward.