Skip to content

What is the intended behavior of C.toString() where C is a class? #26024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kmillikin opened this issue Mar 17, 2016 · 7 comments
Closed

What is the intended behavior of C.toString() where C is a class? #26024

kmillikin opened this issue Mar 17, 2016 · 7 comments
Labels
area-specification (deprecated) Deprecated: use area-language and a language- label.

Comments

@kmillikin
Copy link

Both the VM and dart2js throw a NoSuchMethodError, but I can't quite see where the spec mandates that.

From 16.14.4 Function Expression Invocation we have:

A function expression invocation i has the form
ef (a1, . . . , an, xn+1 : an+1, . . . , xn+k : an+k),
where ef is an expression. If ef is an identifier id, then id must necessarily denote a local function, a library function, a library or static getter or a variable as described above, or i is not considered a function expression invocation. If ef is a property extraction expression (16.18), then i is is not a function expression invocation and is instead recognized as an ordinary method invocation (16.17.1).

So this is not a function expression invocation and is instead an ordinary method invocation.

From 16.17.1 Ordinary Invocation we have:

Evaluation of an unconditional ordinary method invocation i of the form
o.m(a1, . . . , an, xn+1 : an+1, . . . , xn+k : an+k)
proceeds as follows:
First, the expression o is evaluated to a value vo. Next, the argument list (a1, . . . , an, xn+1 : an+1, . . . , xn+k : an+k) is evaluated yielding actual argument objects o1, . . . , on+k. Let f be the result of looking up (16.15.1) method m in vo with respect to the current library L.

The expression C is evaluated to a value vo. From 16.33 Identifier Reference we have:

If d is a class or type alias T, the value of e is an instance of class Type (or a subclass thereof) reifying T.

So vo is an instance of class Type or a subclass thereof reifying C. From 16.15.1 Method Lookup we have:

The result of a lookup of a method m in object o with respect to library L is the result of a lookup of method m in class C with respect to library L, where C is the class of o.
The result of a lookup of method m in class C with respect to library L is: If C declares a concrete instance method named m that is accessible to L, then that method is the result of the lookup, and we say that the method was looked up in C. Otherwise, if C has a superclass S, then the result of the lookup is the result of looking up m in S with respect to L. Otherwise, we say that the method lookup has failed.

And https://api.dartlang.org/stable/1.15.0/dart-core/Type-class.html seems to indicate that the lookup will find the method toString from a superclass.

All the stuff about parameter matching seems to succeed and we get back to 16.17.1 Ordinary Invocation:

If vo is an instance of Type but o is not a constant type literal, then if m is a method that forwards (9.1) to a static method, method lookup fails. Otherwise method lookup has succeeded.
If the method lookup succeeded, the body of f is executed with respect to the bindings that resulted from the evaluation of the argument list, and with this bound to vo. The value of i is the value returned after f is executed.

In this case vo is an instance of Type but o is a constant type literal, so we're OK and we should invoke the method toString from a superclass.

@kmillikin kmillikin added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). docs-language-spec labels Mar 17, 2016
@lrhn
Copy link
Member

lrhn commented Mar 17, 2016

I think you are right. The sentence that avoids looking up static members on Type objects unless the expression uses the type itself:

If vo is an instance of Type but o is not a constant type literal, then if m is a method that forwards (9.1) to a static method, method lookup fails.

might need to be extended to handle the other direction too:

If vo is an instance of Type and o is a constant literal, then if m is not a method that forwards (9.1) to a static method, method lookup fails.

This would avoid ClassName.toString() calling the toString of the Type object, which the current specification seems to specify, and which nobody actually implement.

@kmillikin
Copy link
Author

The Type objects are too complicated. They are first class objects that have different rules for method lookup depending on their syntactic context. My reading with your suggested amendment is that If there's a literal in a context where it's followed by a dot (.) or hash (#) then the actual methods of the Type object cannot be found by lookup but the synthetic forwarding methods can. If an expression in any other context evaluates to a Type object, then the actual methods will be found by lookup and the synthetic methods will not. That's a strange Dart object!

It suggests to me that in all the cases where the lookup finds the synthetic forwarding methods and not the actual methods, you don't really want the Type object at all.

To me, it's much cleaner to just say that in e.x where e is a type literal, it's a static invocation, the subexpression e is unevaluated, and the static method is looked up in the class e. That matches what an implementation would do because it's simpler in terms of the object model (objects just obey the normal lookup rules), safer (there's no possibility that the synthetic methods leak where they shouldn't because they don't exist), and no more complicated in terms of implementation (the spec requires you to peek at the syntactic form of e in e.x anyway, either to lookup up static method x in the class e, my suggestion, or else to use special lookup rules to lookup x in the Type reifying class e, currently).

