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

Early error for same name for static and instance field? (question about lexical scoping) #256

Closed
mheiber opened this issue Jul 13, 2019 · 40 comments

Comments

@mheiber
Copy link

mheiber commented Jul 13, 2019

What is the specced behavior in this case?

class C {
    static #foo;
    #foo
}

Is this an [*early* error as covered here](https://tc39.es/ecma262/#sec-block-static-semantics-early-errors)?
@mheiber mheiber changed the title Questions about lexical scopes: early error for same name for static and instance field? Early error for same name for static and instance field? (question about lexical scoping) Jul 13, 2019
@ljharb
Copy link
Member

ljharb commented Jul 13, 2019

It is a Syntax Error if PrivateBoundIdentifiers of ClassBody contains any duplicate entries.

Does that and https://tc39.es/proposal-class-fields/#sec-private-bound-identifiers cover it?

@mheiber
Copy link
Author

mheiber commented Jul 13, 2019

thanks!

@mheiber mheiber closed this as completed Jul 13, 2019
@rdking
Copy link

rdking commented Jul 14, 2019

Just out of curiosity, why is this an error when the two fields would exist in separate namespaces?

@ljharb
Copy link
Member

ljharb commented Jul 14, 2019

The namespaces you refer to (static vs instance) differentiate objects, but don’t disambiguate the local syntax:

class X {
  static #a;
  #a;
  a(y) {
    return y.#a;
  }
}
var x = new X();
x.a(x); // does this throw, or return undefined?
x.a(X); // what about this one?

Since each syntactic usage of .#a is supposed to unambiguously refer to a single private field, and the notation doesn’t contain a difference for static vs instance, that means that all private fields, static or instance, in fact share a namespace.

@rdking
Copy link

rdking commented Jul 14, 2019

I get it. TC39 decided not to allow the same [[PrivateName]] key to exist on both an instance and the constructor, despite the fact that it would not be semantically different from:

class X {
  static _a;
  _a;
  a(y) {
    return y._a;
  }
}
var x = new X();
x.a(x); // does this throw, or return undefined?
x.a(X); // what about this one?

Private or not, they should both return without throwing since the field would exist in both places. There would be no confusion or complication over this issue since it would simply be the same [[PrivateName]] in the lookup for both the constructor and instances. No different than having the same property name on both an instance and the constructor. Is there any particular reason for not allowing this? That's the part I don't get.

@WebReflection
Copy link

I think @rdking has a point: the current limitation is an implementation detail that doesn't reason with developers expectations. If specs were saying that y.#a would pick the right private in case y is [class] instead of instance there wouldn't be any ambiguity.

class X {
  static #a;
  #a;
  a(y) {
    return y.#a;
  }
}

pseudo transpilation that works

const a = {class: new WeakMap, instance: new WeakMap};
class X {
  constructor() { a.instance.set(this, void 0) }
  a(y) {
    return a[isClass(y) ? 'class' : 'instance'].get(y);
  }
}
a.class.set(X, void 0);

@nicolo-ribaudo
Copy link
Member

If an object has two different private fields with the same name, which do you think should have the precedence?

class Base {
  constructor() {
    return Derived;
  }
}

class Derived extends Base {
  #x = 1;
  static #x = 2;

  static getX(obj) {
    return obj.#x;
  }
}

const result = new Derived();

// result has both a "static #x" and an "instance #x"
console.log(Derived.getX(result)); // 1 or 2? 

@mheiber
Copy link
Author

mheiber commented Jul 14, 2019

@nicolo-ribaudo , re your comment: In JS, class properties are not accessbile from instances of the class via this.foo.
The "class" object is really the constructor function, so this fits with the semantics of desugared classes.

function Konstructor () {}
Konstructor.foo = 3 // static propertiy
new Konstructor().foo // undefined

You can see this behavior if you swap #x for x in your example (try it in Chrome Canary).

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2019

@mheiber You've missed that his code sample using the return override trick to make Derived an instance of itself, I think. (Look at the implementation of Base.)

@mheiber
Copy link
Author

mheiber commented Jul 14, 2019

@bakkot thanks for pointing that out. I'm not following the relevance. When I try with public fields I get 1 (the value of the instance property), as expected. Is there something special about private fields in the example? To be honest, I'm not following how the override trick is related either.

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jul 14, 2019

The return override trick makes it such that result === Derived.

Desugared, it is something like this:

function Konstructor () { return Konstructor }
Konstructor.foo = 3 // static propertiy
new Konstructor().foo // 3

If in my example you expect it to log 1, then try running the following code (which is exactly the same, but with the instance property commented):

class Base {
  constructor() {
    return Derived;
  }
}

class Derived extends Base {
  //#x = 1;
  static #x = 2;

  static getX(obj) {
    return obj.#x;
  }
}

const result = new Derived();

console.log(Derived.getX(result));

@mheiber
Copy link
Author

mheiber commented Jul 14, 2019

Asking again (sorry I'm not 100% following)

What is the relationship to private fields? What specifically does this have to do with privacy?

Here's my stab at an answer:

  1. Derived === result. So we're effectively setting #x twice in the first example.
  • This is not actually a problem for public fields (which can be reassigned) or even a problem for private fields, which can also be reassigned.
  • This is a problem for private methods, which (iirc) cannot be reassigned.

Thanks for your time in catching me up!

@WebReflection
Copy link

WebReflection commented Jul 14, 2019

@nicolo-ribaudo not sure that question was for me, but my transpiled code would work the exact same.

class Base {
  constructor() {
    return Derived;
  }
}

const x = {class: new WeakMap, instance: new WeakMap};
class Derived extends Base {
  constructor(...args) {
    const self = super(...args);
    const classy = isClass(self);
    const _x = classy ? x.class : x.instance;
    _x.set(self, classy ? 2 : 1);
    return self;
  }
  static getX(obj) {
    const classy = isClass(obj);
    const _x = classy ? x.class : x.instance;
    return _x.get(obj);
  }
}

I mean ... I've picked a specific example but the concept of distinguishing between [class] and regular instance remains and works regardless, doesn't it? 🤔

@WebReflection
Copy link

P.S. the isClass could be something like:

const isClass = ((gOPD, fb) => Class =>
  typeof Class === 'function' && !(gOPD(Class, 'prototype') || fb).writable
)(Object.getOwnPropertyDescriptor, {writable: true});

assuming Babel makes prototype not writable once transpiled

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jul 14, 2019

Oh ok, I had assumed that you would have always fields in the constructor to the x.instance weakmap. In your example, it would be the only case in the language where a value which is set earlier has the precedence over a value which is set later. The only other exception is non-writable properties, but they throw in strict mode.

I have tried to rewrite your desugaring to match the current proposal, only removing the restriction that an instance and public field can't share the same name, and I think that it could work. (I don't see why it would be needed, but it doesn't matter)

const x = new WeakMap();

class Derived extends Base {
  constructor(...args) {
    super(...args);
    if (x.has(this)) throw new Error("#x has already been declared"); // (1)
    x.set(this, 1);
  }
  static getX(obj) {
    return x.get(obj);
  }
}
x.set(Derived, 2);

(1) is because private class fields already throw when re-declared:

let result = {};
class Base { constructor() { return result } }

class Derived extends Base { #i = 0 }

// Add #i to result
new Derived();

// Add #i to result again
new Derived(); // throws

@WebReflection
Copy link

@nicolo-ribaudo yup, that's another way to go (probably even better, since a whole WeakMap for a single value was indeed too much).

As we can see, there's no reason to not allow same name between static and instance fields ... we might want to throw in methods if the x.has(obj) returns false, otherwise everything else is really an implementation detail, that shouldn't be spec'd as language limitation (IMHO)

@bakkot
Copy link
Contributor

bakkot commented Jul 14, 2019

Incidentally, there's previous discussion of this in tc39/proposal-static-class-features#47.

@WebReflection
Copy link

WebReflection commented Jul 14, 2019

I didn't see any important use cases, and allowing name sharing would increase complexity a lot.

I tend to agree same name is not a great pattern, but then again we have clear examples in the language itself, where String.length and "string".length both exists and have their own meanings.

If it's accepted on the public field level, consistency would be my use case for this on the private fields too.

Since it doesn't look like complexity is really increased neither, at least from transpilation point of view, I wonder what is so hard in the native side of the affair, that we arbitrarily chose to make the private fields inconsistent with the public one.

From a developer point of view, this will be just yet another laughable JS shenanigan 🤷‍♂️

Maybe @littledan can share more on this

@ljharb
Copy link
Member

ljharb commented Jul 14, 2019

I’ll try to restate, since my reading of the above thread does not leave any doubt for me as to why ohh a static and instance field of the same name can’t be allowed.

Using the return override trick, if both a static and instance field of the same name were allowed, you can make an object that has both the static and the instance field.

How would you extract both fields’ values from that object? (with a public field name, like length, there aren’t two fields - there’s only one - but each private field is a distinct value - it’s not like a string name)

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jul 14, 2019

Using the return override trick, if both a static and instance field of the same name were allowed, you can make an object that has both the static and the instance field.

This is the problem I initially found, but then I realized that the proposal already handles this case. Due to the return-override trick, it is already possible to try to define the same private name twice, and it currently throws (you can test the second snippet in #256 (comment) in an engine with private fields support).
If it was possible to use the same name for a static and an instance field, it would behave the same: my initial example (#256 (comment)) would throw.
This is under the assumption that staitic #x; #x wouldn't create two different private names, but reuse the same one.

EDIT: Throw = throw at runtime

@WebReflection
Copy link

I don't think you can have that situation at all. Mind showing some code example that would create such scenario?

@ljharb
Copy link
Member

ljharb commented Jul 14, 2019

this case is why you can’t have a different field with the same name across static vs instance:

class X {
  static #a = 1;
  #a = 2;
  constructor() { return X; }
  static a(y) { return y.#a; }
}
const x = new X();
X.a(x); // 1, 2, throw?

@nicolo-ribaudo
Copy link
Member

That example doesn't actually install the instance field on X; you have to use super() for that to happen.
Anyway, I would expect it to throw at new X(), like it already does.

@ljharb
Copy link
Member

ljharb commented Jul 14, 2019

lol I’m on mobile rn so i won’t try to make a fixed example with return override; if someone else can in the meantime please do.

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jul 14, 2019

It's this one: #256 (comment)

class Base {
  constructor() {
    return Derived;
  }
}

class Derived extends Base {
  #x = 1;
  static #x = 2;

  static getX(obj) {
    return obj.#x;
  }
}

const result = new Derived();

// result has both a "static #x" and an "instance #x"
console.log(Derived.getX(result)); // 1 or 2? 

If it was allowed to re-use the same private name, I would expect this to throw at new Derived() because it tries to define a private field which is already defined. (Step 4 of https://tc39.es/proposal-class-fields/#sec-privatefieldadd)

@nicolo-ribaudo
Copy link
Member

Btw, my position is:
I don't think that there should be technical reasons for this to be disallowed, and allowing it would make it more consistent with public fields without modifying the current privacy level of private fields.
On the other hand, I can't think of a single good reason for this to be allowed and a class which uses the same name for both a static and an instant field would just cause confusion for who reads the code.
If it was allowed, I would need a linting rule to disallow this bad practice. With private fields we have the possibility of disallowing some bad practices at language level, but I don't know it this is "universally bad" and thus should be disallowed by the language.

@ljharb
Copy link
Member

ljharb commented Jul 14, 2019

There’d be no way to get both fields’ values out, as in your example.

@WebReflection
Copy link

Same with public fields, right?

@robpalme
Copy link
Contributor

robpalme commented Jul 14, 2019 via email

@ljharb
Copy link
Member

ljharb commented Jul 14, 2019

@WebReflection no, because in public fields, every “a” is the same a. Private fields are different, more like symbols where each “a” is a distinct one.

@WebReflection
Copy link

WebReflection commented Jul 14, 2019

I'm not sure I am following

class X {
  static _a = 1;
  _a = 2;
  a(y) {
    return y._a;
  }
}
var x = new X();
x.a(x); // 2
x.a(X); // 1

that's what I read on Chromium ... and I believe static fields are always the same field for the class itself defining them. Once again, I don't have a valid use case now but consistency is what's driving the software world (eslint, standard/semi-standard/prettier) and keep having inconsistencies baked in the language seems wrong.

I'd personally rather enable same field name and flag the circular edge case we have here as not-allowed than rule same name private field out everywhere for a single, irrelevant, edge case.

@WebReflection
Copy link

WebReflection commented Jul 14, 2019

Also (maybe) relevant: TypeScript allows this, and as much as I don't care about TS, I can see already people moaning about TS being superior with (fake) private fields too.

Screenshot from 2019-07-14 18-07-52

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jul 14, 2019

If the same name were permitted on both the class and the instance,
dereferencing that field on an incoming parameter would become ambiguous,
i.e. did the user intend to dereference the static field or the instance
field?

Note that is is already possible to copy the private names from instance fields to static fields:

class Base {
  static #class = null;
  constructor() {
    const ret = Base.#class;
    Base.#class = null;
    return ret;
  }

  static defineStaticFields(Class) {
    Base.#class = Class;
    new Class();
  }
}

class Derived extends Base {
  #x;

  set x(v) { this.#x = v }
  get x() { return this.#x }

  static set x(v) { Derived.#x = v }
  static get x() { return Derived.#x }
}
Base.defineStaticFields(Derived);

Derived.x = 1; // sets static #x
const inst = new Derived();
inst.x = 2; // sets instance #x

console.log(Derived.x, inst.x); // logs "1 2"

For this reason, allowing people to explicitly choose which private names they want to re-use in both positions doesn't weaken any guarantee.

no, because in public fields, every “a” is the same a. Private fields are different, more like symbols where each “a” is a distinct one.

This discussion is based on the possibility that, inside the same class, #a and static #a wouldn't create two different private names but reuse the same one.

@rdking
Copy link

rdking commented Jul 14, 2019

@ljharb If in a class declaration, every #a is the same #a, then the consistency holds between private names and public names. Installing the same name to 2 different objects is no different whether the object is an instance or the class constructor. As was stated before, the return override trick in such a scenario should just cause an error.

IMHO, it's not for a language to restrict the use of ill-advised code patterns. Leave that to tooling and education. Sometimes, those ill-advised patterns are actually the best and most expedient solution to a problem (even though that's rarely the case). The job of the language is to provide the developer with the ability to be expressive and thorough. I don't see the point of this restriction.

@ljharb
Copy link
Member

ljharb commented Jul 14, 2019

It’s imo very much for a language to limit ill-advised patterns - passing burden onto developers and the linting community when it’s avoidable is a bad thing.

Note that the way this is specced, it’s an error, so if there actually turn out to be any good or \ solutions that require the duplicates, the restriction could be relaxed.

@WebReflection
Copy link

passing burden onto developers and the linting community when it’s avoidable is a bad thing.

but no consistency is worse DX

Note that the way this is specced, it’s an error, so if there actually turn out to be any good or \ solutions that require the duplicates, the restriction could be relaxed.

but if it throws no new pattern/ideal/solution can possibly come out 'cause nobody would throw errors, they need their code to execute 🤔

@WebReflection
Copy link

also about that:

It’s imo very much for a language to limit ill-advised patterns

so same name for public fields should throw too, as there's no valid reason for that ... for developers there's no difference, and from the ergonomic point of view, there is no difference indeed.

@ljharb
Copy link
Member

ljharb commented Jul 14, 2019

but no consistency is worse DX

I'm still not seeing the inconsistency you're referring to.

There's a very valid reason for that - every occurrence of a public property name that's the same string is the same property name, so static length = 3; length = 2 doesn't have the same conflict that private fields do.

@rdking
Copy link

rdking commented Jul 14, 2019

It’s imo very much for a language to limit ill-advised patterns

I understand why you might think this. It seems to be a common theme in modern programming. However, that kind of thinking is a trap that leads to dead ends for the language. The reasoning is simply that what is thought of as "bad" today may become tomorrow's new great pattern. It is thus better that a language simplifies the use of "good patterns" without unnecessarily restricting "bad patterns". Doing so limits future innovation in the language.

Note that the way this is specced, it’s an error...

Does this mean that the spec regarding this will be corrected?

@ljharb
Copy link
Member

ljharb commented Jul 15, 2019

no, because it’s not incorrect. It means it could be amended later to do something instead of being an error.

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

7 participants