Make type promotion work on final fields

[alanrussian]

Type promotion only seems to work on local variables. It would be nice if it also worked on final fields. I understand that it's not supported for non-final fields, because it's harder to assure that the instance isn't changed; however, the instance can't be changed with final fields.

Let me know if this is the wrong place to report these types of feature requests.

[leafpetersen]

however, the instance can't be changed with final fields.

True, but the value returned from a final field can change in Dart. :(

class A {
  final num x = 3;
}

class B extends A {
  num y = 3;
  num get x => y;
}
void main() {
  B b = new B();
  A a = b;
  if (a.x is int) {
    b.y = 3.0;
    (a.x as dynamic).isEven();  // Fall down go boom.
  }
}

If we get sealed classes (or non-virtual fields) we could do this.

[matanlurey]

@munificent for FYI (re: #81).

@alanrussian:

however, the instance can't be changed with final fields.

That is not true. For example, see the following example:

class A {
  final num amount = 5;
}

class B implements A {
  var _returnInt = false;

  @override
  num get amount {
     // Evil, but this is a simplistic example.
    _returnInt = !_returnInt;
    return _returnInt ? 5 : 5.5;
  }
}

void main() {
  A a = B();
  if (a.amount is int) {
    print(a.amount.runtimeType); // double
  }
}

EDIT: Oops, @leafpetersen beat me :(.

@leafpetersen: Is there language issue/request for making fields non-virtual (optionally or otherwise)?

[alanrussian]

Ahh makes sense. Thank you.

[rakudrama]

However, we can do much better (at making reasonable code just work) if we are willing to make errors be a function of the whole library.

• you can find all overrides of a private field since they must be in the same library.
• you can detect if a class is de-facto sealed by having private generative constructors which prevent it being extended or mixed-in in another library.

The rules become more complex and harder to explain, but I am of the opinion that type promotion should work in as many places as possible, and the complex rules can be mitigated by errors explaining the failed promotion.

class C {
  final Object x;
  foo() {
    if (x is int) {
      print(x.isEven);
    }

test.dart:5:15: Error: The getter 'isEven' isn't defined for the class 'dart.core::Object'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 'isEven'.
      print(x.isEven);
              ^^^^^^

Rather than suggesting adding a method to Object, explain the full reasoning

test.dart:5:15: Error: The getter 'isEven' isn't defined for the class 'Object'.
      print(x.isEven);
              ^^^^^^
test.dart:5:15: 'this.x' has promoted type 'Object' here.
test.dart:4:11: This test does not change the promoted type of 'this.x' because a subclass could override 'get x' to return a different value on every call.

[leafpetersen]

• you can find all overrides of a private field since they must be in the same library.
• you can detect if a class is de-facto sealed by having private generative constructors which prevent it being extended or mixed-in in another library.

Neither of these is true.

The first is true if you say "same strongly connected component in the library graph", but is not true at the library granularity because of mixins (extend a class outside of the library, then mix it the subclass within the library). Library privacy FTW.

The second is not true because you can still implement the interface with the final field using a getter which returns a different result depending on the time of day.

Agreed that the error message should be better. Kotlin gives a very nice error message for these.

but I am of the opinion that type promotion should work in as many places as possible

Also agreed.

[jmesserly]

The first is true if you say "same strongly connected component in the library graph", but is not true at the library granularity because of mixins (extend a class outside of the library, then mix it the subclass within the library). Library privacy FTW.

I thought we fixed that--didn't we disallow mixins to cause a private field override?

//test.dart
class M {
  int _foo = 42;
  int get foo => _foo;
}
class N {
  int _foo = 123;
}

import 'test.dart';
class C extends M with N {}
main() {
  print(C().foo);
}

prints the error:

[error] The private name _foo, defined by N, conflicts with the same name defined by M. (test2.dart, line 2, col 24)

interestingly, it's not an error if C is declared in the same library as M and N (and dartdevc does not give the right answer for that case - filed as dart-lang/sdk#35231).

[leafpetersen]

I thought we fixed that--didn't we disallow mixins to cause a private field override?

I'm not sure if this was the intention or not, but de facto we only fixed it for a library cycle, not for a library. I think (hope) this is enough for our purposes, since that's our unit of compilation? The program below has no analyzer errors.

In test2.dart:

library B;
import 'test.dart' as t1;

class A {
  final num _x = 3;
}

class B {
  final num _x = 4;
}

class C extends t1.D with B {}  // Causes an override of _x with no error

In test.dart:

library A;
import 'test2.dart' as t2;
//class D extends t2.A with t2.B {}  This is an error
class D extends t2.A {}
void main() {}

[jmesserly]

that's probably bugged in DDC too (for the same reason as dart-lang/sdk#35231). I assumed we'd chosen a simple rule like "can't collide private names via mixins" rather than the error depending on things like "build unit". I'll add a note to the bug. (I suppose it's sufficiently obscure that no one has tried to write web code that relies it, thankfully.)

[leafpetersen]

We may have intended it to be library rather than build unit, and it may just not have been implemented correctly. It's technically a breaking change to change it now, but it's probably obscure enough that we could do it if it matters at all.

[munificent]

The rules become more complex and harder to explain, but I am of the opinion that type promotion should work in as many places as possible, and the complex rules can be mitigated by errors explaining the failed promotion.

I agree with your general goal of making type promotion work as much as possible. But we also need to make sure that the rules that determine where it does and doesn't work are user-comprehensible. If a user changes their library in a way that affects type promotion, that is a breaking change to users of that library, and the library author needs to know that.

For example, with the rules you suggested, adding a public generative constructor to a class which has only private ones would turn off promotion on fields of that type. That in turn would cause method calls on those fields in user code to become static errors since the field is no longer promoted to the tested type.

[leafpetersen]

I filed an issue for discussion of a runtime checked version of promoting instance checks that would allow fields to be promoted at the expense of a runtime check on uses here.

[leafpetersen]

I filed an issue with a concrete proposal for this here, I'm going to close this out.