The spec reads like it's specifying an implementation of something that's unobservable, and that nobody would implement that way.

I don't think the spec is giving a simpler model for programmers, either.

Suggestion: simplify static lookup so it happens on classes, not reified type objects. Remove the synthetic methods from Type objects.


There is a related problem with instances of Type in generalized closurization. C#m evaluates C to an object o which is an instance of Type, binds a fresh variable u to o and then produces a closure (args...) { return u.m(args...); }. But when the closure is applied lookup of the method m in the type object bound to u finds a method that forwards to a static method, so lookup fails because u is a variable and not a type literal.

@lrhn
Copy link
Member

lrhn commented Mar 18, 2016

I agree that it's (too) complicated.

I guess that the long-term design goal is that Type objects should have the static members of classes as instance members, and it doesn't matter whether you access them through a type literal or any other Type object. That would be (part of) having first-class types (issue #10667) .

The current specification has all the plumbing to allow that, and therefore needs to add extra rules to keep the existing behavior where ClassName.foo and (ClassName).foo behave differently - the former doing static look-up on the class, the latter doing instance look-up on the Type object - by filtering on the methods found by the lookup in both cases. It's missing a case, which is why ClassName.toString() falls trough the cracks and should find the Type object instance's toString.

To enable the prepared behavior, all you need to do is to remove the restriction that makes lookup of a static method fail on a non-class-literal. The rest of the specification will should then still work as written (barring other missing cases).

@kmillikin
Copy link
Author

Hmmm. But an implementor or programmer reading the spec today (4th edition) just sees unnecessary complexity. And we've inadvertently specified that the instance members of Type are static members of every class and that closurization does not work for static members.

It might be better to add this machinery when it's needed rather than preemptively.

@kmillikin
Copy link
Author

We've also inadvertently specified that the static type of a static method invocation is dynamic.

From 16.17.1 Ordinary Invocation:

Let T be the static type of o.

From 16.33 Identifier Reference:

Evaluation of an identifier expression e of the form id proceeds as follows:
Let d be the innermost declaration in the enclosing lexical scope whose name is id or id =.
...
If d is a class, type alias or type parameter the static type of e is Type.

Back to 16.17.1:

If T.m exists, it is a static type warning if the type F of T.m may not be assigned to a function type. If T.m does not exist, or if F is not a function type, the static type of i is dynamic; otherwise the static type of i is the declared return type of F.

Unless we want Type to have members for every static method in the program, T.m will not exist and the type of the invocation will be dynamic.

We've also changed the no such method behavior to invoke noSuchMethod on Type instead of throwing NoSuchMethodError, but I guess that doesn't make as much difference.

@munificent munificent added area-specification (deprecated) Deprecated: use area-language and a language- label. and removed area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). docs-language-spec labels Dec 13, 2016
@eernstg
Copy link
Member

eernstg commented Feb 6, 2018

No milestone now: This is not blocking Dart 2.

@eernstg
Copy link
Member

eernstg commented Sep 17, 2018

Closing: I believe the case targeted by the title of this issue is covered today.

Here is the error for C.toString() and similar constructs (in section 'Ordinary Invocation'):

It is a compile-time error to invoke any of the methods of class \code{Object} on a prefix object (\ref{imports})
or on a constant type literal that is immediately followed by the token `.'\,.

And here is the corresponding error for C.hashCode and similar constructs (in section 'Getter Access and Method Extraction'):

It is a compile-time error if $m$ is a member of class \code{Object} and $e$ is either a prefix object (\ref{imports}) or a constant type literal.

I looked at the description here suggesting that we need to consider a lot of other steps, but I actually think we should just apply the above rules as the first step, and then note that we've found a compile-time error.

@kmillikin, please re-open give me a heads up if you think that line of reasoning does not suffice.

Another matter is that this should be a compile-time error reporting that this construct is simply outlawed as such. Currently (version 2.1.0-dev.4.0) both the vm and dart2js report a different error (using the phrase 'Method not found'), which seems to indicate that the common front end searches for a class method and doesn't find it. I've reported that as a new issue #34492.

@eernstg eernstg closed this as completed Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-specification (deprecated) Deprecated: use area-language and a language- label.
Projects
None yet
Development

No branches or pull requests

5 participants