229006 - JS parser has problems parsing i++ when line number greater than 2^16

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The JS parser has problems parsing i++ in a for loop, and perhaps other things,
when the line number is greater than 2^16.  (Asa ran into this on bugzilla
mass-change forms -- the "Check All" and "Uncheck All" bugs are document.written.)

Steps to reproduce:
  load attached testcase

Actual results:
  JS error on console:
Error: missing ) after for-loop control
Source File: file:///home/dbaron/webtest/docwrite2.html
Line: 65540, Column: 20
Source Code:
for (var i=0; i<2; i++) {

Expected results:
  no error

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: testcase

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch (hack!)

This fixes the problem, but brendan isn't crazy about it. :-)

Comment 3

[Brendan Eich [:brendan]]

I'll try for a better patch, but no promises -- dbaron's may land (perhaps with
some macro magic to consolidate the awful truth).

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: hack dressed up via a macro

I'm inclined to check this in and move on.  Fancier fix ideas I'll record here
but not bother with:

1.  Store line number delta relative to first line of script, not file or url
relative (absolute) line number, in JSTokenPtr.lineno (rename it to linedelta
or some such name).

2.  Use a uint32 for JSTokenPtr and split its bits dynamically, according to
how many lines are in the script (or file/url, if 1 isn't done).  We'd need to
know how many lines total up front, or else keep revising the number of bits as
we go (somehow).  Note that with eval, it's easy to get a script contained on
one very long line, so the JSTokenPtr.index field might easily need more bits
than 16.

If anyone has a brainstorm, speak up.

/be

Comment 5

[Mike Shaver (:shaver emeritus)]

Comment on attachment 138324 [details] [diff] [review]
hack dressed up via a macro

DO NOT LOOK INTO MACRO WITH REMAINING EYE.

r=shaver.

Comment 6

[Brendan Eich [:brendan]]

Fixed.

/be

Comment 7

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-229006.js

Thanks to dbaron.

Comment 8

[Bob Clary [:bc] (inactive)]

js1_5/Regress/regress-229006.js checked in.

Comment 9

[Bob Clary [:bc] (inactive)]

verified fixed.