230047 - Fix old glitches in SpiderMonkey's automatic semicolon insertion

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brendan Eich [:brendan]]

They violate ECMA, and they don't make sense in light of the original design,
upon which ECMA was based, which had this goal: for those statements that do not
end in statement non-terminals (primitive-ending statements), which all require
semicolon termination in C or Java, insert a semicolon if one or more newlines
separates such a statement from subsequent tokens.  There are some restrictions
to do with postfix ++/-- and return; vs. return value.

(The list of statements in JS ending in primitives is: break, continue,
do-while, return, throw, var, the expression statement, and the empty statement
-- but note that per ECMA, automatic semicolon insertion must never insert an
empty statement.)

Patch anon.

/be

Comment 1

[Brendan Eich [:brendan]]

Attachment: proposed fix

- Eliminate WellTerminated by coalescing its call sites to the end of Statement

and inlining it there.	This adds a tiny bit of overhead in some cases, due to
the ON_CURRENT_LINE check, which can be proven redundant for cases such as the
debugger statement.  But the cost should be in the noise, and code size wins.

- Remove bogus lastExprType argument to WellTerminated and eliminate its only
use, in parsing an expression statement.  Also eliminate the WellTerminated
check after a function "statement" (which could be a top-level declaration, or
an instance of our function statement extension, e.g. 'if (x) function f(){}',
neither of which should require semicolon termination).

The intent of the lastExprType parameter, the WellTerminated call from the
TOK_FUNCTION case, the call from the default expression-statement case, and the

#if JS_HAS_LEXICAL_CLOSURE code in WellTerminated appears to have been to allow

'function f(){} function g(){}' for JS versions < 1.2, and to forbid it as an
error otherwise, requiring a semicolon in between the function declarations.
But in fact ECMA and good sense should allow as many function declarations on
a line as you please, optionally followed by other statements, and without
requiring semicolons after function declarations.

Since this change removes an error case, I don't expect any incompatibility
bugs to be reported.

- Coalescing/inlining WellTerminated meant removing its call from MatchLabel,
called by the break and continue cases in Statement.

- The debugger keyword could be used without a semicolon terminator to make it
a statement, as in 'debugger 1 + 2;', parsed as a debugger statement followed
by an expression statement -- which seems quite wrong (ECMA reserves debugger
but does not specify its usage).

This bug arose due to the early return at the end of the TOK_DEBUGGER case.
Although that case called WellTerminated, because it returned rather than broke

from the main switch, it failed to match an explicit semicolon terminator (if
present), making 'debugger; 1 + 2;' parse as three statements, with the first
semicolon becoming an empty statement (contrary to ECMA automatic semicolon
insertion rules).

Comment 2

[Brendan Eich [:brendan]]

Testsuite retest list:

# Retest List, smdebug, generated Sun Jan  4 11:50:20 2004.
# Original test base was: All tests.
# 1123 of 1123 test(s) were completed, 7 failures reported.
ecma_2/Exceptions/function-001.js
ecma_3/RegExp/regress-188206.js
ecma_3/RegExp/regress-228087.js
js1_2/function/function-001-n.js
js1_3/Script/function-001-n.js
js1_3/regress/function-001-n.js
js1_5/Array/regress-101964.js

I believe ecma_2/Exceptions/function-001.js and js1_[23]/*/function-001-n.js are
in error, as was the code removed by this bug's patch that made it an error to
put two or more function declarations on one line.

Aside: I thought we fixed 228087 -- Phil, is there still a bug open there?

/be

Comment 3

[Brendan Eich [:brendan]]

Fix is in.

/be