Branding guarantees do not match web platform classes, due to return override

Consider the following:

class Base {}
class Ex extends Base {
  #foo = 1;
  isExample() {
    return this.#foo === 1;
  }
}
const o = {};
Ex.__proto__ = function() { return o; };
new Ex;
Ex.prototype.isExample.call(o); // => true

Now consider the analogous setup with the Node and Text classes:

const o = {};
Text.__proto__ = function () { return o; };
new Text();

Text.prototype.splitText.call(o); // throws!!

or, using ES262 classes:

const o = {};
Uint8Array.__proto__ = function () { return o; };
new Uint8Array();

ArrayBuffer.isView(o); // false

This mismatch is worrying. It means that we cannot use private fields for self-hosting any classes involved in inheritance hierarchies.

I'm not sure what a good fix would be, but it seems like we should investigate what can be done here.

Can you elaborate on why the return override technique is needed to self-host classes involved in inheritance hierarchies?

Is this because these classes don't respect overriding the super class?

Text.__proto__ = function () {
  throw new Error('UNREACHABLE');
};
new Text();
// => Text ""

@ljharb

@ljharb it's not; it's only used to illustrate a "failure" of the brand checks in the example.

@jridgewell good catch, I think you are right. I've lost track of the current proposal in that regard: can we still use private fields with constructors that never call super()?

@domenic

It means that we cannot use private fields for self-hosting any classes involved in inheritance hierarchies.

@domenic then why does it mean this?

can we still use private fields with constructors that never call super()?

I believe that in a subclass, calling super is a requirement to install any private fields.

I've lost track of the current proposal in that regard: can we still use private fields with constructors that never call super()?

Under normal circumstances, no. Fields are installed in a weird step after the parent constructor returns, but before the super returns.

If these classes don't actually have a comparable super() call, then I don't know where they'd be installed.

@ljharb

@ljharb I don't understand; the two things are disconnected.

@jridgewell thanks. That's not great; it drives home the point that we could not use private fields for self-hosting any classes involved in inheritance hierarchies. I guess back to weak maps for those, unless this proposal's champions have any possible fixes?

Don't weak maps/sets have the same problem?

var set = new WeakSet()
class Base {}
class Ex extends Base {
  constructor() {
    super();
    set.add(this);
  }

  isExample() {
    return set.has(this)
  }
}

var o = {}
Ex.__proto__ = function () { console.log("called"); return o };
new Ex;

Ex.prototype.isExample.call(o); // true

@domenic

@domenic i guess i'm not understanding why you'd override __proto__ to self-host in an inheritance chain rather than using class extends and super. (if this is off topic we can certainly discuss separately)

@domenic

@domenic:

The behavior you describe looks to me like the branding is on the base class, rather than the subclass. For example, from the behavior you describe, I'd expect Text and Node to look something like

class Node {
  #isNode;
  static assertNode(o) {
    o.#isNode;
  }
}
const assertNode = Node.assertNode;
delete Node.assertNode; // yes, I know this is awkward; decorators can make it a bit simpler, but it's just for illustration

class Text extends Node {
  splitText(offset) {
    assertNode(this);
    // remaining implementation
  }
}

This matches the behavior you observe:

(new Text).splitText(); // works
const o = {};
Text.__proto__ = function () { return o; };
new Text();

Text.prototype.splitText.call(o); // throws!!

Of course, this doesn't match with the observed behavior of not actually invoking the superclass.

I guess back to weak maps for those, unless this proposal's champions have any possible fixes?

Two possible alternatives:

make the web platform classes actually invoke their superclass, like a regular class; I imagine implementations would very quickly add a fast path to do what they currently do as long as the superclass is the expected thing
make web platform classes have immutable prototype chains (at least up through the base class in the hierarchy), possibly after revisiting Implement meta-object trap(s) to make an object's [[Prototype]] immutable ecma262#538.

Edit: sorry, I misunderstood. The following works for web platform classes:

const o = {};
Text.__proto__ = function () { return o; };

Text.prototype.splitText.call(new Text) // does not throw

and not for the implementation below. So, yes, I can't see a way of making this work without a WeakMap or changes to how web platform classes work.

Original incorrect idea follows:

On further thought, a third alternative would be to do something like:

class Text extends Node {
  #privateStuff = 'abc';
  constructor() {
    if (Text.__proto__ !== Node) {
      return { __proto__: Text.__proto__ };
    }
    super(); // installs #privateStuff on `this`, which is guaranteed to be a Node
  }
  splitText(arg) {
    return this.#privateStuff.split(arg);
  }
}

I think this matches web platform classes in all particulars, at least as described in this thread, down to not invoking artificial (and therefore observable) superclasses:

(new Text).splitText('b'); // works

