ES5's rules for object literals: ObjectLiteral : { } { PropertyNameAndValueList } { PropertyNameAndValueList , } PropertyNameAndValueList : PropertyAssignment PropertyNameAndValueList , PropertyAssignment PropertyAssignment : PropertyName : AssignmentExpression get PropertyName ( ) { FunctionBody } set PropertyName ( PropertySetParameterList ) { FunctionBody } PropertyName : IdentifierName StringLiteral NumericLiteral PropertySetParameterList : Identifier We support |PropertyName: AssignmentExpression|, and we support |[gs]et IdentifierName|, but we don't support |[gs]et (String|Numeric)Literal|. Needs to happen to be able to get rid of |x getter:| syntax.

Note that NumericLiteral is any number literal, possibly fractional, possibly exponential (even with positive or negative exponent), not just DecimalIntegerLiteral.

...which means we also have to handle { 3.141592654: "π" }.

Hm, so we already supported non-DecimalInteger numeric literals as property names (if not as getter/setter names). That makes things a little simpler.

Curious --- why the initialization of funAtom in FunctionDef? That's definitely set by the first 'if'; was some compiler too dumb to see that?

(In reply to comment #3) > Created an attachment (id=417717) [details] > Patch > > Hm, so we already supported non-DecimalInteger numeric literals as property > names (if not as getter/setter names). That makes things a little simpler. Yeah; that's tested in js/src/tests/ecma_5/strict/11.1.5.js.

Shouldn't those tests go in js/src/tests/ecma_5/Expressions/11.1.5.js? We're just implementing what the standard specifies.

(In reply to comment #4) > why the initialization of funAtom in FunctionDef? I think that's a slight bit of patch-cest, anticipating a change much later in my mq where I make |{ set foo foo() { } }| (you don't really want to know) a syntax error.

(In reply to comment #6) > Shouldn't those tests go in js/src/tests/ecma_5/Expressions/11.1.5.js? I don't see an ecma_5/Expressions directory in TM or m-c. (And to be clear, I can remove the patch-cest if desired, don't think it hurts as is tho.)

What's a patch-cest? If it's what it sounds like, the taboo against it stands :-/. /be

That hunk more properly fit in the patch atop this one in my mq. I've moved it out of here and, incidentally, moved the declaration nearer to point of first use; it's probably better moving anyway.

Yes, please do move declarations to nearest dominating position and initialize 'em -- the old C heritage with a thicket of uninitialized vars makes it easy to miss an initializer (not to mention a var). /be

(In reply to comment #9) > What's a patch-cest? If it's what it sounds like, the taboo against it stands > :-/. Little bits like whitespace cleanup I've decided not to complain

[koff koff] ... about because otherwise they'd never get done; no reason to douse someone's enthusiasm by making them do a bunch of dotting and crossing. But stuff with semantic significance... no cest is best.

(In reply to comment #8) > (In reply to comment #6) > > Shouldn't those tests go in js/src/tests/ecma_5/Expressions/11.1.5.js? > > I don't see an ecma_5/Expressions directory in TM or m-c. Yeah, we'll need to make one.

Frankly, I don't think much of sub-organizing tests where distinctions don't map to source directories (as with SpiderMonkey where ~everything's in js/src/*); it just results in angst-fests about where they should go. Even the ecma_5 and friends (especially js1_*) distinction is lost on me -- JS is usually just JS, and subdividing by version is just needless complexity (especially when we stick new tests in the most recent directory regardless whether they're relevant to that version's changes).

Comment on attachment 424883 [details] [diff] [review] Updated Code changes look good to me; the tests need to go in the right place.

Comment on attachment 425129 [details] [diff] [review] . Looks good, but (mea culpa) you'll need to fix up the jstest.list changes. There is now a js/src/tests/ecma_5/extensions directory, so the patches to jstests.list and shell.js will need to be changed. And doesn't js/src/tests/ecma_5/Expressions need to be added to js/src/tests/ecma_5/jstests.list?

Is this something worth mentioning at https://developer.mozilla.org/en/Upcoming_Firefox_features_for_developers? I'm not experienced enough with JavaScript to tell, if it should be, myself.

Yes, probably, should have thought to add the keyword.

Appropriate notes have been added: https://developer.mozilla.org/en/JavaScript/Reference/Operators/Special_Operators/set_Operator https://developer.mozilla.org/en/JavaScript/Reference/Operators/Special_Operators/get_Operator

Oops, wrong bug. This hasn't been done yet, I think.

Actually, the same note that fixes the other bug fixes this one too, so this really is doc-complete.