Restrict the interaction of decorators and private names to ensure limited overhead

[littledan]

Private methods and accessors should not take up more space per instance than a word per "brand" (class in the class hierarchy), even with the conjunction of decorators and hashtable-mode objects. Various JS engine maintainers (@gsathya, @verwaest, @jorendorff) have pointed out to me that the current design does not really support this sort of guarantee in general. Let's think about how we could make it the case, while maintaining the existing "brand check" semantics. At the same time, maybe we could use this as an opportunity to phrase the specification in a way that the brand checking in private methods could be used in other contexts, e.g., WebIDL (@annevk recently suggested to me that WebIDL could use some formalization here).

Some high-level notes about one approach I've been thinking about--I'd be interested if anyone has other ideas about how to achieve this:

• New private object model: Private methods and accessors are stored on the private name, together with a "brand". Objects have a list of brands, alongside their list of private fields.
• Mental model: Private fields are a WeakMap, and private methods/accessors are a WeakSet, which is a consolidated set for all the private methods in that class.
• Logic to read a private name (similar for writing):
  1. Check the private name type, either ~brand~ or ~field~
  2. If ~field~, search the [[PrivateFields]] list on the instance and return the result, or throw a TypeError when missing.
  3. If ~brand~, check the [[Brands]] list on the instance, based on the private name's [[Brand]] internal slot, and throw a TypeError if missing.
  4. If the private name has a [[Value]] (it's a method), return the value
  5. Otherwise, if it has a [[Getter]], call the getter
  6. Otherwise, throw a TypeError
• New private decorator restriction:
  1. Undecorated private class elements get the brand and value/getter/setter fields from the syntax.
  2. A PrivateName can only be used either for a field (reusable as a field) or, if used for a private method/accessor, cannot be used a second time--only usable in one class.
  3. No need to include writability/enumerability/configurability on a private field or method (meaningless/redundant), just initializer/value/get/set
  4. Future proposals may include a way to repeat a brand from one class to another, but that's v2 (it can't really be done well in v1 even before this change). You can leak a PrivateName for reading/checking, but only inherit for sharing writing.
• Use in WebIDL of brands:
  • Platform objects have a cross-realm [[Brand]] that the constructor puts on the instance
  • JS-style mixins in WebIDL could be supported similarly, taking advantage of adding a brand later in an object's lifetime e.g., as suggested by @kenchris in Serializable should probably be a mixin littledan/serializable-objects#3
  • TODO(@littledan): Read how various engines implement platform object brand checks, and make sure it's all consistent

Thoughts?

[nicolo-ribaudo]

A PrivateName can only be used either for a field (reusable as a field) or, if used for a private method/accessor, cannot be used a second time--only usable in one class.

Does this mean that it won't be possible to convert a private field to an accessor using a decorator?

[littledan]

No, supporting that case remains a design goal. The idea is that Private Names would have their field vs accessor-ness assigned after decorators run.

[nicolo-ribaudo]

Ok, so 👍
And also it won't be possible to do something like this, right?

function getPrivateName() {
  var pn;
  class A { @(({ key }) => { pn = key }) #x }
  return pn;
}

function decorator(el) {
  el.key = getPrivateName();
}

No need to include writability/enumerability/configurability on a private field or method (meaningless/redundant), just initializer/value/get/set

writability could make sense, especially if I expose the PrivateName object to a trusted class (for example, to read a private id set during initialization).

[littledan]

Thanks for this detailed review; it's really helpful.

I'm not saying there is no use case to the removed things, but that they can be accomplished other ways, without a loss to expressiveness.

And also it won't be possible to do something like this, right?

To accomplish that, you can either make the private name not show up in the final class A, or throw the name so that class evaluation never finishes.

writability could make sense, especially if I expose the PrivateName object to a trusted class (for example, to read a private id set during initialization).

To accomplish that, you could create a private getter with no setter.

[nicolo-ribaudo]

Ok thank you!
I find these restrictions reasonable; they don't impact common usage of decorators and they can easily be worked-around.

[littledan]

One further change I am thinking of to tie it together: rather than own/prototype placement, I am thinking of a single instance placement, which is own for fields and prototype for methods/accessors. This should cut down on the number of special cases/holes in the grid, and be more analogous to syntax (so more intuitive for decorator authors), while also not really reducing expressiveness.

In particular, this would help give the guarantee that methods don't ever require per-instance overhead. I believe you could get around the missing cases through a combination of static initializers and private accessors--it also doesn't reduce expressiveness.

[nicolo-ribaudo]

Undecorated private class elements get the brand and value/getter/setter fields from the syntax.

Does an undecorated field inside a decorated class count as undecorated?

[littledan]

Since private elements are not accessible to class decorators, yes, they count as undecorated.

[nicolo-ribaudo]

One further change I am thinking of to tie it together: rather than own/prototype placement, I am thinking of a single instance placement, which is own for fields and prototype for methods/accessors. This should cut down on the number of special cases/holes in the grid, and be more analogous to syntax (so more intuitive for decorator authors), while also not really reducing expressiveness.

Yeah it makes sense to me, but we should also ask the opinion of who advocated for fields on the prototype, since it won't be possible anymore to have them.

EDIT: static initializers/finishers should work for that usecase.

[littledan]

I'm wondering, should we break decorator/private interaction into a follow-on proposal, so we can advance the public part of this proposal to Stage 3 soon, and take the time to get this right for private? Now that class decorators don't see private, there's no strong constraint that these land at the same time. (GitHub reacts welcome!)

[rbuckton]

Does this mean that Decorators cannot add new private fields? I'm concerned that this will significantly cut into the use cases for decorators. On the other hand, if we could land a fix for #24 along with this, I would support breaking this out.

[littledan]

Yes, I am suspecting we should have a stronger story for things like #24, to avoid more classes of accidental leaks, at the same time as the feature ships.

I see synthetic private fields and leaking the name of private fields and methods as key use cases. The open question for me is, how much of the rest of it is important? It would be much easier to implement and maybe easier to avoid certain accidental leaks if we just support those two.

[nicolo-ribaudo]

Does this mean that Decorators cannot add new private fields?

If I understand it correctly, it would still be possible:

function PrivateName() {
  let pn;
  class A {
    @(desc => {
      pn = desc.key;
      // Return another descriptor, so that the private name isn't attached to the class
      return { kind: "initializer", placement: "own", initializer() {} };
    })
    #priv;
  }
  // pn hasn't been used by any class, so it can be used.
  return pn;
}

function decorator(el) {
  el.key = PrivateName();
}

[littledan]

I think @rbuckton is talking about #180 (comment) , and @nicolo-ribaudo is talking about the original post. Sorry for making the thread confusing.

[nicolo-ribaudo]

Oh right, don't know how I missed your previous comment.
Anyway, private names aren't the only stage 3 blocker: there are also @dec export vs export @dec and the new name for initializers. Also, class fields are still stage 3. For this reasons, I don't think that we currently need to remove the private names capabilities from this proposal: it wouldn't be enough and we don't really need to rush it.

[littledan]

I'm working on rebasing the decorators spec on top of the current class fields/private methods specs, and will include this restriction in the change.

[littledan]

Closing as #180 is merged.