Skip to content

Incorrect error message #34492

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

Open
eernstg opened this issue Sep 17, 2018 · 6 comments
Open

Incorrect error message #34492

eernstg opened this issue Sep 17, 2018 · 6 comments
Labels
improve-diagnostics Related to the quality of diagnostic messages legacy-area-front-end Legacy: Use area-dart-model instead. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@eernstg
Copy link
Member

eernstg commented Sep 17, 2018

This may well be considered low priority, but checking up on the status of #26024 I noted that the vm and dart2js both report an error containing the words Method not found: 'int.toString' for the following program:

main() {
  int.toString();
}

It is correct to make it an error (see here), but the wording seems to imply that there was a static lookup for a class method (aka static method) named toString in int, and that lookup failed, but the error is actually that it is specified separately that it is a compile-time error to invoke an instance method declared by Object on a type literal:

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 `.'\,.

So the error message ought to be something like Error: Invoking instance method on type literal.

A similar issue arises for getters:

main() {
  int.hashCode;
}

The requirement that a period must follow the type literal is there in order to allow (int).toString() and (int).hashCode, if needed. ;-)

@eernstg eernstg added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) legacy-area-front-end Legacy: Use area-dart-model instead. labels Sep 17, 2018
@kmillikin
Copy link

I agree that the quality of the error message is bad.

Out of curiosity, is there any reason for this behavior? It breaks the equivalence with:

main() {
  var x = int;
  x.toString();
}

for no reason that I can see, and it's not preventing me from doing anything because (int).toString() is a workaround.

@eernstg
Copy link
Member Author

eernstg commented Sep 18, 2018

I believe the original motivation was that it's confusing to have the clash where C.foo() will invoke a class method for all foo (so C is only used as a compile-time namespace navigation device), except that C.toString(), C.hashCode etc. (instance members of Object) will evaluate C to an instance of Type and then call an instance method on it.

I suspect that the rules are sufficient to justify that we won't evaluate C to an instance of Type in the first place, but as you described in great detail it is a non-trivial exercise to conclude that. So it's possible that the separate "this is an error" rule is redundant, but it does at least provide an answer in one step. ;-)

In any case, it's a pure opinion in the spec, not a technical necessity.

[Edit: Just discussed this with @kmillikin IRL and discovered that we should simply delete the sentence in the spec about "immediately followed by ., because it does follow from the normal rules that int.toString() is an error.]

@kmillikin
Copy link

On rereading it, I can't make int.toString() an error without this special case. The static type of int is Type and Type has an accessible toString member so that's OK.

I'm also having trouble understanding how static invocation works at all. Say we've got class C { static void foo() {}} and we invoke C.foo().

First problem: the static type of C is Type and so this is a compile-time error. Type does not have an accessible instance member named foo, the class C does not have a static getter named foo, and Type is not dynamic or Function. (We have to allow C to have a static method, not just getter. Unless we say that it implicitly has a static getter for each static method, but we don't do that.)

Second problem: we look up the method foo in Type which fails, so we look up the getter foo in Type which likewise fails. The spec says that we cannot have failure of both lookups, but we can. So we can't seem to typecheck the invocation with the current spec. (We have to say that if the receiver expression has type Type and it is a type literal followed by a dot, then we typecheck the invocation with respect to the static member of the corresponding class.)

I believe that things could be vastly simplified by saying that C.f(...) is directly a static invocation if C is the name of a class, that C is not even an expression that is evaluated when it occurs to the left of a dot (and it doesn't even have a static type, C.f is typechecked with respect to the static member of C), and then not installing these instance members that forward to the static members on the corresponding instance of Type in the first place (look just a little bit later in this section and see that they have to be hidden with another special case).

@kmillikin
Copy link

I should add: if we could pull off the change I suggest then the spec would match the implementation. We would then not need a special case to make int.toString() an error. It would be a consequence of class int not having a static member toString.

@eernstg
Copy link
Member Author

eernstg commented Sep 18, 2018

I thought it would work, but I walked through it (sections 16.14.5, 16.19, 16.19.3, 16.18.1) and concluded that we will accept int.toString() as an ordinary method invocation unless we have the special rule

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 `.'\,.

just like you said.

I also agree that we should try to introduce the notion of a static invocation whose syntactic form can also be like an ordinary method invocation, and then int.toString() would be an error because there is no class method named toString in int, exactly as reported by the front end today.

@eernstg
Copy link
Member Author

eernstg commented Sep 18, 2018

Created dart-lang/language#3759 in order to improve the specification as indicated.

@askeksa-google askeksa-google added the improve-diagnostics Related to the quality of diagnostic messages label Oct 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve-diagnostics Related to the quality of diagnostic messages legacy-area-front-end Legacy: Use area-dart-model instead. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants