Discuss
  • Map etc and [[Call]]

Map etc and [[Call]]

# Erik Arvidsson (11 years ago)

All of the new constructors, Map, Set, WeakMap and WeakSet should fail when called as a function unless this already has a certain internal property, ie [[MapData]]. This is so that sub classing works as expected.

However, all JS engines that supports Map, Set and WeakMap are future hostile and they all return a new instance when called as a function. For example, here is the V8 code:

function MapConstructor() {
  if (%_IsConstructCall()) {
    %MapInitialize(this);
  } else {
    return new $Map();
  }
}

I understand that fully supporting @@create requires a lot of work but until then we should at least throw when called as function to allow us to get out of this mess eventually.

# Rick Waldron (11 years ago)

It's painful, but I agree. This will prevent a build up of incorrect code that may hold @@create semantics hostage.

# Mark S. Miller (11 years ago)

I like the general idea. But when actually called simply as a function, this would be unpleasant. Understanding that any ES5 behavior will be a compromise to prepare the ground for ES6, I suggest this variant:

function MapConstructor() {
  if (%_IsConstructCall()) {
    %MapInitialize(this);
  } else if (this === void 0) {
    return new $Map();
  } else {
    throw ....;
  }
}

I already have a lot of code that simply calls WeakMap() without saying "new". I suspect that others will too by the time they see v8's new built-in WeakMaps.

# Brandon Benvie (11 years ago)

I wonder if the current spec language is a remnant of before @@create was added to the spec. It should be possible to detect when the constructors are called with a receiver that is not an initializing instance and simply create a new instance. Pseudo code:

function Map(...args){
   if (!IsObject(this) || !%HasMapData(this)) {
     // got a primitive, the global object, or some random object like from `Map.call({})`
     return new Map(...args);
   } else if (!%IsInitializedMap(this)) {
     // A legit Map but has uninitialized [[MapData]]
     %MapInitialize(this);
   } else {
     // [[MapData]] is present but is already initialized
     throw TypeError("Attempted to reinitialize a Map");
   }
}

The main point of introducing @@create (I think) was to allow for distinguishing between initializing, initialized, and everything else.

# Brendan Eich (11 years ago)

Brandon Benvie wrote:

I wonder if the current spec language is a remnant of before @@create was added to the spec. It should be possible to detect when the constructors are called with a receiver that is not an initializing instance and simply create a new instance. Pseudo code:

function Map(...args){
  if (!IsObject(this) || !%HasMapData(this)) {
    // got a primitive, the global object, or some random object like from `Map.call({})`
    return new Map(...args);
  } else if (!%IsInitializedMap(this)) {

Nit: else after return ;-).

More substantive: Mark's suggestion seems better because it throws on random wrong this but supports strict calls by unqualified name.

# Mark S. Miller (11 years ago)

On Tue, Jul 16, 2013 at 4:56 PM, Brendan Eich <brendan at mozilla.com> wrote:

More substantive: Mark's suggestion seems better because it throws on random wrong this but supports strict calls by unqualified name.

Nit: All calls. The non-coercion of this depends only on the strictness of the callee, not the caller.

# Brendan Eich (11 years ago)

Mark S. Miller wrote:

Nit: All calls. The non-coercion of this depends only on the strictness of the callee, not the caller.

Oh right, and Map, etc. are built-in so as-if-self-hosted-strict in this regard.

# Allen Wirfs-Brock (11 years ago)

On Jul 16, 2013, at 4:10 PM, Erik Arvidsson wrote:

All of the new constructors, Map, Set, WeakMap and WeakSet should fail when called as a function unless this already has a certain internal property, ie [[MapData]]. This is so that sub classing works as expected.

Yes, indeed:

let m = Map();  //I think m is a map

is specified as throwing a TypeError.

To implement it otherwise is diverging from the spec.

Using constructors as callable factories is pretty hostile to subclasses that need to do super calls to superclass constructors. While it is possible to carefully code a constructor function that it can work as both a factory and an initializer there are plenty of subtleties (and/or performance traps) that are likely to trip-up everyday JS programmers. For that reason, I expect that callable constructors that allocate is going to be quickly identified as a ES6 anti-pattern. The natural thing to do with ES6 classes is to always use new and to code the constructor function to only act as an instance initializer. In support of that pattern, Map and friends only allocated when invoked via new.

We will be talking about this some more at next week's TC39 meeting.

However, all JS engines that supports Map, Set and WeakMap are future hostile and they all return a new instance when called as a function. For example, here is the V8 code:

function MapConstructor() {
 if (%_IsConstructCall()) {
   %MapInitialize(this);
 } else {
   return new $Map();
 }
}

Yes, this is wrong because if this was super called from a subclass constructor it would return a new Map instance to the subclass constructor rather than initializing (if possible) the subclass provided instance as Map.

I'd don't know much about the V8 self-hosting mechanism, but it looks to me like it should be sufficient to code this as:

function MapConstructor() {
   %MapInitialize(this);
}

assuming that %MapInitialize is smart enough to throw on undefined and other this values that are not appropriate to initialize as Maps.

I understand that fully supporting @@create requires a lot of work but until then we should at least throw when called as function to allow us to get out of this mess eventually.

For background here is how new Foo() is now supposed to work:

let newObj = Foo[@@create]();  //allocate an initialized Foo instance.
Foo.call(newObj);                         //initialize the newObject

It's not very complicated.

# Brandon Benvie (11 years ago)

On 7/16/2013 4:56 PM, Brendan Eich wrote:

Mark's suggestion seems better because it throws on random wrong this but supports strict calls by unqualified name.

I don't think that it'd be nice if Map() could throw or not throw depending on strictness of the caller. Working around that requires something like:

 function Map(...args){
   if (this === undefined || this === %CallerGlobal()) {
     return new Map(...args);
   } else if (!%HasMapData(this)) {
     throw new TypeError("Not a Map");
   } else if (%HasInitializedMapData(this)) {
     throw new TypeError("Map is already initialized");
   }
   %MapInitialize(this);
 }

(V8's implementation is also incorrect in that it doesn't throw when you do Map.call(new Map)) (so is SM's)

# Brandon Benvie (11 years ago)

On 7/16/2013 5:09 PM, Mark S. Miller wrote:

Nit: All calls. The non-coercion of |this| depends only on the strictness of the callee, not the caller.

Ah yes, right! Nevermind on the complaint in my last message.

# Erik Arvidsson (11 years ago)

I completely agree with you Allen. new-less construct is an anti pattern.

On Tue, Jul 16, 2013 at 5:13 PM, Allen Wirfs-Brock <allen at wirfs-brock.com> wrote:

It's not very complicated.

The semantics and the spec are simple but...

...it does require support for symbols, shared symbols between realms, updates to new without any perf regression.

# Allen Wirfs-Brock (11 years ago)

On Jul 16, 2013, at 4:41 PM, Mark S. Miller wrote:

I like the general idea. But when actually called simply as a function, this would be unpleasant. Understanding that any ES5 behavior will be a compromise to prepare the ground for ES6, I suggest this variant:

function MapConstructor() {
  if (%_IsConstructCall()) {
    %MapInitialize(this);
  } else if (this === void 0) {

this test isn't sufficient is you really want to support constructors as callable factories. Consider an namespace object (or a module) such as:

let ES6 = {Map, WeakMap, Set};
ES6.Map();

I already have a lot of code that simply calls WeakMap() without saying "new". I suspect that others will too by the time they see v8's new built-in WeakMaps.

We need to discourage leaving out the new, rather than trying to preserve the small amount (in terms of web scale) of existing code that uses Map/Set/WeakMap as callable factories.