38512 - monkey throws EvalError instead of TypeError on o.eval()

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Mike McCabe]

Given

o.eval("4 + 4")

JS responds with an EvalError

2: EvalError: function eval must be called directly, and not by way of a
function of another name.

instead of a TypeError, which would be more appropriate, given that eval isn't a
property of o.

(I have an problem for which Object.prototype.eval would have been handy, but it
didn't make it into the standard...)

Comment 1

[Phil Schwartau]

Confirmed -

js> var o = new Object();
js> o.eval("1+1");
3: EvalError: function eval must be called directly, and not by way of a 
function of another name.

Comment 2

[rogerl (gone)]

Attachment: Change exception type & fix other formatting issue.

Comment 3

[rogerl (gone)]

[SPAM] bug triage for nsbeta3

Comment 4

[rogerl (gone)]

[From email generated after the above patch]

<rogerl>
    So JSEXN_EVALERR is unused with the patch.  ECMA-262 Ed. 3 chapter 6 says 
that "An implementation is not required to detect EvalError.  If it chooses not 
to detect
    EvalError, the implementation must allow eval to be used indirectly and/or 
allow assignments to eval."  So the patch is not sufficient: obj_eval must not 
report any error
    (neither EvalError nor TypeError) if someone does: 

    o = new Object; 
    o.eval = eval; 
    o.eval("1+2");

The problem is that we've defined eval as a property of Object rather than of 
just the global object (it's there also). In mccabe's test case : 

  o = new Object; 
  o.eval("1+2"); 

  I think we should be returning "TypeError: o.eval is not a function" rather 
than getting into the obj_eval code at all. I can't simply take eval out of the 
object class though since we
  need it there for 1.2 compatibility etc. The trick is to distinguish getting 
into obj_eval via mccabe's example, which is a TypeError, or your example which 
is an EvalError.

<Brendan>
We could do that by testing whether the obj parameter (this in JS parlance) 
hasOwnProperty('eval') and is not identical to Object.prototype.  Ugly, but 
do-able. 

Comment 5

[Mike McCabe]

Supporting obj.eval came up in a recent ECMA-262 post from Microsoft.  I suspect
we chucked it by accident as part of outlawing 'evil = eval; evil('4+4');'

What's the reasoning behind not supporting it?

Comment 6

[Robert Williams]

I concur with Mike McCabe's comment.  Why isn't this supported (other than that 
it was deprecated in NS 4.*, etc)

Why include o.eval?  How do I do this without it?
<FORM NAME="authorize" method=get ACTION="foo.htm">
...
	for (var i = 0; i < pairs.length; i++)
	{
	  // Split at "=", into name and value pair
	  var nameVal = pairs[i].split("=");
	  //create hidden variable in form 
	  document.write('<INPUT TYPE=hidden NAME=' + nameVal[0] + '>');
	  // uses eval to name the new form member 
	  document.authorize.eval(nameVal[0]).value = unescape(nameVal[1]);
...

Comment 7

[Brendan Eich [:brendan]]

Robert: you don't need eval to compute a property name and get its value.  JS
equates o.p and o['p'] or o[expr] where expr evaluates to 'p'.  So you could use

          document.authorize[nameVal[0]].value = unescape(nameVal[1]);

/be

Comment 8

[Brendan Eich [:brendan]]

mccabe: IIRC, ECMA Ed. 3 requires eval to be a direct (hasOwnProperty) property
of the global object.  Someone patched js_InitObjectClass to copy eval to global
from Object.prototype, citing chapter and verse.

I think we have some choices:

1.  Make it a strict warning to invoke obj_eval via JSOP_CALL.

2.  Compile JSOP_EVAL for all calls to a function named eval, whether by
unqualified name or by a qualified property reference (o.eval, o['eval']).

It seems to me we do not want to complain if o.eval(s) invokes an eval inherited
from a prototype of o -- that's legit in JS pre-ECMA, provided the eval method
that's inherited comes from Object.prototype or otherwise has as its value the
native method obj_eval.

Comments?

Comment 9

[Mike McCabe]

I'll guess that those interested in future optimization would probably prefer
not to see o['eval'].  Most likely not worth worrying about, though.

Comment 10

[Phil Schwartau]

This bug is still happening: 

js> var o = new Object();
js> o.eval("4 + 4");
3: EvalError: function eval must be called directly, and not by way of a 
function of another name.

Comment 11

[Brendan Eich [:brendan]]

Attachment: make JSMSG_INDIRECT_CALL be a strict warning

Comment 12

[rogerl (gone)]

r=rogerl, want me to ride the check-in etc?

Comment 13

[Mike Shaver (:shaver emeritus)]

sr=shaver, though I'm with mccabe on wanting indirect call of eval to be illegal
for ECMA versions.

Comment 14

[Brendan Eich [:brendan]]

ECMA-262 Edition 3 doesn't require implementations to throw EvalError, see the
short, section-less Chapter 16.  It does say an implementation that doesn't
throw EvalError must allow assignment to eval and indirect calls of the eval
native method.

With options strict and werror, we can have it both ways.  But since our default
version satisfies JSVERSION_IS_ECMA, and there's too much old script out there
that wants to call o.eval, our default must be lenient.

Fix checked in.

/be

Comment 15

[Phil Schwartau]

This is what I'm now getting in the debug and optimized JS Shells.
Based on my understanding of this bug, I'm marking it VERIFIED.

If for some reason I have misunderstood, and these results are not
correct, please re-open the bug. 


-----------------------  STRICT OPTION ON ------------------------------------
[/d/JS_TRUNK/mozilla/js/src/WINNT4.0_DBG.OBJ] ./js
js> var o = new Object();
js> o.eval("4 + 4");
2: strict warning: function eval must be called directly, 
and not by way of a function of another name.
8



-----------------------  STRICT OPTION OFF -----------------------------------
[/d/JS_TRUNK/mozilla/js/src/WINNT4.0_DBG.OBJ] ./js -W
js> var o = new Object();
js> o.eval("4 + 4");
8

Comment 16

[Phil Schwartau]

I have corrected the JS test suite to acknowledge that indirect eval is legal:


DEPRECATED THESE:
                   js/tests/ecma_2/Exceptions/expression-020.js
                   js/tests/js1_3/misc/eval-001-n.js

RE-WROTE THESE:
                   js/tests/js1_4/Eval/eval-001.js
                   js/tests/js1_4/Eval/eval-002.js
                   js/tests/js1_4/Eval/eval-003.js

Comment 17

[Phil Schwartau]

Note: at present, Rhino is not in synch with SpiderMonkey on this issue.
That is, Rhino still does not permit indirect eval. Bug 67714 has been filed -