Normative: do not call super constructor when ThisBindingStatus is already initialized

[bterlson]

Given:

class A { constructor() { print('a') } };
class B extends A {
  constructor() {
    super();
    super();
  }
}

The current spec says 'a' is printed twice as the second super will
invoke the super constructor before throwing an error due to this
being initialized.

This change throws just before invoking the super constructor if this
has already been initialized. The argument list is evaluated as it
may initialize this by invoking super (i.e. super(super())).

Current implementations agree on the current spec behavior so it's
questionable whether this change is worth doing. However this came up in
real world code (due to a merge mishap) and caused significant confusion
so if we can get away with changing it I think we should.

[bterlson]

I could not find any test262 tests validating that the super constructor is called twice, only that an error is thrown. Only relevant test I found is test\language\expressions\super\call-bind-this-value-twice.js.

[caiolima]

I think it is a fair modification. As @bterlson pointed that it confuses debugging a code, I can imagine cases that aren't common, but can be written, when constructor has some side effects in the app in general. If we throw an error before calling the constructor, it makes debugging less painful.

[littledan]

The semantics seem fairly reasonable to me intuitively. I was surprised when I learned that the super constructor is executed multiple times like this. Too bad there are no test262 tests, cc @leobalter .

In general, the new semantics will require implementations to do a check both before and after (as your patch does), since something that ran as part of the super constructor may have actually filled in the this value. Compilers will be able to eliminate that sometimes, but not all of the time (especially in an interpreter or baseline compiler). So this might make subclass construction (slightly) slower.

Given the potential (admittedly likely small) real slowdown, maybe this would be a normative change to get implementation feedback on before merging.

[ljharb]

@littledan I'm not sure I understand - how can super() be called the first time and not have filled in the this value, such that you don't have to do any check until the next super call?

[rwaldron]

@littledan I wrote all of the original class tests for super()... I guess I didn't consider multiple super() calls 🤷‍♀️

[bakkot]

@ljharb two calls can be occurring simultaneously.

[shvaikalesh]

This PR reminded me of what custom elements constructors do (see step 9 of this algorithm). Definitely 👍 for handling this on ECMAScript spec level for consistency.

[ljharb]

@bakkot gotcha, thanks

[bterlson]

Discussed at TC39. No consensus for now, but if engines can implement this fix without performance degradation, it should achieve consensus. /cc @ajklein @msaboff

[ajklein]

My initial investigation into this suggested it's not at all straightforward in V8 to eliminate the extra check. Curious for @msaboff's thoughts.

[msaboff]

My initial look at JSC code leads me to the same conclusion as @ajklein. We need to emit the second check in our bytecode. It would take a little work to eliminate the second check.

[JemiloII]

I'm just curious, why would someone want to call super twice, or more? What benefit does it bring?

[claudepache]

I'm just curious, why would someone want to call super twice, or more? What benefit does it bring?

Nobody would do that on purpose. This PR is about improving debugging experience.

[littledan]

Thanks for looking into the implementation issues for this proposal, @ajklein and @msaboff . Now that we know that it won't be completely trivial to optimize away in all cases, how should we proceed? Would anyone be interested in making a draft implementation to understand the overhead in practice? Or, are the implementation concerns sufficient to deter us from this proposal?

[ljharb]

ping @ajklein @msaboff

[ajklein]

My opinion is that changing this isn't worth any runtime cost at all. I'd be curious for @bterlson's opinion on that question as well.