210682 - if a line ends with 'continue' and is only terminated by a CR causes a 'SyntaxError: missing ; before statement'

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Brian Gugliemetti]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030529
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030529

if you have the following code snippet:

for (i = 0; i < 100; i++) {
   if (i % 2 == 0) continue
   this.lasti = i;
}

you get the following error: "SyntaxError: missing ; before statement"


Reproducible: Always

Steps to Reproduce:
1.
2.
3.



Expected Results:  
It above code example is valid JS syntax.  The problem lies in TokenStream. 
When the continue keyword is processed, Rhino checks for the optional label. 
Since there isn't one, it pushes back the EOL token.  However,
TokenStream.peekTokenSameLine changes the token from EOL to EOF when it pushes
back the token (member variable pushbackToken).  Finally, when Rhino calls
checkWellTerminated, the TokenStream sees the EOF, continues processing the
stream and 'forgets' it saw an EOL.

I fixed this by not converting the EOL to EOF in
TokenStream.peekTokenSameLine().  TokenStream.getToken() then checks if the
pushbackToken was EOL, and if so, validates for the TSF_NEWLINES flag.  If that
is set, it returns the EOL, otherwise it drops through and continues processing
the stream.

Comment 1

[Phil Schwartau]

cc'ing Igor -

Comment 2

[Brian Gugliemetti]

My code changes for TokenStream.java are:

    public final int peekTokenSameLine() throws IOException {
        flags |= TSF_NEWLINES;          // SCAN_NEWLINES from jsscan.h
        int result = getToken();
        this.pushbackToken = result;    // BG: Removed check for EOL
        tokenno--;
        flags &= ~TSF_NEWLINES;         // HIDE_NEWLINES from jsscan.h
        return result;
    }

and

   public final int getToken() throws IOException {
        int c;
        tokenno++;

        // Check for pushed-back token
        if (this.pushbackToken != EOF) {
            // If this isn't an EOL or it's an EOL and we are tokenizing EOL,
return it.  Otherwise
            // fall through.
            if (this.pushbackToken != EOL || (this.pushbackToken == EOL &&
(flags & TSF_NEWLINES) != 0)) {
                int result = this.pushbackToken;
                this.pushbackToken = EOF;
                return result;
            }
        }
        ....

Comment 3

[Igor Bukanov]

Attachment: fix as a patch

In the patch I made sure that pushbackToken is always set to EOF after its
value is consumed even if it would contain EOL pushed there by
peekTokenSameLine.

Comment 4

[Igor Bukanov]

I committed the patch

Comment 5

[Phil Schwartau]

Testcase added to JS testsuite:

      mozilla/js/tests/js1_5/Regress/regress-210682.js

Comment 6

[Igor Bukanov]

Targeting as resolved against 1.5R5