dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.21k stars 1.57k forks source link

Specification: Identifier reference clarification #34456

Closed eernstg closed 4 years ago

eernstg commented 6 years ago

In dartLangSpec.tex, section 'Identifier Reference', the last bullet concerning evaluation specifies that

Otherwise, evaluation of e is equivalent to evaluation of the property extraction (16.19) this.id.

which means that a plain identifier will refer to an instance member when that identifier is (1) undefined, (2) defined in a superclass (that is, statically known, but not in the lexical scope), and (3) defined in the enclosing class as an instance member (hence also in an enclosing lexical scope).

The case (1) should be singled out as a compile-time error during static analysis, and for the dynamic semantics we should say that it cannot occur.

The case (3) could be handled separately, in a bullet that comes just before the static/top-level bullet, which means that we would process the scopes in the order they are encountered.

The case (2) needs to be handled explicitly, because there is no scoping mechanism which allows us to access inherited declarations. This means that the static analysis should say that we must look up the given identifier in the interface of the class, and find an instance member of that name, and then we'll treat it as this.id. If we don't find those things it is a compile-time error. And the dynamic semantics should also be optimized to only say what is necessary, given that the situation is as guaranteed after static analysis has succeeded.

With these changes, we'll avoid having the current situation where that catch-all 'Otherwise, .. this.id' bullet at the end is so ambiguous.

Next, the description of the static analysis should be completed and adjusted to list the bullets in the same order as the dynamic semantics (e.g., the const case is missing).

lrhn commented 6 years ago

The way it is currently written, it "works" because all three are conflated into "equivalent to this.id in every way", and the static type system rejects this.id if the interface of the surrounding class does not declare a compatible id member (be it not declaring one at all, only declaring a setter, or declaring one with an incompatible type).

We may want to give different error messages for the three cases:

but I'm not actually sure we need to treat them differently at the specification level.

eernstg commented 6 years ago

Well, not really: We have It is a compile-time error if the static type of e may not be assigned to the static type of v in a situation where we have looked up a declaration d whose name may be v or v=, and in the situation where v = e will invoke an inherited setter (implicitly induced or explicitly declared) the search through enclosing scopes fails, and hence there is no "static type of v" that makes sense.

Apart from that, we should have checked against the type of the formal parameter of the setter d when the search found v= (and also when both v and v= are found in that same innermost scope), and when we're invoking an inherited setter it could have an argument type which is different from the return type of the getter, if any.

Even though incompatible types for the getter and setter are now a warning and not an error, nobody suggested that we should have a warning or error for different types as long as they are assignable. So we can run code where that difference matters, even without getting any warnings. We may also reject valid code:

int get v => 42;
set v(Object o) {}

Object get w => null;
set w(int i) {}

main() {
  v = true; // Spurious error: "`bool` is not assignable to `int`".
  w = true; // Missing error: "static type of `w` is Object".
}

The reasonable static analysis would be to detect that there is a setter and it will accept a boolean argument (for v=) or the converse (for w=), rather than blindly using "the static type of v/w".

So I think we need to specify the static analysis such that it will find the declarations when they are available (including inherited ones), and it will be an error to violate the usual constraints, and the dynamic semantics will be based on the situations that may actually occur (which will not include this.foo = e in a situation where there is no foo= in the interface of the class).

eernstg commented 4 years ago

Closing: This topic was completely rewritten in commit 8fa22ea447303d5a544b1762c4bb5366b8634db9, and the underspecified cases have been covered.