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

Should private methods and accessors type-check their receiver? #1

Closed
littledan opened this issue Jun 29, 2017 · 23 comments
Closed

Should private methods and accessors type-check their receiver? #1

littledan opened this issue Jun 29, 2017 · 23 comments

Comments

@littledan
Copy link
Member

This is the main open question for observable private method semantics: Should private methods act like a lexically scoped function declaration, or like (immutable) fields on the instance/constructor? Or, to give an example, what should the following code do:

class C {
  #sayHi() { alert('hi') }
  constructor() { #sayHi.call(undefined) }
}
new C();

If we treat #sayHi basically like a lexically scoped function, that happens to be in method position, then the alert('hi') should run just fine without any issue. On the other hand, if we treat it as an immutable field, then it would throw a TypeError because the #sayHi field doesn't exist on undefined.

The current spec text does not do the type checking, treating it as a lexically scoped function. If we want to add type checking, I think we'd want to take the following into account:

  • All private methods and accessors are added before any fields are. This way, field initializers can call private methods without having to worry about ordering. Since field initializers can call public methods which come textually later this way (since they're already installed on the prototype), private methods should also be available at that time.
  • If we take the "instance"/type-checking approach, making the methods immutable makes sense because they are shared between all instances--just like public methods. There's an additional implementation benefit, that it should be easier to inline methods, represent things in memory (in the hidden class, rather than taking a whole word in the instance), etc.
@littledan
Copy link
Member Author

cc @bakkot @allenwb @erights

@bakkot
Copy link
Contributor

bakkot commented Jun 29, 2017

I think many of the arguments for type-checking when accessing fields apply equally well when to type-checking methods, except possibly the argument from implementability, so I'm lightly in favor of it unless there's good reason not to.

I think that most of the reason you might want not to - in particular, for separating and sharing implementation logic which doesn't necessarily operate on instances - would be adequately handled by using static private methods operating on their arguments.

@erights
Copy link

erights commented Jun 29, 2017

First terminological nitpick:

By "immutable" I think you mean what I would call "unassignable", or in JS a "const" binding. Specifically, in your example you cannot reassign #sayHi to be bound to something else, whether on a per instance basis or not. Regarding object state, "immutable" falls in the middle of a hierarchy of restrictions on mutability:

  1. Frozen: None of the object's own properties or [[Prototype]] can change.
  2. Transitively Frozen: None of the public properties or [[Prototype]]s accessible on the object by transitive reflective property walk can change.
  3. Immutable: Does not provide the ability to cause or observe state changes. None of the state transitively reachable from the object by hypothetical non-mutating traversal of state can change. (This has been called "DeepFrozen" in some previous systems but we avoid that term in JS because of possible confusion with "Transitively Frozen".)
  4. Pure: Cannot cause or observe state changes.

https://people.eecs.berkeley.edu/~daw/papers/pure-ccs08.pdf
tc39/proposal-shadowrealm#35 (comment)
http://monte.readthedocs.io/en/latest/auditors.html
http://www.erights.org/elang/kernel/auditors/index.html
http://wiki.erights.org/wiki/Guard-based_auditing

@erights
Copy link

erights commented Jun 29, 2017

Second terminological nitpick:

"Type" has many meanings, especially in JS currently. I think it is better to talk about the class branding its instances and the question of whether private method invocation should first dynamically verify the brand.

We could say that a class represents a nominal type and that the brand check is a dynamic nominal type check. This is accurate but requires some explanation. JS classes are generative, so nominal type identity is also generative. I do not know if this falls within most people's understanding of nominal types.

The following seminal papers refer to this as trademarking rather than branding, which I am open to. I have used both terms, and I have occasionally used branding for something different.
www.cs.fsu.edu/~langley/COP4020/2016-Summer/p120-morris.pdf
www.erights.org/history/morris73.pdf

@erights
Copy link

erights commented Jun 29, 2017

Whew! With all that said, I favor private methods being unassignable and brand-verifying on invocation -- before running any user code within the function parameter list or body.

Likewise static private methods should do an identity check on their receiver, the constructor object. No separate brand is needed since only this one object would ever carry the brand, and there is no independent mechanism for observing whether something is branded.

I like how all this can still be rationalized as being within orthogonality because

  • private fields and private static fields always already do a brand check. Fields do the brand check on lookup because that's all there is. Here, we're doing the brand check on hypothetical lookup as well as invocation, but that should be fine.
  • unqualified public methods are already shared and accessible by dot from instances. For unqualified private methods to be effectively shared, they must not be able to differ from one instance to another, and so must be unassignable.

@erights
Copy link

erights commented Jun 29, 2017

Btw, I wonder about including private accessors. It might be the right thing. But it is weird that there would be no way to retrieve the accessor's getter or setter function. It would be like, in your example, allowing foo.#sayHi() but prohibiting foo.#sayHi which would reify the function rather than calling it.

@allenwb
Copy link
Member

allenwb commented Jun 29, 2017

I've previously expressed my opinion at tc39/proposal-class-fields#1 (comment)

I've also discussed elsewhere that, given our current concept of lexically scoped private names, the most straight-forward semantics for "private methods" is essentially that of lexically scoped function declarations with a special invocation form.

Brand checking "private methods" does not add any safety to the language but it does add runtime overhead. It also requires adding some sort of class branding mechanism (something different from private fields) to the low level ES object model. That is a heavy-weight addition that seems to add no value. ES is a dynamically typed language. Private methods should not be a back door for introducing dynamic nominal typing.

Finally, here is another way that brand checking private methods would limit their utility.

Imagine a class:

class Example {
    complexMethod1() {
       //many
       //lines
       //of code
   }
   complexMethod2() {
       //many
       //lines
       //of code
   }
}

Note that this method is fully subclassable and that complexMethod1 and compelxMethod2 fully work on subclass instances. Overtime, a developer may realize that there is significant overlap in the code of the two methods and that some shared procedural decomposition would make the code more maintainable. It is desire to do this in a manner that does not expose the subprocedures to direct pubic use. So it might be rewritten as:

class Example {
    #sub1() {}
    #sub2() {}
    #sub3() {}
    //etc.
    complexMethod1() {
       //some code
       this.sub1();
       this.sub(3)
   }
   complexMethod2() {
       //some other code
       this.sub2();
       this.sub3();
   }
}

With per-class private method branding, this would break for subclasses (or require a much more complex, subclass aware branding scheme).

@allenwb
Copy link
Member

allenwb commented Jun 29, 2017

@erights

private fields and private static fields always already do a brand check.

What they really do is a check for the existence of the referenced field. I wouldn't call that a brand check as it doesn't necessarily imply anything about the accessed object over and beyond the existence of that specific field. This would even be more the case if we went in the direction described in tc39/proposal-private-fields#93 (comment)

@bakkot
Copy link
Contributor

bakkot commented Jun 29, 2017

@allenwb, on your last point about procedural decomposition, that's what I meant to address in my second paragraph: i.e., it seems to me that static private methods operating on their arguments adequately handle the procedural decomposition use case, when operating on objects which are not necessarily instances of the class.

But perhaps instance private methods would be easier to reason about.

@bakkot
Copy link
Contributor

bakkot commented Jun 29, 2017

@allenwb:

Brand checking "private methods" does not add any safety to the language but it does add runtime overhead. It also requires adding some sort of class branding mechanism (something different from private fields) to the low level ES object model.

I would expect a private method to be accessible on a given receiver iff a similarly declared private field were accessible. So I don't think this would actually require adding much to the language.

@littledan
Copy link
Member Author

Sorry, to clarify about how branding would work: I'd expect that each class would give a brand in an additive way: when returning from super(), the reciever has a brand added to it which permits all of the private methods to be called. That implies that, if you have subclassing, then methods defined in superclasses are able to call private methods defined in those superclasses.

@allenwb
Copy link
Member

allenwb commented Jun 29, 2017

@littledan

So a brand check would have to check against a set of available brands?? Doesn't that make the check even more expensive. What is the actual benefit that justifies that cost?

@allenwb
Copy link
Member

allenwb commented Jun 29, 2017

@bakkot

I would expect a private method to be accessible on a given receiver iff a similarly declared private field were accessible. So I don't think this would actually require adding much to the language.

The check for private fields is conceptually similar to an own properties lookup except the key is an internal private field identifier rather than a string or symbol. It sounds like you are saying add the equivalent of a private field corresponding to each private method to each instance objects.

@erights
Copy link

erights commented Jun 29, 2017

So a brand check would have to check against a set of available brands??

Yes. I don't think the following (cool) paper is relevant, but in case it is:
https://pdfs.semanticscholar.org/bbb3/3af2016e48b5784c10484eecf82febec1383.pdf

For private field #hi, code in the class that does foo.#hi, where foo is bound to something irrelevant, throws.

Allen, please remind me. In your proposal, for private method #sayHi, when code inside the class does foo.#sayHi(), where foo is bound to something irrelevant, what happens?

@allenwb
Copy link
Member

allenwb commented Jun 29, 2017

@erights

I what I have suggested (I need to find where it was written down???)

class E {
           #sayHi() {}
}

is pretty much the same as

class E {
           function sayHi() {}
}

except that #sayHi is lexically bound as a private name rather than a normal identifier. Also private name bindings need to distinguish whether a private name is bound to a field or to a function.

The other characteristic is that the syntactic form foo.#sayHi() is an early error if #sayHi does not have a lexically visible definition. foo.#sayHi() is (can be) statically determined to be either a direct lexical method call or a indirect private field determined method call based upon the lexical binding of #sayHi.

So, if #sayHi is in scope as a private method, foo.#sayHi() exactly equivalent to:
<function object that is the bound vaoue of #sayHi>.call(foo) just like if it had been declared as function sayHi() {} and invoked as foo.sayHi().

@erights
Copy link

erights commented Jun 29, 2017

So, if #sayHi is in scope as a private method, foo.#sayHi() exactly equivalent to:
<function object that is the bound vaoue of #sayHi>.call(foo) just like if it had been declared as function sayHi() {} and invoked as foo.sayHi().

If sayHi were merely declared as a function, it could not be invoked as foo.sayHi().

In any case, I object to the non-uniformity. It breaks the illusion that foo.#sayHi() in any sense looks up the function starting with the foo object. Dot always implies lookup starting with foo. Yes, in this case, mere verification is an adequate form of lookup. But if the value of foo is something irrelevant, nowhere else in the language will dot succeed at looking up what is sought.

@bakkot
Copy link
Contributor

bakkot commented Jun 29, 2017

The check for private fields is conceptually similar to an own properties lookup except the key is an internal private field identifier rather than a string or symbol. It sounds like you are saying add the equivalent of a private field corresponding to each private method to each instance objects.

Yes.

In fact, no matter what the actual semantics are, I'm reasonably confident that the common mental model will be roughly

function f(){}
class A {
  #m = f;
}

which would imply a brand check before invocation.

Apart from making #m unassignable and defined before any field initializer runs, which I'm in favor of, I think we'd need a fairly strong reason to make the observable semantics differ from that fairly straightforward mental model.

@allenwb
Copy link
Member

allenwb commented Jun 29, 2017

@erights

If sayHi were merely declared as a function, it could not be invoked as foo.sayHi().

Right, actually I originally wrote sayHi.call(foo) and for some reason changed it to the bogus formulation.

@littledan
Copy link
Member Author

OK, it seems like at this point, we're on the same page about the two options. Sorry for my sloppy wording, and thanks for clearing it up everybody.

@allenwb, aside from implementation cost/overhead, what downsides do you see from the brand checking approach? We'll get the early error on a missing method either way.

Another point: If we don't do checking here, should we also not do receiver checking for static private fields, and just make those lexically scoped declarations as well? Maybe this is an argument in favor of doing the brand checks.

As for user intuition: I think we're talking about an edge case here that won't factor much into user intuition one way or another. I expect normal user intuition to be more like, "It's just like a normal method in a class, except I can't use it outside of the class at all."

However, it is important to make a good call here, as I bet code will be written to take advantage of no checking if that's the side we end up coming down on, and we'd have to be comfortable with that. E.g., if we don't do checking, there's no difference between a static and non-static methods.

@allenwb
Copy link
Member

allenwb commented Jun 29, 2017

In any case, I object to the non-uniformity. It breaks the illusion that foo.#sayHi() in any sense looks up the function starting with the foo object. Dot always implies lookup starting with foo.

Yes, it is a total illusion. Which is why I've argued that lexically scoped function declarations is enough to support all of the reasonable "private method use cases". The major push back was the desire to use dot notation for invocation rather than adding an explicit self parameter or using the call method for cases where the "receiver" needs to be passed in.

Treating foo.#sayHi() as a special form addresses that invocation concern. But .#id() has to be thought of as complete special form, not as a .# followed by a function invocation.

Personally, I favor that irregularity in the meaning of . over the complexity that any sort of checked lookup would require.

Yes, in this case, mere verification is an adequate form of lookup. But if the value of foo is something irrelevant, nowhere else in the language will dot succeed at looking up what is sought.

@bakkot
Copy link
Contributor

bakkot commented Jul 2, 2017

It occurs to me belatedly that there are in fact two times you could reasonably check the type of the receiver - on access, and on invocation. These can be different, in principle -

class A {
  #p() {}
  m(other) {
    const f = this.#p;
    f.call(other);
  }
}

I certainly wouldn't want to require engines to do the check twice, though I imagine it could be elided in the common case.

@littledan
Copy link
Member Author

@bakkot That's right, there are multiple possibilities. Let's discuss this over in #4.

@littledan
Copy link
Member Author

I wrote some spec text which attempts to implement the type checking decision from here. You can see it in https://littledan.github.io/proposal-private-methods/ . Any thoughts?

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

4 participants