Last Comment Bug 637204 - Some unconditionally reserved words are reserved only in strict code
: Some unconditionally reserved words are reserved only in strict code
Status: RESOLVED FIXED
fixed-in-tracemonkey
: dev-doc-complete, relnote
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- trivial (vote)
: ---
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-27 13:48 PST by Mark S. Miller
Modified: 2011-08-04 11:02 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, reenable the test (3.92 KB, patch)
2011-03-01 18:51 PST, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review
Oops, missed some changes (9.23 KB, patch)
2011-03-02 13:54 PST, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review
diff -w without function name bits (7.66 KB, patch)
2011-03-03 19:06 PST, Jeff Walden [:Waldo] (remove +bmo to email)
brendan: review+
Details | Diff | Splinter Review

Description Mark S. Miller 2011-02-27 13:48:09 PST
Some unconditionally reserved words are reserved only in strict code:
class, enum, export, extends, import, super

I am submitting this classified as "trivial" because these are properly rejected in strict code, which is all I really care about. If someone does care about this case, please raise the severity.

If this is closed as a wont-fix, could we also have a tag like DeliberateSpecViolation (as v8 now has) so we can more easily keep track of such cases?
Comment 1 Brendan Eich [:brendan] 2011-02-27 14:49:01 PST
See bug 497869. We can try to reserve harder next time (or the time after that, with a quarterly release schedule) -- it will take more testing and possibly even cross-browser coordination.

/be
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2011-02-27 17:15:21 PST
Chrome and Opera are the only other engines that make these not keywords.  IE never unreserved them, and I think WebKit didn't either (or at least in my testing back when they were reserved outside strict mode).  I think this mostly just needs some lag time for Mozilla-centric sites (like the ones mentioned in that bug) that seemingly didn't care about cross-browser behavior to get fixed.
Comment 3 Brendan Eich [:brendan] 2011-02-27 19:15:04 PST
Has anyone filed a bug on V8?

/be
Comment 4 Mark S. Miller 2011-02-27 19:26:43 PST
http://code.google.com/p/v8/issues/detail?id=1214
Comment 5 Mark S. Miller 2011-02-27 19:36:51 PST
See also https://bugs.webkit.org/show_bug.cgi?id=55342
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-01 18:51:48 PST
Created attachment 516127 [details] [diff] [review]
Patch, reenable the test

Interestingly it seems the current code had a bug with just the token-type change.  I didn't notice it when we initially changed this because I'd written a not-complete test at the time.  But since then I landed bug 629187, which beefed up the test quite a bit, adding the new tests that fail without the jsparse.cpp changes in here.  Hurray for more-complete tests finding failures!

It'd be good to land this early this next cycle so anyone using these names as keywords can adapt, therefore doing this now rather than any later.  If we can land when TM/m-c reopen to post-4.0 work that gets us the maximum nightly coverage possible, which can only be a good thing.
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-02 13:54:38 PST
Created attachment 516384 [details] [diff] [review]
Oops, missed some changes
Comment 8 Brendan Eich [:brendan] 2011-03-03 16:43:45 PST
(In reply to comment #6)
> Interestingly it seems the current code had a bug with just the token-type
> change

Do you mean the parser enabling keyword-as-name for function names (identifiers after 'function' keywords and the 'function::' namespace)? That was intentional:

revision 3.227
date: 2006/09/07 11:28:30;  author: igor.bukanov%gmail.com;  state: Exp;  lines: +17 -3
Bug 343675: allow to use keywords as function names. r=brendan

(CVS log entry.)

Undoing this extension is a separate proposal from fixing this bug, unless I am missing something here that requires removing the extension. We don't have to sort this out here. New bug? I will review a focused patch quickly.

/be
Comment 9 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-03 18:29:24 PST
Yeah, that was it.  I'll see what I can do.  I think, with some effort, I can special-case those bits in the test that was detecting this instance of not allowing keywords in this one place.  Back in a bit with that change, I hope.
Comment 10 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-03 19:06:36 PST
Created attachment 516780 [details] [diff] [review]
diff -w without function name bits
Comment 11 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-03 19:14:28 PST
Filed bug 638667 for the function-name change.
Comment 12 Brendan Eich [:brendan] 2011-03-04 18:26:29 PST
Comment on attachment 516780 [details] [diff] [review]
diff -w without function name bits

Does this buy us spec conformance wins with sputnik or test262? It's not good for much more, but we may as well get it in and start breaking all the scofflaw JS content out there.

/be
Comment 13 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-06 17:40:04 PST
Yeah, looks like it's test262 (and based on the test names, Sputnik before it) wins here.  Will blog, too, after I land this.
Comment 14 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-07 12:02:34 PST
http://hg.mozilla.org/tracemonkey/rev/9abe76d26196
Comment 15 Chris Leary [:cdleary] (not checking bugmail) 2011-03-31 15:17:05 PDT
http://hg.mozilla.org/mozilla-central/rev/9abe76d26196
Comment 16 Jorge Villalobos [:jorgev] 2011-05-31 17:03:53 PDT
This should be mentioned in the Firefox 5 compatibility docs since it can and will break a number of add-ons.
Comment 17 Tyler Breisacher 2011-06-07 19:14:19 PDT
https://developer.mozilla.org/en/JavaScript/Reference/Reserved_Words should be updated as well.
Comment 18 Eric Shepherd [:sheppy] 2011-08-04 10:26:26 PDT
Hm... was this in Firefox 5? For some reason, it's flagged in our doc spreadsheet as being for Firefox 7.
Comment 19 Jeff Walden [:Waldo] (remove +bmo to email) 2011-08-04 11:02:03 PDT
Pretty sure this was 5 and changed not at all in 7.
Comment 20 Eric Shepherd [:sheppy] 2011-08-04 11:02:48 PDT
OK, this had been documented already for Firefox 5, but was for some reason on our list for Firefox 7. I've checked the docs, and things are good.

Note You need to log in before you can comment on or make changes to this bug.