Begin forwarded message:

> From: Tom Van Cutsem <[email protected]>
> Date: November 20, 2012 11:36:24 AM PST
> To: Allen Wirfs-Brock <[email protected]>
> Cc: "Mark S. Miller" <[email protected]>, Jason Orendorff 
> <[email protected]>
> Subject: Re: possible excessive proxy invariants for Object.keys/etc??
> 
> 2012/11/19 Allen Wirfs-Brock <[email protected]>
> On Nov 19, 2012, at 10:04 AM, Tom Van Cutsem wrote:
>> The copying is to ensure:
>> a) that the result is an Array
> 
> Why is Array-ness essential?  It was what ES5 specified, but ES5 had a fixed 
> implementations of Object.getOwnPropertyNames/keys so that requirement was 
> simply a reflection of what the  spec's algorithms actually did.  Most places 
> in the specification that consume an "array" only require array-like-ness, 
> not an actual array.  Now that we're make the implementation of the 
> getOPN/keys extensible, it would be natural to relax the requirement that 
> they produce a  [[Class]]==="Array" object.
> 
> It's all a matter of developer expectations and how much leeway we have in 
> changing the "return type" of existing built-ins.
> In theory, it's a backwards-incompatible change. In practice, it may not 
> matter.
>> b) that all the elements of the result are Strings
> 
> And presumably Symbols.  We have to also accommodate Symbols, at least for 
> getOwnPropetyNames.  Regardless, why is this important?  More below...
> 
> Same argument as above.
> 
> I recall there was some concern about symbols showing up in existing 
> reflection methods like Object.getOwnPropertyNames. Not sure how that got 
> resolved. It's basically touching upon the same issue of whether we can 
> change the return type of existing methods.
>  
> 
>> c) to ensure the stability of the result.
>> 
>> You can think of a + b as implementing a type coercion of the trap result to 
>> "Array of String". This coercion is not too dissimilar from what the 
>> getOwnPropertyDescriptor has to do (normalization of the returned property 
>> descriptor by creating a fresh copy).
> 
> Yes, premature type coercion, in my opinion. Also, a classic performance 
> mistakes made by dynamic language programmers:  unnecessary coercion of 
> values that are never going to be access or redundant coercion checks of 
> values that are already of the property type.  Why is it important to do such 
> checks on values that are are just passing through these traps.  When and if 
> somebody actually gets arounds to using one of the elements that are returned 
> as a property key they will be automatically coerced to a valid value.  Why 
> is it important that it happens any sooner than that.
> 
> I don't know if you are familiar with the work of Felleisen et al. on 
> higher-order contracts. In that work, they use the notion of "blame" between 
> different components/modules. Basically: if some module A receives some data 
> from a module B, and B provides "wrong" data, then B should be assigned 
> blame. You don't want to end up in a situation where A receives the blame at 
> the point where it passes that "wrong" data into another module C.
> 
> The "early" type coercions at the exit of the traps can be thought of in 
> these terms: the invariant checks will always blame the proxy handler for 
> producing wrong data, at the point where the wrong data is produced and 
> before it's exposed to client code. Clients are expecting ES5/6 objects to 
> obey an implicit "contract".
>  
> getOwnPropertyDescriptor is also on my list to talk to you about for similar 
> reasons.  I don't see why the property descriptor needs to be normalized and 
> regenerated (including converting accessor properties on the descriptor into 
> data properties and requiring the [[Prototype]] to be object.  I have an 
> alternative for [[GetOwnProperty]]/TrapGetOwnPropertyDescriptor that 
> preserves the original object returned from the trap (while normalizing to a 
> descriptor record for any internal calls on ordinary objects).  This 
> preserves any exotic property descriptor object properties as is.  This seems 
> like exactly the right things.  Such exotic property descriptor properties 
> can only be meaningfully used by other proxy traps or application/library 
> code that knows about them.  I don't see any need to normalize them into data 
> properties or otherwise muck with what the actually implementor of the 
> getOwnPropertyDescrptor trap choose to return. 
> 
> I this alternative implementation is, again, much simpler (and efficient) 
> with fewer externally observable calls in its algorithm steps.  I sure you 
> will want to review it so I will include it in the spec. draft.
> 
> It's all a matter of how much invariants we care to give up.
> 
> I think it's risky to allow descriptors to be returned whose attributes may 
> be accessors, thus potentially fooling much existing ES5 code that often does 
> simple tests like:
> 
> if (!desc.writable && !desc.configurable) {
>   var val = desc.value; // we can assume the property is immutable, so we can 
> cache its value
>   ...
> }
> 
> You can no longer rely on the correctness of such code if desc can't be 
> guaranteed to be a plain old object containing only data properties.
>> 
>> c) on the other hand is crucial for the 
>> non-configurability/non-extensibility checks mentioned below. It's no use 
>> checking some invariants on a data structure if that data structure can 
>> later still be mutated.
> 
> I don't see how an extra copy of the the result array makes any difference in 
> this regard  (also see below).   The array returned from 
> Object.getOwnPropertyNames is still mutable even after you make a copy. It's 
> only as stable as any other non-frozen object you are likely to encounter. 
> 
> The returned copy, even if it is mutable, is fresh, meaning that *no one 
> else* yet has a pointer to it. Hence it can't be modified by the proxy 
> handler.
> 
> If we don't make a copy, but we do check the result for invariants, here's 
> what may happen:
> 1) proxy checks whether the returned array contains all non-configurable 
> properties of the target
> 2) let's assume this is the case, so the proxy returns this array to the 
> client
> 3) the proxy handler kept a reference to the array, and mutates it, e.g. by 
> removing a non-configurable property
> 4) client is now wrongly under the assumption that the returned array 
> contains all non-configurable properties
>  
>>  
>> 3) Every name in the list returned by the trap code is looked up on the 
>> target to determine whether or not it exists, even if the target is 
>> extensible.   Each of those lookup is observable (the target might itself be 
>> a proxy) so, according to the algorithm they all must be performed.
>> 
>> This is where we get into actual checks required to enforce 
>> non-configurability/non-extensibility.
>> 
>> Granted, the ES5 spec is not clear about the invariants on 
>> getOwnPropertyNames and keys. The currently specified invariants are a 
>> common-sense extrapolation of the existing invariants to cover these 
>> operations.
> 
> Yes, but if they aren't essential then we have to take into account the cost 
> of doing non-essential invariant checks.  These costs are both the actual 
> runtime cost of doing the checks, the constraints upon implementation imposed 
> by the need for complete and properly sequenced occurrence of all potentially 
> externally observable algorithm steps, and finally (and least important) the 
> added complexity of the specification which makes it harder to actually 
> achieve interoperable implementations.
> 
> I agree about the cost. The question is whether we regard these invariants as 
> essential or not. To me, they are the common-sense invariants an ES5 
> programmer would expect.
>  
>  
>> In practice, it determines the degree of confidence that a programmer can 
>> have in Object.getOwnPropertyNames and friends when dealing with a frozen 
>> object. If we waive these invariant checks, then the result of those 
>> operations can never be trusted on to reliably introspect on a frozen 
>> object's list of property names:
>> 
>> Object.isFrozen(proxy) // true
>> Object.getOwnPropertyNames(proxy) // ['foo']
>> Object.getOwnPropertyNames(proxy) // [ ]
>> 
>> Here, the 'foo' property apparently disappeared on a frozen object.
> 
> Yes, but copying the result an extra time doesn't ensure that two separate 
> calls produce the same result. 
> 
> It's not the copying alone that guarantees it, it's the invariant check that 
> 'foo' is in the result + the copying that guarantees it.
>  
>> 
>> If neither the for-in loop nor Object.getOwnPropertyNames nor Object.keys 
>> can reliably report an object's own properties, then we've made it 
>> impossible to reliably traverse and inspect a presumably deep-frozen object 
>> graph.
> 
> If that is the goal, that maybe what you need is a new operation that cannot 
> be trapped.  Your algorithm is essentially doing all the work, and more, 
> necessary to generate an accurate report of the actual non-configurable 
> properties of the target.  If that is what is needed for your deep frozen 
> traversals, add that operation.  That seems better than allowing the Proxy 
> trap to do the work to report anything it wants, but then coming in after the 
> fact and checking whether what was report matches what you demand.  In this 
> case, it seems preferable to just provide the results you demand without any 
> handler intervention. 
> 
> This seems to harken back to the original proxy design that essentially 
> turned frozen objects into regular objects with no handlers. If that is what 
> you need, you should provide access to the results you need for the 
> situations where you need them.
> 
> But you should not be adding this overhead for Proxy uses that don't have 
> those requirements.
> 
> The problem with, say, a non-trappable [[GetNonConfigurableOwnProperties]] 
> internal method, is that membranes can't adapt. We were previously in this 
> situation with the [[Extensible]] internal property. Eventually we ended up 
> trapping Object.isExtensible and Object.preventExtensions, even though these 
> traps can't actually return a result that is different from applying the 
> operation directly to the target. They're still useful to trap, as the proxy 
> may then respond to these operations and change the state of its own target 
> before returning a result.
> 
> I'm pretty sure introducing a new non-trappable method is going to lead to 
> the same problems for membranes.
>>  
>> 4) Every own property of the target, is observably looked up (possibly a 
>> second time) even if the object is extensible  and has no non-configurable 
>> properties.
>> 
>> We may be able to remove the redundancy of two lookups by restructuring the 
>> algorithm.
>> There previously was some redundancy in other checks as well.
>>  
>> It isn't clear to me if any of this work is really necessary to ensure 
>> integrity.  After all, what can you do with any of these names other than 
>> use them as the property key argument to some other trap/internal method 
>> such as [[SetP]], [[DefineOwnProperty]], etc.  Called on a proxy, those 
>> fundamental operations are going to enforce the integrity invariants of the 
>> actual properties involved so the get name checks doesn't really seem to be 
>> adding anything essential.
>> 
>> Perhaps we can just get rid of all the above checking.  It seems like a good 
>> idea to me.
>> 
>> Alternatively,  it suggests that a [[GetNonConfigurablePropertyNames]] 
>> internal method/trap would be a useful call to have as the integrity 
>> invariants only care about non-configurable properties. That would 
>> significantly limit the work in the case where there are none and limit the 
>> observable trap calls to only the non-configurable properties.
>> 
>> That would be one way to speed things up.
>> 
>> I share your concern that the current spec algorithm may be a bit too 
>> restrictive for implementations to allow optimizations. It's very 
>> imperative. We should ask the spec implementors to have a look at the draft 
>> spec and share their concerns.
> 
> Maybe we should move this discussion to es-discuss.  What do you think?
> 
> Yes, I would be happy to continue the discussion in public.
> 
> Cheers,
> Tom

_______________________________________________
es-discuss mailing list
[email protected]
https://mail.mozilla.org/listinfo/es-discuss

Reply via email to