379566 - Keywords after get/set

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

In the same was as SpiderMonkey allows keywords to be used as property or function names, it should allow the keywords with new style getter or setter.

I.e. SM should allow:

var obj = { get in() { return this.for; }, set in(value) { this.for = value; } }

Currently both get in and set in generates syntax error:

js> var obj = { get in() { return this.for; }, set in(value) { this.for = value; } }
typein:11: SyntaxError: missing : after property id:
typein:11: var obj = { get in() { return this.for; }, set in(value) { this.for = value; } }
typein:11: ................^

Comment 1

[Igor Bukanov]

Attachment: Fix v1

The fix and bug 379482 raised a question:

Why 
  {2: 0, 3.14: 0, "an str": 0 } 
is allowed while
  {get 2() { }, get 3.14() { }, get "an str"() { } }
is not?

Perhaps for consistency get/set should allow the same tokens that the property name accepts? Then get/set would have the same power as old getter/setter syntax.

Comment 2

[Igor Bukanov]

Attachment: diff -b version of fix v1

Comment 3

[Jeff Walden [:Waldo]]

(In reply to comment #1)
> Perhaps for consistency get/set should allow the same tokens that the property
> name accepts?

It's not as pretty, but the inability to create object literals with getters for non-name properties hurts.

Also, note Brendan's comment of 2006-12-12 15:45 at <http://developer.mozilla.org/es4/proposals/getters_and_setters.html> which notes that ES4 intends this (and even notes that SpiderMonkey supports it :-) ).

Comment 4

[Igor Bukanov]

(In reply to comment #3)
> (In reply to comment #1)
> > Perhaps for consistency get/set should allow the same tokens that the property
> > name accepts?
> 
> It's not as pretty, but the inability to create object literals with getters
> for non-name properties hurts.

Right, 
  { get 'ha ha'() {}, set 2.718281828459045() {} } 
is definitely worse then
  { 'ha ha' getter: function() {}, 2.718281828459045 setter: function() {} }

Moreover, the older syntax also allow to reuse the getter/setter through sharp variables while the new one does not if one ignores toSource implementation attempt to do so:

js> var obj = {a getter: #1=function() {return 10;}, b getter: #1#}
js> obj.a + obj.b
20
js> uneval(obj)
({get a #1=() {return 10;}, get b #1#})
 
So in SpiderMonkey the older getter/setter syntax should continue to exist. Given its much bigger flexibility I wonder what was the main reason to reject the old getter/setter syntax for ES4?

Comment 5

[Igor Bukanov]

I filed bug 379630 about the shared getter problem.

Comment 6

[Brian Crowder]

> So in SpiderMonkey the older getter/setter syntax should continue to exist.
> Given its much bigger flexibility I wonder what was the main reason to reject
> the old getter/setter syntax for ES4?

I actually wonder about this, too.  "get/set" syntax seems to have a lot of problems, for not much payoff.  And what does this mean for other JS implementations?  Is getter/setter syntax also in the ES4 spec (doesn't seem to be on my cursory examination)?  What do they fall back to for these odd cases?

Comment 7

[Brendan Eich [:brendan]]

See http://www.mozilla.org/js/language/js20/rationale/syntax.html#getter-setter -- the old docs do not seem to specify getters and setters in object initialisers, only getter and setter function definitions. The rationale for function get foo over getter function foo was, as noted, aesthetic.

I'll get ES4's spec fixed.

I doubt we can resurrect getter: and setter: -- I'm also not sure we can get rid of them in SpiderMonkey, at least before Mozilla 2.

/be

Comment 8

[Igor Bukanov]

(In reply to comment #7)
> See http://www.mozilla.org/js/language/js20/rationale/syntax.html#getter-setter
> -- the old docs do not seem to specify getters and setters in object
> initialisers, only getter and setter function definitions. The rationale for
> function get foo over getter function foo was, as noted, aesthetic.

But that js20 document does not even mention defining getters/setters in object initializers. Even if one forgets about sharp variables, there the older syntax allows to define the getter function one and refer to it several times while the new syntax requires to write code a function for each getter:

var storage = {}
function g(id) { return storage[id]; }

// old syntax
var obj = { a getter: g, b getter: g }

// new syntax
var obj = { get a() { return g('a'); }, get b() { return g('b') } }

I would understand if aesthetic considerations would suggest to replace "a getter:" via "get a:" like in:

var obj = { get a: g, get b: g }

It indeed looks better while allowing to do anything that "name getter" allowed, but that "get a() { }" syntax also removes the functionality. 

So I restate the question:

What was the rational for the new syntax in the property initializers?

> I doubt we can resurrect getter: and setter: -- I'm also not sure we can get
> rid of them in SpiderMonkey, at least before Mozilla 2.

As bug 379630 and bug 358594 demonstrates, if SM will continue to support sharp variables, one has to continue to either support the old syntax or modify "get name" syntax to allow for sharp variables.

Comment 9

[Igor Bukanov]

I guess it is too late to propose for "get a() {...}" to be just a syntax sugar for "get a: function() { .. }"? 

Comment 10

[Jesse Ruderman]

I like the idea in comment 9.

Comment 11

[Igor Bukanov]

In reply to crowder comments on IRC:

> Is that ts->flags pattern common? Turning on the KEYWORD_IS_NAME bit and then turning it off without regard to what it was before?

The KEYWORD_IS_NAME is just for single token, it should always be set back after GetToken or similar. But that suggests to add an assert at least.

Comment 12

[Brendan Eich [:brendan]]

I wasn't involved except to hear Waldemar tell me that getter: lost to get foo -- IIRC he did mention a concern about forward compatibility, but I forget what it was (I wasn't convinced, that much I remember -- either syntax can be injected as a new form, which was not legal before; get or getter is not reserved in all contexts, etc.).

I've updated the ES4 getters and setters proposal and I'll report back when we've had time to go over it in TG1.

/be

Comment 13

[Brendan Eich [:brendan]]

Comment on attachment 263572 [details] [diff] [review]
Fix v1

r=me, thanks.

/be

Comment 14

[Igor Bukanov]

I committed the patch from comment 1 to the trunk:

Checking in jsparse.c;
/cvsroot/mozilla/js/src/jsparse.c,v  <--  jsparse.c
new revision: 3.280; previous revision: 3.279
done

Comment 15

[Igor Bukanov]

Nominating for js 1.7 - correct me if I am wrong.

Comment 16

[Biju]

(In reply to comment #15)
> Nominating for js 1.7 - correct me if I am wrong.
I would like to see regression Bug 381303 and Bug 381304 fixed 

Comment 17

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_7/extensions/regress-379566.js,v  <--  regress-379566.js
initial revision: 1.1

Comment 18

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0 linux/mac*/windows.