const o = {};
Text.__proto__ = function () { console.log('reached'); return o; };
new Text(); // does not print 'reached', is not === o

Text.prototype.splitText.call(o); // throws

@nicolo-ribaudo

@nicolo-ribaudo: Don't weak maps/sets have the same problem?

Yes, it's a core issue with subclassing.

@ljharb: i guess i'm not understanding why you'd override proto to self-host in an inheritance chain rather than using class extends and super.

He's not. He want's to implement class Text extends Node and Uint8Array natively. But with native class syntax, there's the ability to override the superclass after declaration. These builtin classes do not allow this, meaning there's a difference.

Further, if we were to fix the override difference, there's still the issue of allowing a subclasses privates to be installed on a foreign instance by overriding the superclass.

@bakkot: For example, from the behavior you describe, I'd expect Text and Node to look something like... assertNode(this)

Doesn't this defeat the point of branding by default if I still have to manually brand?

@domenic: Do these classes actually use inheritance, other than to define the prototype's inheritance? If they don't, they could be implemented as base classes, then override the prototypes.

@bakkot

To clarify what I meant about "back to weak maps", I think the following would be the only way to implement web platform / ES262 class semantics:

const _data = new WeakMap();
class Text extends Node { // let's ignore CharacterData for this example
  constructor(data = "") {
    const o = Object.create(new.target.prototype);
    _data.set(o, data);
    return o;
  }

  splitText(offset) {
    if (!_data.has(this)) {
      throw new TypeError("Not a Text!");
    }
    return _data.get(this).substring(offset); // (not really what splitText does)
  }
}

It's a shame we couldn't do something similar to

class Text extends Node {
  #data;
  constructor(data = "") {
    const o = Object.create(new.target.prototype);
    o.#data = data; // !?!
    return o;
  }

  splitText(offset) {
    return this.#data.substring(offset);
  }
}

i.e. it's a shame there's no way to use private fields with the return-override feature that apparently is always in use by web platform and ES262 classes. (Does ES402 have any subclasses?)

@bakkot regarding your (1) and (2), do you think it would be reasonable to do either of those two alternatives to all the class hierarchies in the ES262 spec which also have this problem? I think that's mostly the typed arrays, although maybe also the various Function subclasses.

@jridgewell

I personally wouldn't be opposed to changing either 1 or 2 for classes in the spec, but would prefer other options. Spec classes can be weird in other ways anyway.

Per @jridgewell's comment, could you do something like

class Text {
  #data;
  constructor(data = "") {
    this.#data = data;
  }

  splitText(offset) {
    return this.#data.substring(offset);
  }
}
Text.prototype.__proto__ = Node.prototype;
Text.__proto__ = Node;

? i.e. manually wire up the class hierarchy, but keep Text as nominally a "base" class, i.e., such that it does not invoke Node in the process of constructing its instances. This gives you all of

the correct inheritance for the class objects themselves
the correct inheritance for instances
Text has private fields
constructing Text does not invoke Text.__proto__
if you modify the Text.__proto__, instances of Text inherit from it, but still have the private fields of Text.

This only works as long as Node does not have its own private fields it needs to install on instances of Text. (Same for your example with Object.create, of course.)

It's a shame we couldn't do something similar to

You can seriously abuse the return-override trick to install private fields on arbitrary objects if you really want to:

class DoNotDoThis {
  constructor(o) {
    return o;
  }
}

class Text extends Node {
  #data;
  constructor(data = "") {
    const thiz = new Node();
    thiz.__proto__ = Text.prototype;
    Text.__proto__ = DoNotDoThis;
    super(thiz);
    Text.__proto__ = Node;
    this.#data = data;
  }
}

buuuut don't.

@jridgewell

Spec classes can be weird in other ways anyway.

Do you think they should be weird in these ways, though? I guess I'm unclear why spec classes (in particular ES262 classes) don't follow the same patterns as author-defined ones.

Per @jridgewell's comment, could you do something like

Hmm yeah, that seems superior to my Object.create() approach at least....

This only works as long as Node does not have its own private fields it needs to install on instances of Text.

Right, they kind of do... i.e. these things should be branded as Nodes, not just as Texts.

But I guess this gets into a whole thing where the real requirements are much more complicated than my simple examples so far, e.g. you want "protected" type behavior so the base class can declare private fields that Text's methods can access. And the architecture for doing that properly would probably instead involve a whole different setup (the impl/wrapper distinction).

So not being able to meet the real requirements is OK-ish, I guess. (Which makes me wonder why I opened this thread in the first place...)

I'm not sure there's much actionable left for this thread; thanks for all the attention to detail on walking me through it. I guess it might still be worth pursuing the question as to whether ES262 classes (and probably web classes, following them) should call into their super-constructor.

Right, they kind of do... i.e. these things should be branded as Nodes, not just as Texts.

