[WIP es-classes] Fix extending from ES Classes

[pzuraq]

This PR seeks to fix the behavior of ES classes, specifically when calling .extend() on a native class. Currently, a new base class constructor (makeCtor) is made each time we call .extend(). With ES classes, this no longer works as expected because the base class does not ever call it's super constructor, breaking both the constructor of the native class and (in the future) class fields, which are assigned in the constructor.

The changes here detect if the class being extended from is actually a makeCtor, and if not provide a simple anonymous class that extends from the superclass instead. This allows the constructor to follow the prototype chain upward instead to whatever the last real makeCtor was, which then does all of the work of assigning properties, running proto, etc. In order to do this, metadata such as wasApplied had to be moved to the meta object for the object (see #15580) and static methods such as proto had to remove direct references to the class in makeCtor itself so they could be reused for upstream classes.

TODO:

• Make sure this is the right strategy for fixing cross-compatibility
• Cleanup code (some quirky/hacky things made it in)
• Throw an error when trying to reopen native classes
• More tests
  • Testing reopen and reopenClass, particularly when they retrigger a proto
  • Testing reopen and reopenClass fail on native classes
• Perf benchmarks

[pzuraq]

This is a bit of a hack, I found that in some cases (particularly ObjectProxy) the following would be true:

let m1 = meta(this);
this.constructor.proto();
let m2 = meta(this);

m1 !== m2

For some reason, proto can confuse meta this way (something to do with the way WeakMap links between objects and values?). On the otherside, if we were to do:

meta(Object.getPrototypeFor(this)).wasApplied

The meta for a native class does not have proto set (it was created outside of extend, no chance to add it) so the check in the getter/setter for wasApplied fails.

I think ideally we only load meta once, and just use the parent value.

[rwjblue]

Make sure this is the right strategy for fixing cross-compatibility

I believe that this is indeed going down the right path...

[rwjblue]

@pzuraq - Mind rebasing on top of the "single constructor arg" work?

[pzuraq]

Rebased! I should have some time later on this week to work on this so let me if there are any specific changes I should make. I want to try to get rid of the meta() twice bug (or at least find the root cause), other than that I think this is in a pretty good spot actually (just needs more tests)

[pzuraq]

Alright, this is ready for review! Turned out the meta bug was not a bug, meta changes because we wrap self in a proxy (which makes sense).

Here are the perf benchmarks, taken just before I added some tests (will redo them with the current git hash shortly). I ran 100 samples, and unfortunately it does look like this is a minor regression:

results.pdf
results.json.txt

[krisselden]

@pzuraq that isn't a significant result, it has too high of a chance of incorrectly rejecting the null hypothesis.

[pzuraq]

Alright, so should I rerun with more samples? Or is this good enough to merge?

Currently on vacation but I’ll get it rebased as soon as I get back!

[rwjblue]

FYI - I just rebased it (@krisselden and I are poking at this stuff, so figured I'd rebase while I was in there...)

[pzuraq]

👍 I can look into the test failures as soon as I’m back next week

[pzuraq]

This has been superceded by a better strategy implemented in #16436