376052 - Anonymous function expression statement API vs. E262-3 compatibility

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[george kangas]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.10) Gecko/20070224 Red Hat/1.5.0.10-0.1.el4 Firefox/1.5.0.10
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.10) Gecko/20070224 Red Hat/1.5.0.10-0.1.el4 Firefox/1.5.0.10

At the URL given, and at several others (such as http://www.stowlake.com/web_pro/js_shell.html) a textbox is provided which accepts Javascript input, feeds it to "eval", and puts the result in another box. A nice toy!  I tried to play with the anonymous function constructor (Javascript's "lambda"), and it didn't work right.

1) Safari browser, on Macintosh: works fine;
2) Firefox browser, on Mac or linux: works wrong;
3) Mozilla browser, on Mac: "syntax error", when there is none;
4) SeaMonkey browser, on linux: works wrong.

Reproducible: Always

Steps to Reproduce:
1. Go to any of those sites, that provide a Javascript box;
2. Enter this: function(x, y) {return x+y} (100, 13);
3. Click the button which runs the code.
Actual Results:  
13 (result returned by Firefox on linux or Mac, or SeaMonkey on linux)
syntax error (Mozilla browser, on Mac)

Expected Results:  
113

Safari browser, on Mac, gives the correct answer (113).

Comment 2

[Ed Lee :Mardak]

-> core: js

george: For now, you can do (function(x, y) {}) (1, 2).

It seems like it's being split into 2 separate expressions. One function and one  comma separated expression which evaluates to the last thing in the comma separated expressions. (That's why the example returns 13.)

The ecma 262 spec defines the following..

CallExpression :
MemberExpression Arguments

MemberExpression :
FunctionExpression

FunctionExpression :
function Identifier_opt ( FormalParameterList_opt ) { FunctionBody }

Comment 3

[Ed Lee :Mardak]

Jesse mentioned bug 301693 which points out "function() {};" is *not* a valid expression.

ExpressionStatement :
[lookahead ∉ {{, function}] Expression ;

This means you /aren't/ supposed to be able to do "function() {} ();" (that's a CallExpression followed by a semicolon -> ExpressionStatement) because the lookahead should be preventing it to be an ExpressionStatement in the first place.

This is probably why "(function() {}) ();" (with starting paren) needs to be done to bypass the lookahead check. However, because "function() {};" /does/ work right now, the opposite of bug 301693 should be filed.

Brendan, does that sound right or am I completely misreading the spec?

Comment 4

[Brendan Eich [:brendan]]

See bug 303723, which was not fixed correctly.

API compatibility wants JS_EvaluateScript(... "function(){}", ...) to succeed. But this should not break the case cited in comment 0, or violate ECMA-262 Edition 3 otherwise. Confirming and taking.

/be

Comment 5

[Brendan Eich [:brendan]]

Attachment: fix

Backward compatibility (see bugs linked from comments in this bug) makes me want to fix this only if a JS option is set on the context being used for compilation. We can flip this for Gecko 1.9 if doing so doesn't break the web or Mozilla XUL scripts -- we can try to ascertain that, at any rate, in a separate bug.

/be

Comment 6

[Igor Bukanov]

Comment on attachment 260793 [details] [diff] [review]
fix

>+        case 'A':
>+            JS_ToggleOptions(cx, JSOPTION_ANONFUNFIX);
>+            break;
>+

Do we really want to have an option flag for each and every JSOPTION_* ? A more generic command line option to alter any JSOPTION_ would be better, like in -o anonfunfix.

Comment 7

[Brendan Eich [:brendan]]

Attachment: fix, v2

Per Igor's suggestion, added -o <option-name> support to the js shell instead of -A (but left older option-setting flags for testsuite compatibility).

/be

Comment 8

[Igor Bukanov]

Comment on attachment 260868 [details] [diff] [review]
fix, v2

Nice and matches options() nicely.

Comment 9

[Brendan Eich [:brendan]]

Fixed on trunk:

js/src/js.c 3.135
js/src/jsapi.h 3.141
js/src/jsparse.c 3.276

/be

Comment 10

[Brendan Eich [:brendan]]

See bug 377433 for the followup action.

/be

Comment 11

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-376052.js,v  <--  regress-376052.js
initial revision: 1.1

Comment 12

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0 linux/mac* 2007-05-05 shell.