Then my approach won't work. You have to construct a Node instance to install its private fields.

Or, use decorators to extract the necessary private field's key from Node, and another decorator to install that key onto Text. That would let you keep the same "Text is a base class" hack.

This would be simpler if you could install private fields onto foreign objects. It's easy to achieve the same guarantees as branding on top of general encapsulation, but rather cumbersome to do normal property semantics the other way.

@devsnek

@devsnek I don't quite follow. Internal slots are not tied to things that have a certain prototype chain, but rather to what the constructor outputs. Public fields work like this too--they're just a way that the constructor can add things to the object, and don't have anything to do with prototype chains.

The issue here was, even if you mess up the prototype chain of the constructor, you should be able to have the private fields added to the instance. I agree that it's OK to be able to write a class that doesn't have the private fields and instead returns something else. The question was, how should you be able to use the original prototype chain and still add the private fields/methods to the instance, when you want to?

@Igmat I can see how making private symbols that you can add to arbitrary existing instances is a solution to the problem, but does @jridgewell 's proposal (which doesn't do this, at least in the first step) address it?

@littledan

@littledan @devsnek
I don't know if I'm getting the right impression, but let me ask a question. Since it's already the case that the functions within the lexical scope of the class definition contain the only code that is aware of and able to access private data defined in that class, that all instances of that class contain private data accessible via the same name (the type of that name is irrelevant for this question), and that the only guarantee of brand-checking is that a given object has passed through a given constructor, for the sake of attaching private fields, is it a priority that the object in question is the result of using new on that class?

Or put more concisely, what does it matter where the object came from? Is the constructor of a class that has private fields free to put those fields on any object?

is it a priority that the object in question is the result of using new on that class?

In this issue, we're talking about matching the branding semantics of web platform and JS builtin classes, which do something special with things that were the output of new.

I'm going to unsubscribe from my own thread as this appears to have degenerated into being the same as every other issue thread in this repository, i.e. a place for people to complain about the current proposal and push their counterproposals.

Let me know through out-of-band communication if any more interesting on-topic discussion requires my attention.

I've hidden comments that were about other proposals. Folks can follow up about private symbols in https://github.com/zenparsing/proposal-private-symbols .

In this issue, we're talking about matching the branding semantics of web platform and JS builtin classes, which do something special with things that were the output of new.

That's a prime example of how existing "weird" (exotic) behaviors are cutting into what a good language feature can actually be, at the expense of end users.

New features don't have to match what existing weird/exotic objects do. Give users the best features for their code, and leave the old mistakes as they are without limiting innovation just to explain existing mistakes.

rant

a place for people to complain about the current proposal and push their counterproposals.

Also a place for people who aren't paid while writing here to feel as if there's little hope. :)

Consider yourself very lucky if you can write here while "at work" and getting paid!

More importantly than creating other proposals, is stopping or slowing this one.

Requiring that some other proposal be accepted in order to stop this one (or else this one will get released and implemented) is not a nice thing for the community to be subjected to.

"Make a better proposal now or else this one will be implemented with its flaws and all."

Its in some ways similar to "Jane gets the electric chair in one week, for killing John, unless you can prove she didn't do it" while having seen someone else do it with your own eyes but not fortunate enough to have evidence.

I think we have examined this topic sufficiently, so I'm closing the thread.

Navigation Menu

Repository navigation

Branding guarantees do not match web platform classes, due to return override #179

38 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Branding guarantees do not match web platform classes, due to return override #179

Description

Activity

ljharb commented on Dec 10, 2018

jridgewell commented on Dec 10, 2018

domenic commented on Dec 10, 2018

ljharb commented on Dec 10, 2018

jridgewell commented on Dec 10, 2018

domenic commented on Dec 10, 2018

nicolo-ribaudo commented on Dec 10, 2018

ljharb commented on Dec 10, 2018

bakkot commented on Dec 10, 2018

bakkot commented on Dec 10, 2018

jridgewell commented on Dec 10, 2018

domenic commented on Dec 10, 2018

bakkot commented on Dec 11, 2018

domenic commented on Dec 11, 2018

jridgewell commented on Dec 11, 2018

38 remaining items

littledan commented on Jan 8, 2019

rdking commented on Jan 8, 2019

littledan commented on Jan 8, 2019

Igmat commented on Jan 8, 2019

littledan commented on Jan 8, 2019

littledan commented on Jan 8, 2019

Igmat commented on Jan 8, 2019

littledan commented on Jan 8, 2019

jridgewell commented on Jan 8, 2019

rdking commented on Jan 8, 2019

domenic commented on Jan 8, 2019

littledan commented on Jan 8, 2019

trusktr commented on Apr 7, 2019

littledan commented on Apr 8, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions