tc39 / proposal-private-fields

A Private Fields Proposal for ECMAScript
https://tc39.github.io/proposal-private-fields/
318 stars 19 forks source link

Generalize spec internal slots as private fields? #17

Closed zenparsing closed 8 years ago

zenparsing commented 8 years ago

It would be ideal if private slots could be a strict generalization of the internal slots used by the ES spec for the private state of built-in objects. Unfortunately, making this happen would require a breaking change to ES6.

To see why, consider this example:

class B extends A {
    constructor() {
        super();
    }
}

// OK, now let's change the prototype chain to make it a Map subclass!
Object.setPrototypeOf(B, Map);
Object.setPrototypeOf(B.prototype, Map.prototype);

// Now let's create an instance!
let b = new B();
// Everything works OK
b.set("a", 1);

That example works in ES6 because the Map constructor creates an object with a [[MapData]] internal slot. The internal slot list of the allocated object is not affected by NewTarget.

For this proposal, the user-defined private slot list is determined by NewTarget, so you cannot swap constructor prototypes and expect things to work.

class B extends A {
    constructor() {
        super();
    }
}

class C {
    #foo;
    constructor() {
        this.#foo = 1;
    }
}

// OK, now let's change the prototype chain to make it a C subclass!
Object.setPrototypeOf(B, C);

// Now let's create an instance!
let b = new B();
// Nope, that that fails because the object isn't allocated with a #foo slot...

I would prefer to remove this incongruence and re-spec all built-in internal slots using the same semantics as user-defined private slots, but that would be a breaking change.

Thoughts?

@wycats @littledan @allenwb

littledan commented 8 years ago

I would support this change. I think it would enable all the important use cases of subclassing builtins. Adding internal slots based on the class-initialization-time 'extends' chain is easy and intuitive to understand, and seems to me like a good mental model to unify internal slots, private state and property declarations. I don't think most users would expect that they could do exotic things like make a class whose constructor makes instances with several different builtin classes by setting the prototype within the constructor before calling super, as ES2015 provides; it's hard for me to think of a use case here, but maybe others could help.

Instead, internal slots, private state and property declarations could be created and initialized to undefined soon after object instantiation, before calling any constructors, based on the 'extends' chain. Then, if the Map constructor runs as part of the super chain, and it accesses an internal slot which doesn't exist due to a dynamic proto change, it would throw a TypeError.

Although this is technically a breaking change to ES2015, I'd expect it to be web-compatible and lead to fewer issues than some of the other ES2015 changes that we're shipping and coping with anyway. It might even simplify implementations a little.

I really like this drive for consistency, and your proposal here seems to be the simplest and most consistent overall that I can think of.

allenwb commented 8 years ago

Actually, you initial example doesn't work unless you add the line:

Object.setPrototypeOf(B.prototype, Map.prototype);

or assign Map.prototype.set to B.prototype.set.

Before considering a breaking change we should probably consider what we want to happen when this sort of manipulation occurs in the context of classes that are explicitly declared with private slots. In particular, is it reasonable to try to make your second example (dynamically extending C) work without error? I think it is as long as B does not define or statically inherit any internal slots. In other words, we could allow the [[Prototype]] of a slot-less class constructor to be dynamically set to any constructor value without breaking the inherited constructor in cases like this example.

So, how could we make

let b = new B();

work?

We can include in the new private slot aware definition of ordinary [[Construct]] the following rules in place of the current step 5:

If kind is "base", then 
   Let slots be newTarget.[[instanceSlots]].
   If slots is undefined and F.[[instanceSlots]] is not undefined, then
       starting with newTarget, do a [[GetPrototypeOf]] walk until either null is reached or a 
       constructor is reached whose [[instanceSlots]] is not undefined.  In the latter case 
       slots is set to that [[instanceSlot]] value.
   Let thisArgument be ? OrdinaryCreateFromConstructor(newTarget, "%ObjectPrototype%", slots).

Assuming that analogous behavior is incorporated into any built-in constructors that have internal slots, this preserves the current ES6 behavior shown in the first example. I appreciate that the Chrome implementors seem to have a distaste for this style of secondary proto walk but it preserves the legacy (and arguably desirable) ES6 semantics and would only actually occur in situations where proto chains have been manually modified in the manner shown in these examples.

zenparsing commented 8 years ago

@allenwb

Actually, you initial example doesn't work unless you add the line:

Fixed!

we could allow the [[Prototype]] of a slot-less class constructor to be dynamically set to any constructor value without breaking the inherited constructor in cases like this example.

