crowell / modpagespeed_tmp

Automatically exported from code.google.com/p/modpagespeed
Apache License 2.0
0 stars 0 forks source link

js_lexer.cc is not ready to parse real-world JavaScript #743

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
From Alex Wu:

I am experimenting some code, and realized and js_lexer,cc under ./js cannot 
tell '/' is a division operator for this javascript code:

{do{m}while(e()/h)};

It tells that '/' is a start of reg expression. I have simplified test case to 
this level so that the bug can be reproduced in a simple way.

Original issue reported on code.google.com by sligocki@google.com on 11 Jul 2013 at 5:31

GoogleCodeExporter commented 9 years ago
If it's really about js_lexer.cc and not js_minify.cc then this shouldn't go to 
Matthew.

Original comment by morlov...@google.com on 11 Jul 2013 at 5:34

GoogleCodeExporter commented 9 years ago
This is js_minify.cc, I don't think we use js_lexer.cc

This is just like issue 675 but for a different preceeding token.

Original comment by jkar...@google.com on 11 Jul 2013 at 5:59

GoogleCodeExporter commented 9 years ago
I am not sure; the reporter seems to be doing some stuff w/the code, so it's 
possible they are actually using js_lexer.cc for something. 

Original comment by morlov...@google.com on 11 Jul 2013 at 6:04

GoogleCodeExporter commented 9 years ago
Indeed looks like this is about js_lexer.cc, not js_minify.cc. I didn't even 
know we had this class, I doubt it's being maintained. Seems like if we want 
this to stay up to date we should share code with the actually maintained 
js_minify.cc.

Original comment by sligocki@google.com on 11 Jul 2013 at 6:26

GoogleCodeExporter commented 9 years ago
Yes, js_lexer is an unmaintained class.  It'll receive some love if we ever 
need to do a full JavaScript parse; Josh and I have looked into that a couple 
of times but the gains have never justified the time investment so far.

Note that getting parsing of / right in Javascript is probably the hardest part 
of the language (and the source of several now-fixed bugs in our current 
minifier).  It may not be possible to do it with a pure scanner / lexer split 
in classic compiler-class fashion.

Original comment by jmaes...@google.com on 11 Jul 2013 at 6:57

GoogleCodeExporter commented 9 years ago
Yes that the time I wrote js_lexer I was hoping to eventually change the JS 
Minifier to use it so that we'd keep it up-to-date & correct.

However we still haven't developed a real JS parser.  When we do, I think we 
should change js_minify to share the lexer.

Note that js_lexer.cc was adapted from js_minify, but js_minify has had bugs 
fixed since then.

So this isn't really a mod_pagespeed bug at this point; changing the Summary to 
be more accurate & the type from Defect to Enhancement.

Original comment by jmara...@google.com on 6 Aug 2013 at 8:09

GoogleCodeExporter commented 9 years ago

Original comment by jmara...@google.com on 6 Aug 2013 at 8:12

GoogleCodeExporter commented 9 years ago
FYI js_lexer.cc has been deleted from the system, and is now replaced by 
pagespeed/kernel/js/js_tokenizer.cc.

Alex -- can you try this one (in trunk) and see if it meets your needs better?  
The plan is that we will remove the old minifier and use a new minifier based 
on this.

Original comment by jmara...@google.com on 12 Feb 2014 at 7:55

GoogleCodeExporter commented 9 years ago
I saw that. Thanks. I'll have work of merging changes ahead. Will test it out 
soon.

Original comment by yw95...@gmail.com on 12 Feb 2014 at 9:28

GoogleCodeExporter commented 9 years ago
Obsolete js_lexer.cc removed from system.

Original comment by sligocki@google.com on 29 Oct 2014 at 8:09