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

Classes extending a function with .prototype=null #699

Closed
bakkot opened this issue Sep 28, 2016 · 14 comments · Fixed by #755
Closed

Classes extending a function with .prototype=null #699

bakkot opened this issue Sep 28, 2016 · 14 comments · Fixed by #755

Comments

@bakkot
Copy link
Contributor

bakkot commented Sep 28, 2016

Currently (see ClassDefinitionEvaluation 6.g.i, 10.a, and 15), class B below is considered to be a base class.

function A(){ this.x = 1; }
A.prototype = null;

class B extends A {}

From what I can tell, this means that A will never be invoked - i.e., this is identical to class B extends null {}, except that the [[Prototype]] of B will be A. This seems wrong. I believe instead that 10.a and 15 should read "if superclass is not null" instead of referring to "protoParent".

@bakkot
Copy link
Contributor Author

bakkot commented Sep 28, 2016

Note that this would be a normative change, but both SpiderMonkey and v8 currently treat B as being derived (as I think they ought to), contrary to the current spec.

@littledan
Copy link
Member

+1 this looks like a bug to me

@getify
Copy link
Contributor

getify commented Oct 18, 2016

Just curious: what does this mean for the concept of which is the "class"... is it the function (constructor) that's the "class", or is it the prototype object that's the "class"? I think from my experience that most people think of the prototype object as the class, since it's where all the "inherited" functionality derives from.

@bakkot
Copy link
Contributor Author

bakkot commented Oct 18, 2016

@getify: both the constructor and its .prototype play a role (normally, except in the case this issue is pointing out, which is probably a spec bug). This is true in exactly the same way for functions as for classes, whether instantiating or inheriting.

@getify
Copy link
Contributor

getify commented Oct 18, 2016

but my point is, if "superclass" is meant to refer only to the constructor and not the prototype, that muddies the waters... if "class" means both, but we're using only one or the other as the null check, we shouldn't call that specific one, only, a "superclass".

@bakkot
Copy link
Contributor Author

bakkot commented Oct 18, 2016

I'm not sure what uses of those terms you're referring to.

In this specific algorithm, superClass is the result of evaluating the class heritage. In general, I'd normally use 'class' to refer to the constructor itself (which is in accord with superClass here), but the constructor's .prototype is an important component of said constructor.

@allenwb
Copy link
Member

allenwb commented Oct 18, 2016

I need to do some research on the history of this detail, but my first reaction is that this is not necessarily a bug.

I have a vague recollection that this use case was on that was discussed at a TC39 meeting, but I'll have to do some research in the meeting notes (and perhaps ES6 drafts or bugs.ecmascript.org) to verify that.

Regardless, here is a rationale for the currently specified behavior:

Setting the prototype of a function (possibly a class constructor function) to null and then using that function in a class extends is just like explicitly using null in the extends clause except that the subclass inherits static properties from the function. This essentially defines an abstract class that extends that the behavior of Function.prototype (which manifests as inherited static methods of subclasses) while ensuring that subclass instances do not inherit from Object.prototype.

However, here is something that is precluded by the present spec. language:

class BareExoticArray extends Array {
   //I want to create exotic array objects that don't have any of the Array.prototype methods
   construtor () {
      super();
   }
}
BareExoticArray.prototype = null;

According to the spec, new BareExoticArray() will throw upon returning from the super() call because this has already been bound since BareExoticArray is marked as a base constructor.

Alternative, is we had just left the constructor out of the class definition:

class BareExoticArray extends Array {
   //I want to create exotic array objects that don't have any of the Array.prototype method
}
BareExoticArray.prototype = null;

We would not throw when invoking new BareExoticArray() but we would have created an ordinary object, rather than the exotic Array object that was expect.

I'm pretty sure I didn't consider this exotic object subclass use case when write the original spec. for ClassDefinitionEvaluation. So I'm starting to think maybe this is a bug.

This is a pretty obscure corner case and the "right" or most desirable behavior is probably debatable. In situations like this the most important thing is consistency of implementations. For such non-intuitive edge cases it is more important that implementations all conform to what the spec. says and less important whether or not the spec. "got it right".

I think it is reasonable to treat this as a bug, and try to "get it right". Or not. Most important is getting a spec. and implementations that all match.

@allenwb
Copy link
Member

allenwb commented Oct 18, 2016

@getify We can meaningfully talk about "classes", "superclasses", and "subclasses" as long as we are referring to syntactic class definition and the runtime object they generate. But those terms start to loose their meaning as soon as somebody starts doing meta-level tampering (eg, modifying prototype or `proto of the constructor function or the prototype object) with the runtime object created by class definitions.

@getify
Copy link
Contributor

getify commented Oct 18, 2016

@allenwb if the spec says "if superclass is not null" and what it only means is there is a non-null constructor function (regardless of its prototype), that implies the class is just its constructor, since prototype can be empty but we still have a "superclass".

I think the most consistent (wrt existing precedent) approaches to this spec wording would be:

  1. I think it should be that "if superclass is not null" means both constructor and its prototype must be non-null. This reinforces the idea that a superclass is both, not just one or the other. IOW, extending a function with null prototype shouldn't be allowed.
  2. if intent is to check ONLY for the non-null prototype, say "if superclass prototype is not null".
  3. if intent is to check ONLY for the non-null constructor function, say "if superclass constructor is not null".

@bakkot
Copy link
Contributor Author

bakkot commented Oct 18, 2016

@allenwb, I don't think BareExoticArray would be marked as a base constructor, since Array.prototype is not null. (The constructor is marked as base or derived at class definition time, and afaict does not change.) (Edit: and in fact BareExoticArray would work exactly as you want it to, I believe.) But

new class DerivedBareExoticArray extends BareExoticArray {}

would not create an Array exotic object, since Derived would, counterintuitively (to me), be marked as a base class because BareExoticArray.prototype === null.

I think it is surprising in general to have a class which extends something constructible and then does not call that thing during construction.

@bakkot
Copy link
Contributor Author

bakkot commented Oct 18, 2016

@getify, superClass is the name of a variable in a spec algorithm which currently resolves to the value returned by evaluating the 'extends' clause; it can't refer to both the parent and the parent's prototype.

We could rename it, or have the algorithm check both, in principle (although checking both would be pointless - if an object is null, it does not have a .prototype property).

@allenwb
Copy link
Member

allenwb commented Oct 18, 2016

@bakkot You're right that the BareExoticArray with the explicit constructor will work and that it would not be marked as a Base constructor.

I think the change that would allow the case without the explicit constructor to allow work is to change step 15 to:

If ClassHeritageopt is present and superclass is not null, then set F.[[ConstructorKind]] to "derived".

Note that I'm intentionally using superclass there and not constructorParent.

@bakkot
Copy link
Contributor Author

bakkot commented Oct 18, 2016

@allenwb, I think your example works without an explicit constructor as well, because in your example protoParent is indeed not null.

To make my DerivedBareExoticArray work, step 15 would indeed need to be changed as you suggest, with the same change to 10.a to make the implicit constructor case work. That's the fix I proposed when filing.

@allenwb
Copy link
Member

allenwb commented Oct 19, 2016

yes, need to also use superclass in 10a.

This is actually a bit hooky in that superclass was originally intended to be local to the step 6 else clause. It doesn't have a defined value if step 6 isn't evaluated. The "classHeritage is present" predicates should be an adequate guard to prevent that from being a problem.

Perhaps, this should be cleaned up but need to be careful to maintain the distinction between superclass and constructorParent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants