NNBD_TOP_MERGE(int?, int*) does not produce int?

[iarkh]

Dart VM version: 2.8.0-edge.aadcb4418b1a7ccbb74a7cc925ad55020ce4a924 (Thu Feb 6 02:00:49 2020 +0000) on "linux_x64"

According to NNBD Spec:

NNBD_TOP_MERGE(T?, S*) = NNBD_TOP_MERGE(T, S)?
NNBD_TOP_MERGE(T*, S?) = NNBD_TOP_MERGE(T, S)?

In the following code source:
testlib_out.dart:

// @dart=2.6
class A<T> {
  Type getType() => T;
}

class out_int extends A<int> {}

test.dart:

import "testlib_out.dart";

class B extends A<int?> {}
class C extends out_int implements B {}

Type typeOf<X>() => X;

main() {
  print(typeOf<int?>() == C().getType());
}

direct super-interface type parameter of the class C should be int?.
However, this is not so and dart prints false here.

[vsmenon]

@eernstg @sigmundch @fishythefish - do we have consensus on the right behavior here?

[eernstg]

For me: Yes, NNBD_TOP_MERGE(int?, int*) should yield int?; dart-lang/language#824 makes no difference here.

However, depending on the outcome of dart-lang/language#821 (actually, also independently of that issue), there would be a need to use a different technique than using Type.== to check that a given type argument has the expected value: If we make Object* == Object (which would be the case without issue 821) then it is still an unsafe technique.

[johnniwinther]

The CFE correct computes C to be a A<int?> (see the test added in https://dart-review.googlesource.com/c/sdk/+/136130 ).

The problem might be in the VM.

[crelier]

The VM indeed prints false for this example, which, in my opinion, is correct.
Indeed, the method getType() is resolved along the super class chain C -> out_int -> A<int>, as specified in the language specification, section "16.19 Lookup". Therefore, T is instantiated to int, and not int?. Note that the proposal of dart-lang/language#841 does not apply here, and it would not change the outcome anyway.
This behavior has nothing to do with NNBD_TOP_MERGE.
Now, if you augment main like this:

main() {
  print(typeOf<int?>() == C().getType());
  print(C() is A<int>);
  print(C() is A<int?>);
}

The VM will print:

false
true
true

which IMHO is correct too. But feel free to disagree :-)

[eernstg]

I wrote a bunch of tests about mixed inheritance ('tests/language/nnbd/mixed_inheritance/legacy_*'), and they are concerned with the type arguments of superinterfaces for the purposes of dynamic checks (like C() is A<T> for some T) and static checks (as in A<T> a = C(); for some T).

But they do not cover the run-time value of an expression in inherited code where a type variable is evaluated (like A.getType). I'm going to write tests covering this area, I just haven't gotten around to it yet.

It may indeed be the most useful choice to let C.getType() evaluate to int* (considering (1) that this is transitional, it goes away when mixed inheritance goes away, and considering (2) what's the cost, in terms of implementation efforts as well as run-time performance).

This means that instances of C contradict themselves ("C() implements A<int?>" vs. "C().getType() returns int*"), but if we insist that C().getType() must return int? then we get the effect that we may need to invalidate code which has been generated under the assumption that the value of the type argument is int*.

// Legacy 'testlib_out.dart'.
// @dart=2.6

Type typeOf<X>() => X;

class A<T> {
  Type getType() => T;
  bool test(Function f) => f.runtimeType == typeOf<void Function(T)>();
}

class out_int extends A<int> {}

// 'test.dart', with null-safety.
import "testlib_out.dart";

class B extends A<int?> {}
void bar(int i) {}
class C0 extends out_int {
  bool get test2 => super.test(bar); // Optimized to `=> true`.
}
class C extends C0 implements B {}

Class C would then need to have an implicitly generated overriding implementation of test2 if it must return false, because C insists that the implementation of inherited methods must behave as if it implements A<int?>.

I think it's very likely that we never optimize a case like the one above (for instance, it's crucial that we call super.test(...) not just test(...)), so it isn't trivial to come up with an example. ;-)

However, there could be other cases where implementations do perform similar optimizations that need to be invalidated if we "change our mind" about the value of a type variable in spite of the fact that it is already statically known (as in extends A<int> above).

I think it's fair to say that this is still blocked on #841.

PS: I created #841 exactly to clarify whether we should require inherited methods to reflect the mitigated implemented superinterfaces (e.g., that C implements A<int?> rather than being an error because it has both A<int*> and A<int?> in its superinterface graph), or we should allow the inherited methods to keep the semantics which is implied by the specific choice of superinterface at the declaration (for instance, C().getType() is allowed to return int* even though C implements A<int?>). So I think that #841 is relevant here.

[crelier]

Since the consensus is that mitigation proposed in dart-lang/language#841 will not be implemented, this issue becomes invalid and there is nothing to fix in the VM.