Closed Bug 447713 Opened 12 years ago Closed 12 years ago

Remove the import/export functionality from spidermonkey

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.1a2

People

(Reporter: graydon, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch to delete the legacy code (obsolete) — Splinter Review
As Brendan suggested, the "import" and "export" functionality in spidermonkey is legacy stuff from ancient netscape days and is, among other things, consuming a bit of property-attribute space we'd like to free up for properly modeling "const".

This patch removes, as delicately as possible, the legacy functionality. I'd appreciate careful review from anyone who feels qualified.
Attachment #331022 - Flags: review?(brendan)
Comment on attachment 331022 [details] [diff] [review]
patch to delete the legacy code

>+++ b/js/src/jsemit.cpp	Wed Jul 23 16:52:50 2008 -0700
>@@ -2393,11 +2393,7 @@
>                        CG_OFFSET(cg) - pn2->pn_offset) < 0) {
>         return JS_FALSE;
>     }
>-    if (!pn->pn_atom) {
>-        JS_ASSERT(op == JSOP_IMPORTALL);
>-        if (js_Emit1(cx, cg, op) < 0)
>-            return JS_FALSE;
>-    } else {
>+    if (pn->pn_atom) {
>         if (!EmitAtomOp(cx, pn, op, cg))
>             return JS_FALSE;
>     }

No degree of freedom allowing null pn->pn_atom remains, so this code should simplify to one line:

    return EmitAtomOp(cx, pn, op, cg);

(no need to assert pn->pn_atom is not null, we prefer clean null deref crash in general).

/be
Attachment #331022 - Attachment is obsolete: true
Attachment #332621 - Flags: review?(brendan)
Attachment #331022 - Flags: review?(brendan)
Comment on attachment 332621 [details] [diff] [review]
freshened, reformatted patch with suggested fix

Thanks, r=me.

/be
Attachment #332621 - Flags: review?(brendan) → review+
Pushed to m-c in revision 216fff17c130.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
I'm going to pass creating a test for this. push back if you disagree.
Flags: in-testsuite-
Flags: in-litmus-
Target Milestone: --- → mozilla1.9.1a2
I've been running with this exclusion file for trunk but failed to check it in. Here it is including new exclusions for import|export tests:

e4x/Regress/regress-361451.js
ecma_2/Exceptions/lexical-010.js
ecma_2/Exceptions/lexical-022.js
ecma/LexicalConventions/7.4.3-3-n.js
js1_5/decompilation/regress-349484.js
js1_5/extensions/regress-355622.js
js1_5/extensions/regress-418730.js
js1_5/GetSet/regress-353264.js
js1_5/Regress/regress-249211.js
js1_5/Regress/regress-350692.js
js1_5/Regress/regress-354924.js
js1_5/Regress/regress-362583.js
js1_7/extensions/regress-353214-01.js
js1_7/lexical/regress-346642-03.js

/cvsroot/mozilla/js/tests/spidermonkey-n-1.9.1.tests,v  <--  spidermonkey-n-1.9.1.tests
initial revision: 1.1

mozilla-central: changeset:   16534:ea3a755dbab5
You need to log in before you can comment on or make changes to this bug.