421433 - Error.prototype properties are JSPROP_ENUMERATE

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brendan Eich [:brendan]]

Contrary to ECMA-262 Edition 3, section 15 intro and 15.11.4.

Easy fix, good for ACID3? Dunno, but trivially safe. Brian, can you do the deed?

/be

Comment 1

[Brian Crowder]

Attachment: don't enum these props on the prototype

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 308484 [details] [diff] [review]
don't enum these props on the prototype

Seems not to hit exn_resolve, need that too.

diff -pu8 is best, old style is strangely nostalgic, tho ;-).

/be

Comment 3

[Brian Crowder]

I don't think we do want to remove the JSPROP_ENUMERATE setting from exn_resolve, since we do want non-prototype properties to be enumerable, don't we?  Should I be special-casing in exn_resolve for the prototype?

Comment 4

[Brendan Eich [:brendan]]

Good question -- it's an efficiency bug that exn_resolve does not cast out JSRESOLVE_ASSIGNING with an early return. It is trying to bind instance props such as message, fileName, lineNumber on demand, even though the class init code bothered to put default-valued props of the same names (not stack, though) on the *Error.prototype objects.

The reason it's doing this is to reflect the native JSExnPrivate member back into JS, and it can't do this via prototype getters without making a new string (for the string-valued props) on each get.

But the effect is to make a property in each instance, on demand. ES3 says there are no per-instance props, though:

15.11.5 Properties of Error Instances
Error instances have no special properties beyond those inherited from the Error prototype object.

I think the spec is kind of broken here, but no matter. It allows additional properties.

But this brings us back to the current bug. That we shadow the uselessly-default-valued proto-props (save for stack, which is always per-instance, not in any prototype) in order to avoid making extra strings, should not change the enumerability per ES3 of at least the 'message' prop.

So I'd still think non-enumerable properties are right for the per-instance ones too.

/be

Comment 5

[Brian Crowder]

This is a spec bug that probably needs more love than I can give.