Closed Bug 376052 Opened 15 years ago Closed 15 years ago

Anonymous function expression statement API vs. E262-3 compatibility

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha4

People

(Reporter: kangas, Assigned: brendan)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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).
Duplicate of this bug: 376048
-> 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 }
Assignee: nobody → general
Component: General → JavaScript Engine
OS: Linux → All
Product: Firefox → Core
QA Contact: general → general
Hardware: PC → All
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?
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
Assignee: general → brendan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Target Milestone: --- → mozilla1.9alpha4
Summary: Javascript's anonymous function constructor works incorrectly. → Anonymous function expression statement API vs. E262-3 compatibility
Attached patch fix (obsolete) — Splinter Review
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
Attachment #260793 - Flags: review?(igor)
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.
Attachment #260793 - Flags: review?(igor) → review+
Attached patch fix, v2Splinter Review
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
Attachment #260793 - Attachment is obsolete: true
Attachment #260868 - Flags: review?(igor)
Comment on attachment 260868 [details] [diff] [review]
fix, v2

Nice and matches options() nicely.
Attachment #260868 - Flags: review?(igor) → review+
Fixed on trunk:

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

/be
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
See bug 377433 for the followup action.

/be
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-376052.js,v  <--  regress-376052.js
initial revision: 1.1
Flags: in-testsuite+
verified fixed 1.9.0 linux/mac* 2007-05-05 shell.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.