Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

'this' in static initializers #50

Closed
bakkot opened this issue Aug 20, 2016 · 18 comments
Closed

'this' in static initializers #50

bakkot opened this issue Aug 20, 2016 · 18 comments

Comments

@bakkot
Copy link
Contributor

bakkot commented Aug 20, 2016

If we stick with the model last settled on of initializer bodies being scoped as methods, and do the same for static fields, then classes would be accessible while they were being constructed via this. We have a TDZ for class names specifically to avoid this (it previously only arose for computed method names); I believe this should likewise have a TDZ, being bound at the same time as the class name.

Thus the following code would be an error:

class C {
  static a = this;
}

and this would be legal:

class C {
  static a = () => this;
}
C.a(); /// === C

Thanks to @ajklein for pointing this out.

(NB: The spec text as it currently stands has no special scoping applied to static initializers, so this refers to whatever it would outside of the class, but I believe that doesn't reflect the last consensus of the committee.)

@ljharb
Copy link
Member

ljharb commented Aug 20, 2016

This might be problematic, for one of the big use cases is static singleton = new this();

@bakkot
Copy link
Contributor Author

bakkot commented Aug 20, 2016

@ljharb, that involves instantiating the class before it is finished being defined, and looks to me like an excellent reason to prohibit access to this.

What would it even mean?

If I have

class C {
  static singleton = new this;
  static foo = sideEffect(0);
  [(sideEffect(1), 'a')] = sideEffect(2);
  constructor(){ ... }
}

is sideEffect called with 1 before 0 (ugh)? If so, does that mean that it is called with 2 before 0 (😨)? If not, does C.singleton lack a property named a (also ugh)? Is it instantiated using the default constructor, or the one defined later? This is extremely worrying.

Also, I'm not sure what the use case is for it. This isn't Java; I'd think you could just use an object literal. Or static singleton = () => this._singleton ? this._singleton = new this : this._singleton, or follow the class with C.singleton = new C, or whatever.

@jeffmo
Copy link
Member

jeffmo commented Aug 20, 2016

classes would be accessible while they were being constructed via this.

No, this is specifically enabled in steps 29-32 here: https://tc39.github.io/proposal-class-public-fields/#runtime-semantics-class-definition-evaluation

The class is guaranteed to have finished construction and binding before static field initializers execute (which happens in static-field order).

@jeffmo
Copy link
Member

jeffmo commented Aug 20, 2016

(Note that the class's variable binding is initialized in 23.a)

@jeffmo
Copy link
Member

jeffmo commented Aug 20, 2016

is sideEffect called with 1 before 0

Assume sideEffect is just console.log. You would see:

> 1
> 2
> 0

Is it instantiated using the default constructor, or the one defined later?

It is instantiated with the one defined by the user

Also, I'm not sure what the use case is for it

At least one compelling use case (as with instance fields) is to reference other statics on the class without having to state the name of the class (refactoring, etc).

class UnitCircle {
  static pi = 3.14;
  static circumference = 2 * this.pi;
}

@jeffmo
Copy link
Member

jeffmo commented Aug 21, 2016

Going to close this out, but as always: Please re-open in the case that I've missed anything

@jeffmo jeffmo closed this as completed Aug 21, 2016
@littledan
Copy link
Member

One thing that I didn't see discussed here: if you let this point to the class while initializers are evaluated, you get to see a half-formed class. For example:

class C {
  static has = 'has' in this;
}

Then, C.has will be false. Do we need to expose this additional intermediate state of class definition? TDZ semantics would make it unobservable.

I'm also wondering how the current semantics mesh with the @bterlson and @wycats 's proposal for unified class evaluation; I'd be interested in their thoughts (or has this already been discussed somewhere?).

@bterlson
Copy link
Member

FWIW, TS does not allow this in static initializers, however I can definitely see it getting used. It's also somewhat confusing to allow this in instance property initializers and not in static initializers. I wouldn't oppose disallowing it in static initializers if there's a strong reason to do so - @littledan can you advance such a reason?

The proposal here gels exactly with the BT/YK evaluation order - static initializers run after the lexical binding of the class is removed from TDZ (as in, it's effectively the same as if the static members were added after the class declaration).

@littledan
Copy link
Member

@bterlson Good to hear that it fits with your model. My only objection was that adding initializers afterwards would make the intermediate state of the class observable, showing that the static properties are added one by one. But, considering that this is already the case with instance properties, it's not so bad. As for this, if C is already not in TDZ anymore, it seems like the initializer would be more readable if C were just referred to directly. I'm not objecting to this very strongly, but it's hard for me to think of an important use case. Anyway, I'm fine with the spec as it stands if everyone else feels that way.

@bterlson
Copy link
Member

@littledan I think this gets used as it's a way to refer to my current class always on the static side. This is more important for methods where this might be a subclass. I agree with you that it's not super important, but I can't think of a good reason to disallow it really. TS disallows it in initializers because it's potentially confusing - maybe that's enough? But maybe disallowing it would be confusing too.

@littledan
Copy link
Member

littledan commented Aug 23, 2016

No, I can't think of a good reason to disallow it, and it makes sense since this is there for static methods. Seems fine to me. I just hope no one imagines that static properties are "class instance" fields as @erights has pointed out is a legitimate point in the design space.

@caitp
Copy link

caitp commented Jun 6, 2019

I am not a fan of this this refers to the class stuff. I would rather you simply use the class name from the lexical scope, or a new metaproperty (class.self or something).

@littledan
Copy link
Member

@caitp Why should the value of this in a static field initializer be different from the value in static methods? Do you think it should be different in both cases?

@caitp
Copy link

caitp commented Jun 10, 2019

@littledan syntactically, it’s not a method/function, and even implementation-wise there isn’t a strong argument for it to be.

It’s more in line with object literal elements:

(function() {
  let a = ({
    a: this.a
  }).a; // “a”

  let b = (class {
    static b = this.b;
  }).b; // “b”
}).call({ a: “a”, b: “b” });

Since there is no clear function boundary, it is bizarre for this to change meaning, it’s more complicated impl-wise, and would be better to have something like Rust/Swift’s Self

@littledan
Copy link
Member

I wonder if something related to https://github.com/tc39/proposal-class-access-expressions would help.

Unless it's really really hard to implement (it does seem harder than I imagined), I would rather stick with the current scoping semantics, given the lengthy debate that led us here, multiple implementations, and large test suite that we've already developed.

@caitp
Copy link

caitp commented Jun 10, 2019

It looks straight forward to me, I would 100% prefer that to messing with this.

There is one thing that could make it slightly more difficult (only slightly), which is whether class expressions refer to the current class or outer class in computed property name expressions (which doesn't seem to be mentioned in the README).

Given that there are workarounds which don't require the proposal, the proposal which would only improve things (and be more in line with modern languages), and the fact that altering this is super weird, this court rules that there is no good reason to mess with this for static field initializers. [gavel emoji]

@littledan
Copy link
Member

Well, class access expressions is more proposed as an additional thing on top, rather than a replacement for this semantics.

I still think it's a bit late to change semantics here; it's best to change them during Stage 2 or earlier.

@caitp
Copy link

caitp commented Jun 10, 2019

I disagree, I think we should revisit it. There are few VM implementations, and the current semantics are not good for anybody.

The “extra fluff on top” proposal isn’t needed for the initial implementation, since named classes can already reference themselves easily. It’s just a bonus convenience to allow it in anonymous class contexts.

You don’t mess with this,

static [this.prop + i++] = this.value;

should refer to the same this on the left and right, because people can only take so much craziness.


Addendum:

With the current design, including instance fields, you have 3 different meanings of this in a class definition, which is entirely bonkers.

You could make the argument that [this.prop + i++] = this.value; is perfectly fine for instance fields, and static fields should follow the same rules --- But of course nobody expects the instance fields initializer to operate immediately, it of course applies when the class is constructed, so the change to this is understood. It still looks horrible next to a computed field, but it's understood.

For static fields, the initializer can be assumed to be evaluated inline in the current execution context, intuitively that's how it would be evaluated, so altering this is bananas [gwen-stefani.gif]. Fixing this reduces the number of meanings of this from 3 to 2, which is objectively a good thing and nobody should need to be extra-convinced of this argument. You don't want your PL to be bananas.

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

No branches or pull requests

6 participants