Discuss
  • get/setIntegrity trap (Was: A case for removing the seal/freeze/isSealed/isFrozen traps)

get/setIntegrity trap (Was: A case for removing the seal/freeze/isSealed/isFrozen traps)

# Tom Van Cutsem (12 years ago)

2013/2/18 Andreas Rossberg <rossberg at google.com>

On 16 February 2013 20:36, Allen Wirfs-Brock <allen at wirfs-brock.com> wrote:

It's to simplify the MOP and that simplification is directly reflected as a simplification to the Proxy hander interface. Instead of 6 traps (preventExtensions, isExtensible, freeze, isFrozen, seal, isSealed) only two are needed.

Also, having an explicit frozen object state simplifies some of the object invariants which would otherwise perform explicitly specified accesses to the target object which would be observable (if the target is itself a proxy).

Well, that is either a breaking change, such that implementations can not actually be lazy about it, or it doesn't really remove complexity, since you still need to infer the state as a fallback (i.e., it's just an optimisation).

I don't necessarily oppose making that breaking change, but we have to be aware that, even though it's an optimisation, the change is yet another complication of the object model. The additional state modifies the meaning of per-property descriptors on a second level. IIUC, defineProperty now has to check against that state, and getOwnPropertyDescriptor somehow has to take it into account, too. For direct proxies, the respective traps have to extend their validation logic. Overall, not a simplification, as far as I can see.

I've been thinking some more about get/setIntegrity and I've come to the same conclusion. While get/setIntegrity gets rid of 4 traps (sealed/isSealed/frozen/isFrozen), it does so at the expense of extra complexity in other parts of the MOP, in particular, adding yet more state to check and update in [[GetOwnProperty]] and [[DefineOwnProperty]].

Allen, in light of this, wouldn't you agree that it's better to keep the extra traps?

# Allen Wirfs-Brock (12 years ago)

On Feb 20, 2013, at 9:24 AM, Tom Van Cutsem wrote:

2013/2/18 Andreas Rossberg <rossberg at google.com> On 16 February 2013 20:36, Allen Wirfs-Brock <allen at wirfs-brock.com> wrote:

It's to simplify the MOP and that simplification is directly reflected as a simplification to the Proxy hander interface. Instead of 6 traps (preventExtensions, isExtensible, freeze, isFrozen, seal, isSealed) only two are needed.

Also, having an explicit frozen object state simplifies some of the object invariants which would otherwise perform explicitly specified accesses to the target object which would be observable (if the target is itself a proxy).

Well, that is either a breaking change, such that implementations can not actually be lazy about it, or it doesn't really remove complexity, since you still need to infer the state as a fallback (i.e., it's just an optimisation).

I don't necessarily oppose making that breaking change, but we have to be aware that, even though it's an optimisation, the change is yet another complication of the object model. The additional state modifies the meaning of per-property descriptors on a second level. IIUC, defineProperty now has to check against that state, and getOwnPropertyDescriptor somehow has to take it into account, too. For direct proxies, the respective traps have to extend their validation logic. Overall, not a simplification, as far as I can see.

I've been thinking some more about get/setIntegrity and I've come to the same conclusion. While get/setIntegrity gets rid of 4 traps (sealed/isSealed/frozen/isFrozen), it does so at the expense of extra complexity in other parts of the MOP, in particular, adding yet more state to check and update in [[GetOwnProperty]] and [[DefineOwnProperty]].

Allen, in light of this, wouldn't you agree that it's better to keep the extra traps?

Actually, no. Reducing API complexity (in this case, the Proxy handler API) at the expense of a little bit of added spec. complexity seems like a very reasonable trade-off. Plus, we are talking about spec. complexity, not necessarily implementation complexity. An implementation is free to distribute the complexity however it chooses. For example, if an implementation always sets all of an objet's property descriptors to configurable: false when [SetIntegrity] is performed (which Object.freeze is specified to do in ES5) then there shouldn't be any extra work that needs to be done in the ordinary implementations of [[GetOwnProperty]] or [[DefineOwnProperty]].

Also, I'm concerned that we have been overloading the meaning of the [[Extensible]] state by hanging other meanings off of it, because it is the only "integrity state" we currently have at the object level. For example, we have tentative agreement at the timevalue of Date objects can not be modified if [[Extensible]] is false and that RegExp.prototype.compile will not update an RegExp instance if [[Extensible]] is false. Both of these seem like things that would be better to associate with an explicit "frozen" object state. We are also, from ES5, using [[Extensible]] to control whether [[Prototype]] can be modified. This is a case where I think a new state might be appropriate as it seems very reasonable for an object to want to fix [[Prototype]] but still allow own properties to be added

Finally, I still think we should further consider the feasibly of a breaking change where

Object.preventExtensions(obj);
for (let k of Object.getOwnPropertyKeys(obj))
  Object.defineProperty(obj,k,{configurable:false});

is no longer equivalent to

Object.freeze(obj)

in terms of causing Object.isFrozen(obj) to return false.

I think I previously asked if anybody is aware of situations where Object.isFrozen tests are done but Object.freeze is not used to set objects to the frozen state. So far, no answers. Anybody?

Allen

Cheers, Tom

-------------- next part -------------- An HTML attachment was scrubbed... URL: esdiscuss/attachments/20130220/ea2b2fc2/attachment

# Nathan Wall (12 years ago)

Allen Wirfs-Brock wrote:

I think I previously asked if anybody is aware of situations where  Object.isFrozen tests are done but Object.freeze is not used to set objects to the frozen state. So far, no answers. Anybody?

Speaking just from my own experience as a user of ES5, I have not found Object.isFrozen to be helpful yet.  If I want to see if I can configure a property, I check Object.isExtensible and !Object.prototype.hasOwnProperty.call(obj, key) || Object.getOwnPropertyDescriptor(obj, key).configurable.

Object.isFrozen and Object.isSealed don't really seem that helpful to me for the very reasons you've discussed: They don't represent any real object state, so they don't accurately tell me what can be done with an object.  If I could I would argue in favor of their removal, though I know it's too late for that.

I would be curious to see legitimate uses of isFrozen and isSealed in existing code if anyone has anything to offer.

Nathan

# Kevin Reid (12 years ago)

On Wed, Feb 20, 2013 at 11:52 AM, Nathan Wall <nathan.wall at live.com> wrote:

Object.isFrozen and Object.isSealed don't really seem that helpful to me for the very reasons you've discussed: They don't represent any real object state, so they don't accurately tell me what can be done with an object. If I could I would argue in favor of their removal, though I know it's too late for that.

I would be curious to see legitimate uses of isFrozen and isSealed in existing code if anyone has anything to offer.

I just took a look at uses of Object.isFrozen in Caja and I find that all but one are either in tests (test that something is frozen) or in sanity checks (if this isn't frozen, do not proceed further, or freeze it and warn).

The remaining one is in a WeakMap abstraction used for trademarking: an object cannot be given a trademark after it is frozen. (The rationale here, while not written down, I assume is that a defensive object's “interface” should not change, and it is an implementation detail that this particular information is not stored in the object.) There is a comment there suggesting we might strengthen this check to only permitting extensible objects to be marked.

# David Bruant (12 years ago)

Le 20/02/2013 21:08, Kevin Reid a écrit :

On Wed, Feb 20, 2013 at 11:52 AM, Nathan Wall <nathan.wall at live.com <mailto:nathan.wall at live.com>> wrote:

`Object.isFrozen` and `Object.isSealed` don't really seem that
helpful to me for the very reasons you've discussed: They don't
represent any real object state, so they don't accurately tell me
what can be done with an object.  If I could I would argue in
favor of their removal, though I know it's too late for that.

I would be curious to see legitimate uses of `isFrozen` and
`isSealed` in existing code if anyone has anything to offer.

I just took a look at uses of Object.isFrozen in Caja and I find that all but one are either in tests (test that something is frozen) or in sanity checks (if this isn't frozen, do not proceed further, or freeze it and warn).

The remaining one is in a WeakMap abstraction used for trademarking: an object cannot be given a trademark after it is frozen. (The rationale here, while not written down, I assume is that a defensive object's "interface" should not change, and it is an implementation detail that this particular information is not stored in the object.) There is a comment there suggesting we might strengthen this check to only permitting extensible objects to be marked.

And in an ES6 world, you'll probably use an actual WeakMap anyway?

Thanks for sharing this experience from Caja,

# Kevin Reid (12 years ago)

On Wed, Feb 20, 2013 at 12:15 PM, David Bruant <bruant.d at gmail.com> wrote:

And in an ES6 world, you'll probably use an actual WeakMap anyway?

Using an actual WeakMap does not change matters: the intent is that after Object.freeze(o), you can't add new trademarks to o. Since the trademark info is not stored on the object but in the WeakMap (whether emulated or actual), we have to add an explicit test.

If 'private properties' (in whatever form they come to ES6) were available to us, then it would be natural to use them instead for this purpose (at least, so it seems to me at the moment) and so we would not need a test since non-extensibility would presumably reject the addition of a new private property.

# Mark S. Miller (12 years ago)

(I do not yet have an overall opinion about get/setIntegrity. Here, I'm just answering and clarifying, not advocating.)

On Wed, Feb 20, 2013 at 10:57 AM, Allen Wirfs-Brock <allen at wirfs-brock.com>wrote: [...]

Also, I'm concerned that we have been overloading the meaning of the [[Extensible]] state by hanging other meanings off of it, because it is the only "integrity state" we currently have at the object level. For example, we have tentative agreement at the timevalue of Date objects can not be modified if [[Extensible]] is false and that RegExp.prototype.compile will not update an RegExp instance if [[Extensible]] is false. Both of these seem like things that would be better to associate with an explicit "frozen" object state.

Another instance of the same issue in a different guise is the restriction in Object.observe (for ES7) that you cannot obtain from a frozen object the right to notify its observers. In this case, in order to be more permissive, we did attach the restriction to frozenness rather than just extensibility. However, when frozenness is a complex test for a pattern of restrictions that may or may not be accidental, this seems weird.

In other words, if an object is purposely frozen, no problem. But let's say that in v1 of a program, object A is non-extensible, its foo property is non-configurable, non-writable, and its bar property is writable. A later refactoring realizes it doesn't need the bar property, and so deletes it. It is weird that this would break previously correct code to obtain the right to notify observers.

We are also, from ES5, using [[Extensible]] to control whether [[Prototype]] can be modified. This is a case where I think a new state might be appropriate as it seems very reasonable for an object to want to fix [[Prototype]] but still allow own properties to be added

While sensible in theory, I don't think that case is worth yet another state bit. If, like Object.observe, we had attached this restriction to frozenness, we could not consider attaching this to an explicit frozenness bit.

Finally, I still think we should further consider the feasibly of a breaking change where

Object.preventExtensions(obj);
for (let k of Object.getOwnPropertyKeys(obj))
  Object.defineProperty(obj,k,{configurable:false});

is no longer equivalent to

Object.freeze(obj)

in terms of causing Object.isFrozen(obj) to return false.

I think I previously asked if anybody is aware of situations where Object.isFrozen tests are done but Object.freeze is not used to set objects to the frozen state. So far, no answers. Anybody?

I know of no uses of Object.isFrozen beyond those Kevin lists. None of these would be broken by this change.

As for the rationale for the trademark restriction, more on that in a separate email.

-- Cheers, --MarkM -------------- next part -------------- An HTML attachment was scrubbed... URL: esdiscuss/attachments/20130220/0720ea1c/attachment

# Brendan Eich (12 years ago)

Allen Wirfs-Brock wrote:

Actually, no. Reducing API complexity (in this case, the Proxy handler API) at the expense of a little bit of added spec. complexity seems like a very reasonable trade-off. Plus, we are talking about spec. complexity, not necessarily implementation complexity.

Sure, but the spec is "the first implementation". If implementors will not add another state bit (I predict they won't) then the spec is doing a disservice here.

The Proxy API is complex already, due to ES5 in this case. It is not that much less complex with setIntegrity. I don't think the trade-off is worth it, and I suggest the "all implementors ignore the spec complexity" condition is one to avoid.

# Tom Van Cutsem (12 years ago)

2013/2/20 Allen Wirfs-Brock <allen at wirfs-brock.com>

Finally, I still think we should further consider the feasibly of a breaking change where

Object.preventExtensions(obj);
for (let k of Object.getOwnPropertyKeys(obj))
  Object.defineProperty(obj,k,{configurable:false});

is no longer equivalent to

Object.freeze(obj)

in terms of causing Object.isFrozen(obj) to return false.

I think I previously asked if anybody is aware of situations where Object.isFrozen tests are done but Object.freeze is not used to set objects to the frozen state. So far, no answers. Anybody?

I did a little code search via GitHub on uses of Object.isFrozen in JS code. The large majority seems to occur in test cases (incl. Test262) or libraries involving ES5 shims for ES3. There's no doubt this breaking change will get noticed, as Test262 contains code such as:

assertEq(Object.isFrozen(Object.preventExtensions({})), true);

However, here and there you can find some code that branches on frozenness, although it's not always clear what the rationale behind it is, e.g.:

petekinnecom/portfolio_chaplin/blob/b241300968fe6011c54548f24e0c8cfcd5d6d663/app/views/tube_view.coffee#L39

That said, I don't think this is enough evidence either for or against the breaking change.

Cheers, Tom -------------- next part -------------- An HTML attachment was scrubbed... URL: esdiscuss/attachments/20130221/62c90b0e/attachment

# Brendan Eich (12 years ago)

Tom Van Cutsem wrote:

That said, I don't think this is enough evidence either for or against the breaking change.

I have a hard time believing we can break ES5. It has been shipping for years (plural, at least in one case) in major browsers that evergreen their user bases.

# Tom Van Cutsem (12 years ago)

Allen's latest draft (Rev. 14) contains the change where [[Freeze]],[[Seal]] and [[PreventExtensions]] have been consolidated into [[HasIntegrity]]/[[SetIntegrity]]. While no changes were made to the Proxy API (i.e. no has/getIntegrity traps yet), the definition of Object.{freeze,seal,preventExtensions} did change, and this is sufficient to expose an incompatibility with ES5, namely:

Object.isFrozen(Object.preventExtensions({})) // true in ES5, false in ES6 Rev14 draft

I still feel like the consolidation isn't worth this incompatibility.

Allen, could you clarify what your intent is? Is it your intent that this incompatibility will be fixed with further spec changes?

Cheers, Tom

2013/2/21 Brendan Eich <brendan at mozilla.com>

# André Bargull (12 years ago)

The incompatibility you've noticed is just a spec bug in [[HasIntegrity]]. In step 2a of 8.3.3, the value of [[Extensible]] needs to be inverted. With that change applied, the code snippet will return true.

  • André
# Allen Wirfs-Brock (12 years ago)

Tom recently suggested that that we really don't need MOP-level or trap operations from freezing, sealing and testing those states. Also, there seems to be minimal support for having explicit freeze/sealed integrity states or for adding integrity integrity states. So I'm probably going to go make to an style design. We'll only have [[IsExtensible]] and [[PreventExtensions]] MOP/trap/Reflect operations. Object.freeze/seal/isFrozen/isSealed will be derived operations.

# Tom Van Cutsem (12 years ago)

Ok, somehow I had completely missed "9.3.10 TestIntegrityLevel (O, level)" which does exactly the derived computation for sealed and frozen objects.

I think André is right though about the bug in 8.3.3 step 2.a.

Cheers, Tom

2013/3/17 Allen Wirfs-Brock <allen at wirfs-brock.com>

# Tom Van Cutsem (12 years ago)

Allen and I had a conversation that clarified things.

Essentially, the plan is to only have [[PreventExtensions]]/[[IsExtensible]] internal MOP methods, and to only have the corresponding preventExtensions/isExtensible traps for proxies.

Proxies won't have isSealed, isFrozen, seal and frozen traps and there will be no corresponding Reflect.{isSealed,isFrozen,seal,frozen} methods.

Calling Object.freeze(proxy) instead triggers the proxy's getOwnPropertyNames trap and then updates each own property of the proxy via defineProperty. Similar story for Object.isFrozen(proxy).

Pro:

  • We get rid of 4 MOP methods ([[Freeze]],[[Seal]],[[IsFrozen]],[[IsSealed]]). This simplifies both the internal MOP and the Proxy API.
  • Freezing and sealing always behave consistently, even in the face of proxies. There's no chance for a proxy to implement multiple traps inconsistently.

Con:

  • Object.freeze(proxy) and Object.isFrozen(proxy) must observably iterate over all own properties of the proxy (same for seal/isSealed).

Cheers, Tom

2013/3/18 Tom Van Cutsem <tomvc.be at gmail.com>