Gotcha. Seems a bit hacky but it would work.

Either way, there seems to be agreement that we want private slots to be a generalization of ES6 internal slots, either with a breaking (but possibly web-compatible) change or by adding the special-case logic you describe.

littledan commented 8 years ago

Can we avoid doing a walk like this? There's already an implicit walk based on the super calls that the user makes, and of course the walk on property access to objects up the proto chain. These are clear, well-established constructs, and additional walks feel to me like a less-clear, harder-to-understand secondary usage of the prototype chain. We've had to remove similar walks, e.g., in the TypedArray code, due to poor interaction with other language features. In general, I think we benefit from making each language feature as locally simple as possible, and interact with each other in simple ways.

This particular change feels a bit brittle because it only works with changing from no slots to some slots. It would preserve most of ES2015 backwards compatibility, but it wouldn't meet what I'd imagine are user expectations, that if it works going from no-slots to slots, then it should work going from some-slots to some-other-slots.

@allenwb Do you have an idea for an example of where these semantics would meet concrete user needs or expectations?

allenwb commented 8 years ago

@zenparsing Yup. It has ways been a harmony requirement that built-ins should be self-hostable. In ES6 internal slots were problematic probably requiring the use of weakmaps for self-hosting. I've always assume that a requirement for private slots was that they could be used to self-host any required internal slots

@littledan I don't think you can reasonably avoid the second walk if you want compatibility with the ES6 semantics. The initial implicit (it's actually explicit) walk via super calls actually works via invocations of the [[Construct]] internal method which is refied as Reflect.construct(target, arguments [,newTarget]). It would take an additional argument to pass along the slot list. But we very much do not want to expose the slot list to any sort of reflection. So what we would probably have to pass as the additional argument the intermediate constructor that encapsulates the slot list. However, if the slot list isn't exposed to reflection there is no way for a construct trap handler to decide whether or not the current target sis the one that provides the slot list.

I don't see any problems or with the additional walk (in the very rare cases that it is needed). Instantiating a subclass with private slots doesn't have a clear well established semantics. It currently doesn't exist. It seems a better question is can it be clearly explained in prose. It think it can: When instantiating a subclass, the private slot descriptor is provided by the most derived constructor that has such a descriptor.

I don't see the complexity risk of the second walk is the same that we get when proxying [[getProtoypeOf]], which we already have.

Regarding use cases. It is the same as for any JS [[Prototype]] hacking. JS has always been a very dynamic language in that regard and people invent interesting was of using it. During ES6 TC39 made an explicit decisions that we want to preserve such capabilities. For example, we could have said that any constructor defined using a class declaration has an immutable [[Prototype]]. But we choose not to do so. Given that permissive position, we need to just make sure that we have a well defined semantics of what will happen when developer choose to use that permissiveness.

littledan commented 8 years ago

