Legacy code interaction with Records

[lrhn]

tl;dr: I don't think there is any problem with this, it should just work. Writing this to give a place to write concerns, if we come up with some.

If we introduce records in Dart 2.x.0, existing pre-2.x.0 code will may up interacting with record types and record values.
(Some of it pre-2.12.0 code too.)

What we've done with type system changes so far (aka: the null safety update) is to have a shared type system that both legacy and new code interacts with, but the legacy code has none of the new syntaxes available.

Legacy code seeing record values or types.

Record values can flow into legacy code typed as Object.

Opaque record types can flow into legacy code as type parameters, or type arguments of instances of generic classes (like <(int, int)> or List<(int, int)>. The static type of those in the legacy code will likely not contain record type.

Concrete record types can be introduced in legacy code by referencing type aliases from new code, or by having record types be filled in by type inference for legacy code referencing new code. (At the extreme, a legacy class extending a new-code class can inherit the parameters and return type of a method, which can contain record types.)

All of these should probably just work.

Since we make Record a subtype of Object, the legacy code won't need to do anything special, just treat the record types and objects as types and instances of types declared in other libraries that it hasn't seen.
(It actually has seen at least one of them, since Record will be in dart:core, unless we hide that declaration from pre 2.x.0 code. We probably don't need to hide it.)

There probably shouldn't be any problems with that. Legacy code treats the record types as any other foreign type.

Member access

I have suggested that member access on records could be "extension getter-like" so that it doesn't work with dynamic invocations. (Also means you cannot pass (x: 1, y: 2) to something which expects a Point, but does dynamic member access instead of typed member access.)

If we do that (EDIT: We don't! Members are normal instance getters), a legacy library can only destructure a record using typed access to field getters, r.$0 and r.fieldName, since it cannot use pattern matching, cannot cast the record to a static record type (unless it uses a type alias from another library), and cannot access field getters, even through dynamic invocations.

If member access uses normal virtual methods, a dynamic invocation of (o as dynamic).$0 or (o as dynamic).fieldName would also be able to destructure the record.

Legacy code deliberately using record types, by using type aliases from new-code libraries, is probably going to be rare. Might as well just migrate. If the code is pre-2.12 too, you should really prioritize null safety migration over using records.

I don't think legacy code not being able to interact with records, other than by shuffling them around, is a problem. Might even be seen as an advantage, and any legacy code that actually works on concrete record types may be worth warning about.
(At least upgrade the SDK min requirement, and add a //@dart=2.x-1 marker, since the code definitely depends on a 2.x.0 SDK.)

[rakudrama]

An aspect of legacy code is the lack of sound null safety which causes an asymmetry between is and as, with as T accepting null when is T returns false.

Does this asymmetry push down into structural type checks on records as R?

dynamic r = (null, null);
r as (int, int); // throws or accepts?

This test will most likely occur in parametric covariance checks, like that for the argument of List.add.

List<(int,int)> spans = [];
void addSpan(int start, int end) { spans.add((start, end)); }

For dart2js and DDC is and as are implemented as testing stubs on the dynamic type representation (aka Rti).
The general as cast for non-sound mode is implemented as a null check followed by an is check, something like:

T.as(o) => o == null || T.is(o) ? o : throw TypeError(... o ... T ...);

If (null, null) should be accepted by as (int int) then this is an extra implementation burden. We will need to implement as and is via independent structural checking rather than use the one-line trick of implementing as via is. We will essentially need a third method isForAs.

I hope we can make as R 'more' sound and avoid this dual work that is throw-away work.
Are there problems with this, for example, when the record contains functions? Or are the problems with a sound null safety 'membrane' more to do with actually calling the functions?

class Transcoders<A, B> {
   List<(A Function(B), B Function(A))> endodeDecodePairs ...
}

[lrhn]

I'd generally say that we should be pushing down casts. An (a, b) as (X, Y) should be completely type-equivalent to (a as X, b as Y) (not completely equivalent because b is evaluated before the value of a is cast in the former).

That said, I'd still be fine with not having backwards compatible unsound checks, though. It's fine to reject (null, null) as (int, int) (IMO). From the legacy code's perspective, a record type is just some opaque class that it doesn't know anything about. There should be no legacy code which relies on (null, null) being accepted as (int, int), and existing legacy code is the only reason we have that behavior at all.

I'd also expect per-field implicit downcasts, so var x = (1, 1 as dynamic); (int, int) y = x; will be an implicit (int, int) y = (x.$0, x.$1 as int);.
(Are we doing that? EDIT: We are not.)

Or, to be precise, I expect the assignment (T1 v1, T2 v2) = (e1, e2); rest; to work precisely where (T1 v1, T2 v2) { ... } (e1, e2); would work. A record assignment is like a parameter list binding to an argument list, each entry is handled independently. It's like a pair is just two independent values in trenchcoat.

Could your testing stubs on the representation of a record type recursively call the testing stubs of the field types on the field values of the incoming record (if it is a record, otherwise it's definitely rejected)?

Or would you want it to do Record<(X, Y)>.as(o) => o is Record<_,_> && X.is(o.$0) && Y.is(o.$1) directly?

The record won't be carrying around type information separate from the objects anyway, so you can't check the type of a record without checking the types of its elements. (Well, it can carry extra information around, but that could be unnecessarily expensive.)

[rakudrama]

I'd generally say that we should be pushing down casts. An (a, b) as (X, Y) should be completely type-equivalent to (a as X, b as Y) (not completely equivalent because b is evaluated before the value of a is cast in the former).

So should the error message say

type 'bool' is not a subtype of type 'int'

instead of

type '(int, bool)' is not a subtype of type '(int, int)'

?

Could your testing stubs on the representation of a record type recursively call the testing stubs of the field types on the field values of the incoming record (if it is a record, otherwise it's definitely rejected)?

We can't rely on one general 'recursive' stub because it will be too inefficient. There is a lot of polymorphism at the JavaScript level for a general stub.

Or would you want it to do Record<(X, Y)>.as(o) => o is Record<(_,_)> && X.is(o.$0) && Y.is(o.$1) directly?

Yes. I want a specialized is stub per shape where the 'loop' is unrolled.

Record<(X, Y)>.is(o) => o is Record<(_,_)> && X.is(o.$0) && Y.is(o.$1)

that is possibly further specialized for some cases, e.g.

Record<(X, num)>.is(o) => o is Record<(_,_)> && X.is(o.$0) && typeof o.$1 === "number"
Record<(dynamic, Y)>.is(o) => o is Record<(_,_)> && Y.is(o.$1)

et cetera.

The as stub Record<(X, Y)>.as(o) will be the single as stub T.as mentioned earlier.

If we must push down 'legacy-as' and permit null values then we need either (1) to duplicate this generated pattern and specializations for as

Record(<X,Y>).as(o) {
  if (o is! Record<(_,_)>) throw ...
  X.as(o.$0);
  Y.as(o.$1);
  return o;
}

or (2) have another stub that is almost like is but for the test that as performs.

I really want to avoid having to do all this specialization twice.

[lrhn]

I'd be fine with the

type 'bool' is not a subtype of type 'int'

message, if it's easier to achieve. It might be harder to point to a specific place in the code. If you have pair as PairType, you can't point to the second element of either input or type, but saying;

type 'bool' is not a subtype of type 'int' of `pair.$1` (line, column of "pair")

I think it's readable enough.

Also fine with not pushing down legacy-as. Treat records in legacy code like opaque classes.
(Will records even release before Dart 3.0?)

[eernstg]

It might depend on the textual representation of the record type. If it's a typedef (MyRecordType) or a parameter type (not shown in the code that I'm looking at) then "'(int, int)' is not a subtype of 'MyRecordType'" might be more helpful than "'int' is not a subtype of 'bool'", especially if the discrepancy occurs in some nested record type.

[munificent]

So should the error message say

type 'bool' is not a subtype of type 'int'

instead of

type '(int, bool)' is not a subtype of type '(int, int)'

?

Nooo. While the mechanics of the subtype testing might be to recurse into the fields, from the user's perspective, a record type is a type, and not a transparent bag of types that can be piecewise compared to other transparent bags of types.

It might depend on the textual representation of the record type. If it's a typedef (MyRecordType) or a parameter type (not shown in the code that I'm looking at) then "'(int, int)' is not a subtype of 'MyRecordType'" might be more helpful than "'int' is not a subtype of 'bool'", especially if the discrepancy occurs in some nested record type.

This is another very good reason not to show the error only as a piecewise failure. I do think it might be nice to report the error like:

type '(int, bool)' is not a subtype of type '(int, int)' because field 1 `bool` is not a subtype of `int`

[leafpetersen]

Proposed specification here.

[munificent]

Completed by #2538.