Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inaccurate definition of essential internal methods #2345

Open
raulsebastianmihaila opened this issue Mar 15, 2021 · 5 comments
Open

Inaccurate definition of essential internal methods #2345

raulsebastianmihaila opened this issue Mar 15, 2021 · 5 comments
Labels
needs consensus This needs committee consensus before it can be eligible to be merged. needs editorial changes

Comments

@raulsebastianmihaila
Copy link

The definitions of the [[GetPrototypeOf]] and [[SetPrototypeOf]] essential internal methods are not very accurate because they imply that an object always inherits from its prototype object, but that's not always the case. These are the current definitions:
[[GetPrototypeOf]] - Determine the object that provides inherited properties for this object. A null value indicates that there are no inherited properties.
[[SetPrototypeOf]] - Associate this object with another object that provides inherited properties. Passing null indicates that there are no inherited properties. Returns true indicating that the operation was completed successfully or false indicating that the operation was not successful.

There are objects that don't necessarily inherit from their prototype, such as proxy objects (and potentially other exotic objects), because there is no explicit invariant regarding inheritance in section 6.1.7.3 Invariants of the Essential Internal Methods. I give an example in this article: https://dev.to/raulsebastianmihaila/prototypal-noninheritance-in-javascript-b8f

The reason I'm bringing this up is that when I raised this bug regarding freezing (essentially regarding [[DefineOwnProperty]]), which was fixed in #666, the fix didn't change the wording in section 6.1.7.3 Invariants of the Essential Internal Methods for [[DefineOwnProperty]] to prevent the old bug. However Table 6: Essential Internal Methods did mention that if [[DefineOwnProperty]] returns true it means that the operation was successful, which can be interpreted such that freezing the object successfully (without error) must mean that now the existing non-configurable properties must also be non-writable. Without the wording in the table, this interpretation is not enforced. Therefore the table has the highest degree of importance with regard to expressing the semantics of the language. Without the table, just updating the algorithm and wording for proxies wouldn't have been enough because the same buggy behavior could in theory be present in other nonstandard exotic objects.

Therefore, either the table should be updated in a way that clarifies this aspect about non-enforced inheritance so to speak, or the existing behavior regarding inheritance in the context of proxies is incorrect. If I remember correctly there was a discussion in the past about a reform regarding the invariants that @claudepache was working on, to make the invariants clearer, but the spec wasn't updated in this regard.

@bakkot
Copy link
Contributor

bakkot commented Mar 15, 2021

The descriptions in the table aren't normative. They're intended to be a high-level summary of the purpose of the essential methods, not an exhaustive description of their behavior. (This came up recently in #2210 (comment), which observes that the description for [[Set]] is inaccurate for certain indices for typed arrays.)

If there are invariants which are missing from the 6.1.7.3 Invariants of the Essential Internal Methods, we should update that list. Is the invariant you're asking about something along the lines of If [[DefineOwnProperty]] returns *true*, then _P_ is considered to be observed to have the attributes of _Desc_.? If not, can you say what invariant you think is missing?

Therefore the table has the highest degree of importance with regard to expressing the semantics of the language.

Just to be maximally clear, the table has no importance with regard to expressing the semantics of the language (except for giving the list of methods and their signatures). The descriptions in it are strictly editorial.

Therefore, either the table should be updated in a way that clarifies this aspect about non-enforced inheritance so to speak, or the existing behavior regarding inheritance in the context of proxies is incorrect.

I'd be opening to adding a sentence prior to the table clarifying that the "Descriptions" just describe the intent of the method, rather than actually constraining behavior (which is what the Invariants section is for).

@raulsebastianmihaila
Copy link
Author

Is the invariant you're asking about something along the lines of If [[DefineOwnProperty]] returns true, then P is considered to be observed to have the attributes of Desc.

Something like that, yes. Your approach sounds good to me, but I think the entire table should be reviewed because I don't think this is the only one. For instance, I would expect [[Delete]] to have something like If [[Delete]] returns true, then P is considered to be a non-existent own property. This is relevant when extensibility comes into play.

@bakkot bakkot added needs editorial changes needs consensus This needs committee consensus before it can be eligible to be merged. labels Mar 15, 2021
@bakkot
Copy link
Contributor

bakkot commented Mar 15, 2021

Yup, agreed. Offhand I don't see any others, but I'll try to check more thoroughly.

I think these changes will need consensus from the committee, but I expect that should be easy to obtain.

@bakkot bakkot added editor call to be discussed in the next editor call and removed editor call to be discussed in the next editor call labels Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs consensus This needs committee consensus before it can be eligible to be merged. needs editorial changes
Projects
None yet
Development

No branches or pull requests

3 participants
@bakkot @raulsebastianmihaila and others