What I have trouble understanding is why we would want to support switching the slots in some cases (where there's no private state to some private state) and not other cases (from some private state to some other private state).

zenparsing commented 8 years ago

After thinking this over, I'm worried that our desire to have a static [[InstanceSlots]] list is fundamentally incompatible with prototype mutation. Let's reverse the direction of the initial example:

// This could be a self-hosted built-in
class A {
    #foo;
    constructor() {
        this.#foo = 1;
    }
}
// A.[[InstanceSlots]] = [#foo]

class B extends A {
    constructor() { 
        super();
        this.x = 2;
    }
}
// B.[[InstanceSlots]] = [#foo]

class C {} // No slots!

// OK, now let's change the prototype chain to make it a C subclass!
Object.setPrototypeOf(B, C);
Object.setPrototypeOf(B.prototype, C.prototype);

// Now let's create an instance!
let b = new B();

/*

That works, now `b` looks like this:

{
    [[#foo]]: undefined
    x: 2
}

*/

The b variable above has been allocated with the shape of A (i.e. it has a #foo slot), but it's completely uninitialized with respect to the A constructor. One of the lynchpins of the current ES6 class design was that instances of built-ins should never be in an observable "allocated-but-uninitialized" state, but here we've seemingly violated that constraint.

It seems that our options are:

  1. Allow objects (even perhaps built-ins) to be in "allocated-but-uninitialized" states,
  2. Lock down prototype mutation somehow to prevent cases like the one above, or
  3. Give up on a static [[InstanceSlots]] list associated with NewTarget, and instead allow private slots to be dynamic (specifically, created in the home constructor when the this binding is initialized).

This analysis might be overly pessimistic though. It's pretty late at night here on the east coast : )

littledan commented 8 years ago

OK, I see your point. If we had a static [[InstanceSlots]] field, it doesn't seem like we could unify that with subclassable builtins, as their uninitialized behavior is not defined and we would rather not go into the weeds defining it. So what is the alternative to go for? To add the private slots right after super() returns in the constructor?

If we do go with that option, another benefit will be that Web Components subclassing HTMLElement will be able to have private state, if it goes by this proposal: https://github.com/w3c/webcomponents/blob/gh-pages/proposals/Constructor-Dmitry.md . The core of that proposal, from our perspective, is that the superclass uses a special trick where the super constructor returns a different instance, and then the subclass constructor executes on it, "upgrading" the element. If we add private state fields imperatively in the constructor, after super() returns, then web components would be able to have private state as the programmer would expect. (No idea how protected fields would work, though!)

zenparsing commented 8 years ago

@littledan Yeah, there are some nice properties here. Also, the super constructor can do "return overriding" and everything works like the user would expect.

Exploring this idea further: one of the primary motivations for a static [[InstanceSlots]] was to provide engines with an "early" way to determine the "shape" of an object based on the constructor, thereby allowing performance optimizations. What might be the performance impact of dynamic private slots, from a V8 perspective?

allenwb commented 8 years ago

@littledan I'm not sure what you mean by "switching slots"? I don't really see that concept in the preceding messages.

We have to deal with the possibilities introduced by the inherent mutability of the JS object model. That includes the fact that JS programs can dynamically do things like move methods (that may reference private state) between classes mutate [[Prototype]] slots, constructors that return do something other than default allocation, etc. At the very least we have to make sure crazy things fail safely, but it is more in the JS spirit to try to make borderline-crazy things actually work with a borderline-rational semantics.

allenwb commented 8 years ago

@zenparsing

// OK, now let's change the prototype chain to make it a C subclass!
Object.setPrototypeOf(B, C);
Object.setPrototypeOf(B.prototype, C.prototype);
// Now let's create an instance!
let b = new B();
/*
That works, now `b` looks like this:
...

Does it actually work? Doesn't this create a [[Prototype]] circularity between B and C such that the such that the super() calls in the B and C constructors will call each other until the call stack overflows?

allenwb commented 8 years ago

@zenparsing

...One of the lynchpins of the current ES6 class design was that instances of built-ins should never be in an observable "allocated-but-uninitialized" state, but here we've seemingly violated that constraint.

It was a requirement for ES6. I believe that the primary motivation was a desire that implementations of the built-ins that are not self-hosted (eg, C++ code) could use a record-like representation and should not have to contain a lot of dynamic initialization checks. We accomplished this in ES6 by essentially giving each built-in a custom [[Construct]] internal method where each internally ensured that invariant. That works fine in ES6 because only a built-in base class can define internal slots (no subclass provided slots) and because the spec. pseudo could be carefully written to never expose a built-in instance with uninitialized slots.

However, once you allow subclasses that introduce slots and allow constructors to contain arbitrary JS code (that could expose the object being constructed at any point) there doesn't seem to be any reasonable way to meet that requirements. We've all thought about it a lot and haven't found a solution. That is why our private state proposal initializes all private slots to undefined.

It seems that our options are: I don't think these are independent options

  1. Allow objects (even perhaps built-ins) to be in "allocated-but-uninitialized" states,

For the general case we must. "initialized" is an application domain specific concept. There is no generalized way at a low level to know whether, from a application perspective, a particular private slot has been property "initialized". Especially when arbitrary JS code is involved in the initialization process.

But this doesn't mean that a "native" built-in implementation couldn't use implementation specific means enforce the ES6 built-in spec invariant.

  1. Lock down prototype mutation somehow to prevent cases like the one above, or

I think this is a plausible alternative (and something that is independent from the first alternative). Arguably, the private slots aspects of the new object model depends upon the superclass hierarchy at the time the class definition is evaluated. Given that dependency, making [[Prototype]] immutable for class definitions that define or inherit private slots seems like it might be conceptually reasonable from a user perspective.

But it doesn't fix the problem at the top of this thread. It allows for a (relatively) static [[InstanceSlots]] list and record-like allocation of private slots but would still require something like I proposed in https://github.com/zenparsing/es-private-fields/issues/17#issuecomment-170100704 to preserve ES6 compatibility.

  1. Give up on a static [[InstanceSlots]] list associated with NewTarget, and instead allow private slots to be dynamic (specifically, created in the home constructor when the this binding is initialized).

I think static, record like allocation of all slots is a strong requirement that will be hard to eliminate. A static accumulating [[InstanceSlots]] is not strictly speaking needed, if we allow the slot allocator to dynamically walk the constructor chain. I could be convinced to relax that, but I'm pretty sure the Chrome implementors wouldn't like it

I like the idea of a immutable [[Prototype]] as I think it would make slot access much easier to optimize. But I'm pretty sure some people will argue that it goes against the grain of JS. But, we can't please everybody.

littledan commented 8 years ago

I've often heard from JavaScript users (@wycats) that they would like reliable performance, with the spec offering mechanisms for using JavaScript in a way that more efficient object representations would be reliably used. If we were to sacrifice some staticness in favor of [[InstanceSlots]], perhaps by throwing exceptions if objects were instantiated the wrong way after the wrong prototype chain, then that could be one way that the reliable performance could be provided. On the other hand, if we take as our goal that we want to allow users to do all sorts of borderline-crazy things and have it work, then users may find themselves accidentally depending on these features, and getting code that's less optimizable. It's a decision that we could make either way as language designers.

@zenparsing It would mean that private slots are just like ordinary properties, with our existing optimizations, rather than the possible improvement of a more reliable static shape. A more reliable static shape, which [[InstanceSlots]] would give us a path towards, could help programmers to get reliable performance with less of a possibility of adding a little bit of code getting off the fast path.

@allenwb By "switching slots" I mean changing the prototype to one that has different slots. This discussion is directly about whether we want to support dynamic prototype modifications with respect to private slots. If it's already a settled conclusion that we do, then I agree that to use [[InstanceSlots]] we would need some kind of support like you suggested (though I think the particular mechanism you offered is a little too brittle). However, I think we still have some leeway to revisit some of these decisions if we want to, based on user requests for more reliably optimizable coding patterns.

zenparsing commented 8 years ago

Great discussion.

@allenwb

There is no generalized way at a low level to know whether, from an application perspective, a particular private slot has been property "initialized". Especially when arbitrary JS code is involved in the initialization process.

I agree, in general. However, I have a hard time letting go of the idea that an object can have private slots without ever having passed through a constructor whose purpose is largely to initialize them.

class A {
    #foo;
    constructor() { this.#foo = 1 }
}

let arbitrary = Reflect.construct(Object, [], A);
// `arbitrary` has { [[#foo]]: undefined }

Making the [[Prototype]] immutable for constructors which have non-empty [[InstanceSlots]] seems good, but I'm not sure how to restrain Reflect.construct to prevent situations like the above while still allowing subclassable proxies. Do you think it's not a problem?

@littledan

Just focusing on deoptimization for a minute. Simplifying things to a degree, I think the main issue with respect to the current optimizations is that users tend to write things which appear completely innocuous but that have far-reaching performance impacts. The language encourages users to treat ordinary objects like dynamic property bags, for instance.

If private slots were more dynamic, then what would be the deopt paths?

  1. A superclass constructor returns an arbitrary object, bypassing the normal base class object allocation.

    class Parent {
       constructor() {
           return { a: 1 };
       }
    }
    
    class Child extends Parent {
       #foo;
    }
  2. The prototype chain of the constructor is mutated.

    class A { #foo; }
    class B { #bar; }
    class C extends A { }
    C.__proto__ = B; // Switch super chain
    C.prototype.__proto__ = B.prototype;

In both cases, I think it's pretty clear to the user that they are doing something weird, and as such, a deopt wouldn't be that surprising.

On the other hand, I wouldn't want to give up any potential performance improvements that truly static [[InstanceSlots]] might give us.

I think I'm OK with either direction. It seems to me that dynamic slots handle all of the edge cases with little or no effort (e.g. Reflect.construct), whereas static slot lists have that nice "staticness" to them.

allenwb commented 8 years ago

@zenparsing

I'm not sure how to restrain Reflect.construct to prevent situations like the above while still allowing subclassable proxies. Do you think it's not a problem?

Uninitialized (at the application level) slots are only a problem if some code actually tries to access the value of such a slot. Initializing a slot to undefined at instantiation time can be used to detect such accesses but it requires explicit application code at every access point to detect that the slot has not been logically initialized by the application level.

There is a pretty straight forward fix for this. Give every slot an out-of-band initialized flag that is never reified as a value of the slot. Initially all slots are created and flagged as uninitiated when the containing object is instantiated. The GetPrivate abstract operation always checks the slot initialized flag and throws when asked to retrieve the value of an uninitialized slot. The SetPrivate abstract operation always and unconditionally clears the initialized flag.

This adds a bit of overhead to GetPrivate but for optimized code most such checks can be eliminate via value propagation and dead code elimination. The flag doesn't have to be a separate piece of state. As long as all such slots are containers of ECMAScript language values the unitialized state can be represented as a unique object reference that is never exposed to ECMAScript code.

I think this would be a good change to make to the proposal.

littledan commented 8 years ago

@zenparsing From V8's perspective, I don't think any particular deopt would be involved. It would be exactly as if you had

class A { constructor() { this.foo = 1; } }
// ...

The dynamic proto chain modification happens after the class is instantiated, but before it's constructed even once, so actually everything should be fine. We don't currently inline super() calls, but when we do, I don't imagine that it'd be until after the constructor is called a few times.

A slower case is more likely to be from a polymorphic callsite. For example, say the super constructor could return an instance of A or B, and there are different lists of private slots. Then the code that follows would have to be capable of handling multiple hidden classes.

All together, it would be really great if we could get the extra determinism from things like the inheritance chain being more locked down, but because ordinary properties are already so flexible, and we have to optimize those, it wouldn't be the end of the world if private state were similarly dynamic.

It's hard for me to see how we could really soundly lock things down--what if you extend a class with no [[InstanceSlots]], and then replace the proto with something that has some private state, and then replace it later again with something without private state? I don't want to go down a brittle path with a halfway solution. I've seen a few libraries which already do subclass builtins pre-ES2015 with some interesting constructor tricks and modifying proto, so this isn't a completely theoretical case.

On the other hand, adding TDZ checks as @allenwb suggests would certainly add some overhead. If the constructor isn't reached due to proto chain modification, then GetPrivate won't be run before SetPrivate, and the checks and state would have to be reified. It's difficult to propagate such initialized information interprocedurally, so we'd probably be left with a lot of these checks. I'd rather we not go down that route, as it really risks making private state slower than ordinary properties.

allenwb commented 8 years ago

Even with a single class we have the possibly of incomplete construction unless we invent new usual syntactic constructor forms that don't expose the this value to ES code during construction.

So we basically have two possible approaches that are already used in the language. We can preinitialize all instance slots to undefined or we can use a TDZ for slots. In my private state proposal I choose to go down the undefined path because it was simpler overall. But a TDZ approach seems plausible and is probably a more reliable mechanism and viewed from userland. @littledan argues that a TDZ was necessarily add runtime overhead. I'm not so sure, even without a TDZ GetPrivate/SetPrivate logically still have to check that the object that is being access actually has the slot that is being accessed (because slot references methods can be installed as properties on objects than instances of the class they were defined by). I think it is plausible that a slot existence guard and a TDZ guard could be combined by an implementation into a single test.

I think either alternative is viable. For now I think I'll stick with undefined in by proposal but I wouldn't mind going the TDZ route.

zenparsing commented 8 years ago

@allenwb If we wanted per-field TDZ we would have to make sure that SetPrivate doesn't in general clear the flag: the field should only be considered initialized when set to an initial value within the constructor.

Such a TDZ would fix the Reflect.construct issue, but we'd still be left with the hacks for the prototype mutation issue. The fact that we need to plug these holes in non-orthogonal ways indicates that there may be a more clean and simple solution.

I'm in favor of dynamic slots as described above. With dynamic slots, you don't need to have special-case logic to support prototype mutation and you don't need per-field TDZ to deal with Reflect.construct. It all just works in harmony with the existing ES6 class design.

I'm definitely a big fan of adding more static features to JS, and in this case I think we can strike the right balance between making it easy for users to define static shape to their programs, while at the same time fitting in seamlessly with the rest of the language. With our proposed syntax, it makes defining classes with static shape easy and pleasant, and more "creative" uses of slots (via prototype mutation or return-overriding) stick out like ugly sore thumbs.

We also have the possibility of introducing "const classes" (@erights ?) which would offer the ability to lock down the prototype chain and other things like that.

What are your objections to dynamic slots, other than the possibility that implementers will balk?

littledan commented 8 years ago

Sounds like we have some pretty good consensus here on the overall shape, going with the "dynamic slots" proposal. There are some edge cases to lock down from here. For example, what are the semantics when the same private slot is added to an instance multiple times, e.g., through the super return trick? Options include

  1. Throw an exception (my favorite)
  2. Redefine as undefined
  3. Preserve the previous value (least favorite, as this actually kinda maybe leaks private data sorta)
zenparsing commented 8 years ago

@littledan Currently I have it doing option 2 (https://zenparsing.github.io/es-private-fields/#sec-addinternalslots).

I was thinking that someone might want to have a pool where the super constructor returns a reclaimed object from the pool, and then the subclass would want the fields "zero'd" out to undefined. It's a stretch, admittedly.

What are your reasons for preferring option 1?

allenwb commented 8 years ago

@zenparsing

If we wanted per-field TDZ we would have to make sure that SetPrivate doesn't in general clear the flag: the field should only be considered initialized when set to an initial value within the constructor.

Why is it necessary that all slots be explicitly initialized within the constructor. For example, consider a private slot that is solely used as the backing store of an accessor property whose contract is that the property must be "set" before doing a "get" on it. Why should be require the initialization of such a slot in the constructor. It would just require that the "get" accessor invent some other mechanism (for example a second slot) for determining whether or not an initial "set" has occurred. Slot TDZs that apply in all methods eliminates that necessity.

More generally, as I've argued other places, object initialization is an application level concept. It is up to the application developer to decide what the initialization invariants are for each of their classes. Such invariants may be complex and involve constancy relationships between many objects and their associated slots and properties.

There is no way, at the language level, than we can ensure an object has been fully or correctly constructed or "initialized". All we can do is ensure that the objects that are exposed to ES code are "safe" in terms of the language level semantics. Preinitializing all slots to undefined or placing all slots within a TDZ are simply to different was of ensuring language level memory access safety.

Preinitializing to undefined is already familiar to JS programmers. It has the disadvantage that for object values undefined may propagate along the execution path before it is noticed via an attempted property access and that automatic coercion to primitive values may camouflage such *_undefined_s.

TDZ has the advantage to automatic exception generation at the immediate point of access. It has the disadvantage that it requires additional runtime mechanism and probably overhead.

allenwb commented 8 years ago

@littledan

sounds like we have some pretty good consensus here on the overall shape, going with the "dynamic slots" proposal.

Absolute not true! A fixed shape (contiguous storage block at allocation) is the very first requirement listed in the proposal that Yehuda and I authored. This requirement was strongly put forward to us by several implementors (including from V8) in addition to our own experience.

It also reflects concrete experience from an earlier attempt to require that the set of built-in slots of an object be dynamically extensible. This idea was eventually dropped because of very strong push back (see es-discuss archives) from browser implementors at that time. They very much required the ability to allocate such objects as fixed-size records.

For example, what are the semantics when the same private slot is added to an instance multiple times, e.g., through the super return trick?

Such cases are addressed in our static private slots proposal.

littledan commented 8 years ago

@allenwb Although we'd prefer to be able to find some way to have static object shape, the earlier discussion in this thread (about how to deal with __proto__ mutation and consistency with built-in use of internal slots) has left me not seeing a good way to do it. It seems strange that if a parent class has private state, and you change the __proto__ to another class with private state, the constructor wouldn't have the intended behavior, if we preserve the behavior that reparenting a class which subclasses a builtin would change which internal slots are created. Both your proposal and Kevin's earlier proposal are subject to this issue. Providing it some of the time--when the class and its parent classes have no private state, but later a parent class is replaced by one with private state, would only add confusion and not fix the general case.

I've talked this over with a number of people in the V8 team and the consensus is that this approach would be fine; it's disappointing but not unexpected, and we would probably be able to apply the existing optimizations that we apply to ordinary properties.

I'd really rather not add an additional TDZ case unless we have a strong use case for it. For let, we decided to initialize it to undefined after the let is reached. I think running the object constructor is analogous to passing the let declaration. ECMAScript programmers are used to being able to read undefined from uninitialized properties, and providing that behavior here would seem to be a good analogy as well. If we could represent the TDZ in the hidden class of an object to reduce the number of checks, it might not be a perfect solution--creating more hidden classes for different initialization states could lead to more difficult-to-optimize, polymorphic code.

zenparsing commented 8 years ago

@allenwb

Why is it necessary that all slots be explicitly initialized within the constructor.

You're right, it's not necessary. I was getting mixed up with another discussion/proposal.

littledan commented 8 years ago

There's an advantage of initializing all slots within the constructor, which is that it brings us closer to static object shape with fewer possibilities for the hidden classes underneath (or, alternately, extra checks at access sites). This is just the same advantage as property declarations. Users have been asking us for a way to make this more predictably fast, and "declare all your properties/private state, and don't mutate your proto chain" would be a nice answer.

allenwb commented 8 years ago

@littledan

Although we'd prefer to be able to find some way to have static object shape, the earlier discussion in this thread (about how to deal with proto mutation and consistency with built-in use of internal slots) has left me not seeing a good way to do it.

Gee, I thought we had identified perfectly reasonable ways to address these concerns.

On Jan 8, 11:31 AM I showed one way to deal with the issue. Doing a [[Prototype]] walk starting from the new.target, but only in the rare cases where the base constructor has [[instanceSlots]] but the new.target does not. There are already a number of places in the ECMAScript spec. were we provide fall back behaviors to deal with backwards compatibility issues that might be revealed by similar low level tampering. This is a similar case and dealing with it in the way I suggest is a much smaller remediation than switching to fully dynamic private slots.

We also discussed freezing the [[Prototype]] slots of classes declared to define or inherit private slots. And also the possibility of giving private slots a TDZ as a means to help find application level constructor initialization errors. I think both of these are plausible embellishments but probably not necessary to address @zenparsing's original issue. But either or both would be preferable (and I believe lower impact) than abandoning a static shape for internal slots.

So let's get back to basics. What specific problems do you think can't be solved by any of these approaches. Let's work through each one individually and see if there really is a problem and how it could be addressed.

zenparsing commented 8 years ago

@allenwb If I may answer.

(This turned out quite long. Thanks in advance for reading!)

There are three issues with the static shape design. None of them are fatal.

Return overriding

The current ES6 class design allows the superclass to return arbitrary objects which may or may not be related to the derived class. Some users will probably expect that they can use the same technique, while making certain fields in the derived class "private". The static shape design does not support this.

I think we can all agree that returning an arbitrary object in this manner is rather ugly and distasteful. It's probably fine for subclasses with private state to not work for this use case. On the other hand, it might be a nice-to-have if we could support return overriding. (The DOM wants to use something along these lines, I believe.)

Reflect.construct

Reflect.construct allows us to invoke a constructor with an arbitrary NewTarget.

class C {
    #a = 1;
}

// `new Object()`, but with C as new.target
Reflect.construct(Object, [], C);

In this case, we get an object which has all of the instance slots of C, but which hasn't passed through C at any time. It will seem to be an instance of C, without any relationship at all to the initialization logic of C. This is a big problem for me.

We can probably address this by per-field TDZ. However, even with per-field TDZ, we have the possibility of allowing (unwanted) partially initialized instances:

// This class definition appears bug-free
class C {
    #a;
    #b;
    constructor() {
        this.#a = 1;
        this.#b = 2;
    }
    set a(value) { 
        this.#a = Number(value);
    }
}

// But I can fool it to create a partially constructed instance
let c = Reflect.construct(Object, [], C);
c.a = 1;
c;
/*
{  [[#a]]: 1,
   [[#b]]: uninitialized }
*/

This may not be a huge problem, but it's something a bit weird at any rate.

Prototype Mutation

Prototype mutation is the worst. Unfortunately, we have to deal with it. I believe that Node is defining it's Buffer class using a regular function and then mutating the prototype to point to UInt8Array. (@littledan correct me if I'm wrong.) So it appears that users are already taking advantage of the behavior illustrated in the original post. If we cannot make a breaking change, that means we have to adopt something like @allenwb 's patch upthread.

The patch might work for the simplest of cases, but I'm worried that it's just going to make a mess of things in general. For example, what if I have something like:

class A { } // No slots
class B extends A { #x; } // 1 slot
class C { }
C.__proto__ = B;
C.prototype.__proto__ = B.prototype;

It seems like the patch won't work for that case. And also, I really don't want to be patching something that was just landed last year.

And then there's the issue that I raised above, where if the prototype is mutated it can (again) leave us in a situation where an object has slots but the "home constructor" for those slots has never been executed. We can fix some of that by making the prototype immutable in certain cases.

Dynamic slots?

So when I take this all together, it seems that dynamic slots offer a clean solution to these problems:

  1. Return overriding: Works, but might result in a performance degradation.
  2. Reflect.construct: Not a problem. Since only the home constructor creates the slots, an object can't have the slots without at least trying to go through the home constructor.
  3. Prototype mutation: Works, with one caveat: if you want to make sure that your instance actually has all of the slots of the original superclass, you can't allow prototype mutation. That seems logical to me.

What about static shape? We actually still have static shape (with something like per-class TDZ) whenever:

  1. The complete set of internal slots in the constructor's prototype chain is unmodified, and
  2. No super constructor in the inheritance chain performs return overriding.

I'm sure we'd all like to get to a place where most classes obey these two rules. Perhaps a const class syntax as Mark has suggested in the past would be helpful in enforcing them.

littledan commented 8 years ago

Not sure what you mean by "per-class TDZ". That an exception would be thrown if you violated the return overriding rule?

Your description matches my understanding of Buffer, though Buffer-style subclassing would work under all of the proposals, since it's just monkey-patching an instance. I like this idea of using const classes for this purpose.

Speculating, maybe const classes could tie into the other proposal to make Object.prototype exotic in that its [[Prototype]] cannot be overridden, which might eventually introduce a new MOP op, or alternately could be triggered just by syntax like this. const classes could require that the extends clause is const, which would work if it's all based on a const Object.prototype.

zenparsing commented 8 years ago

Oh right, b/c Buffer doesn't do a super call. Eeek, I'm tired.

allenwb commented 8 years ago

@zenparsing

Return Override

A similar situation can already occur in ES6:

class MyRegExp extends RegExp {
   constructor() {return {__proto__: RegExp}}
}

new MyRegExp(".foo").test("xfoo");   //throws when inherited test method tries to access internal state

class MyOtherRegExp extends MyRegExp {
   constructor(...args) {
       super(...args);
       console.log(this.source);  //throws in inherited source method
    }
}

new MyOtherRegExp("some pattern");  //throws in constructor

Private slots throw in a similar manner if code in a subclass (constructor or anywhere) try to access a missing private slot.

There are many crazy things that people can code in JS. We don't have to try to make them do whatever such a crazy mind considers to be rational. We just need to make sure that memory safety is also enforced.

Reflect.construct

So, don't do this. Again, low level hacking must be memory safe but we can't turn things that are crazy at the application level into sanity. Or, as I say, we can enforce arbitrary application level "initialization" invariants".

BTW, slot declarations with initializes are in your sketch but not in mine :-). One of the reason of excluding such things (at least for now) is to avoid creating expectations such as this one.

Prototype Mutation

Prototype Mutation is crazy. My private slot proposal is memory safe even when somebody does crazy proto mutations. That's good enough.

That said, because the static and runtime semantics of private/protected slots are so tightly coupled to syntactic class declarations including the extends clause, I think it would b quite reasonable to say that instances of any class that defines or inherits private/protected slots are created with an immutable [[Prototyupe]]

Dynamic Slots

I still believe we don't need them. The problems they address are not things that need to be fixed.

erights commented 8 years ago

[+Dean, +Chip, +Michael]

The additional criteria to apply to crazy cases divides the roles: can a crazy adversary threaten the integrity of a sane defender?

I don't yet know specifically how this bears on these issues, but we need to keep it in mind.

Since I momentarily mentioned raised this over dinner yesterday with Kevin before realizing how stupid it is, I'll mention that I am not concerned about a defender who extends an adversary. Such a defender has already lost.

On Thu, Jan 28, 2016 at 8:41 AM, Allen Wirfs-Brock <notifications@github.com

wrote:

@zenparsing https://github.com/zenparsing

Return Override

A similar situation can already occur in ES6:

class MyRegExp extends RegExp { constructor() {return {proto: RegExp}} } new MyRegExp(".foo").test("xfoo"); //throws when inherited test method tries to access internal state class MyOtherRegExp extends MyRegExp { constructor(...args) { super(...args); console.log(this.source); //throws in inherited source method } } new MyOtherRegExp("some pattern"); //throws in constructor

Private slots throw in a similar manner if code in a subclass (constructor or anywhere) try to access a missing private slot.

There are many crazy things that people can code in JS. We don't have to try to make them do whatever such a crazy mind considers to be rational. We just need to make sure that memory safety is also enforced.

Reflect.construct

So, don't do this. Again, low level hacking must be memory safe but we can't turn things that are crazy at the application level into sanity. Or, as I say, we can enforce arbitrary application level "initialization" invariants".

BTW, slot declarations with initializes are in your sketch but not in mine :-). One of the reason of excluding such things (at least for now) is to avoid creating expectations such as this one.

Prototype Mutation

Prototype Mutation is crazy. My private slot proposal is memory safe even when somebody does crazy proto mutations. That's good enough.

That said, because the static and runtime semantics of private/protected slots are so tightly coupled to syntactic class declarations including the extends clause, I think it would b quite reasonable to say that instances of any class that defines or inherits private/protected slots are created with an immutable [[Prototyupe]]

Dynamic Slots

I still believe we don't need them. The problems they address are not things that need to be fixed.

— Reply to this email directly or view it on GitHub https://github.com/zenparsing/es-private-fields/issues/17#issuecomment-176276015 .

Cheers, --MarkM

zenparsing commented 8 years ago

Closing this issue, as we've reached consensus on the underlying model.