CFE/Analyzer inconsistent behavior of instantiate to bounds

[jmesserly]

Instantiate to bounds works differently if a bound is omitted in DDC depending on the front-end. Analyzer fills in dynamic, CFE/Kernel fills in Object. Inference behavior is different as well:

void f<T>() => print(T);
void g<T extends dynamic>() => print(T);
void h<T extends Object>() => print(T);
main() {
  print('Runtime behavior:');
  (f as dynamic)(); // DDC: dynamic, DDK: Object
  (g as dynamic)(); // both: dynamic
  (h as dynamic)(); // both: Object
  print('\nStatic behavior:');
  f(); // both: dynamic
  g(); // both: dynamic
  h(); // DDC: Object, DDK: dynamic
}

DDC and DDK both use the same runtime. The reason it behaves differently is the compile time bound is recorded differently. CFE/Kernel fills it in like this:

  static method f<T extends core::Object>() → void
    return core::print(test::f::T);

Statically, I'm not totally sure what's going on, in particular, why front_end chooses dynamic for T in h(). It appears the inference algorithms are slightly different in how they treat the bound.

I like that DDC/Analyzer's behavior is consistent: both runtime and compile time give the same results. But regardless we need the two implementations to be more consistent.

Thoughts? @leafpetersen @lrhn @eernstg

My reading of https://github.com/dart-lang/sdk/blob/master/docs/language/informal/instantiate-to-bound.md suggests that dynamic is the correct answer for omitted bounds, but that informal spec covers generic classes rather than generic methods. (The Dart 1 spec said that omitted bounds were Object, but that had little practical effect, as raw types were instantiated with dynamic, and they're both top types for the purposes of checking whether the type argument is legal w.r.t. the bound.)

[jmesserly]

Note: bug title is about DDC/K but in reality, this is more of an inconsistency between analyzer and front_end

[eernstg]

Right, the "default default" (that is, the top type chosen by instantiate-to-bound as the type argument when there is no bound) is dynamic and not Object, and when there is an explicit bound it will be used.

For (f as dynamic)(), f is a generic function which is invoked dynamically with no type arguments, and in that situation the feature spec says 'instantiate to bound ... is used to provide type arguments to dynamic invocations of generic functions when no actual type arguments are passed'. So it should print dynamic. So should (g as dynamic)(), because dynamic is explicitly specified as the bound, and (h as dynamic)() should print Object. So DDC is right so far.

For f(), the missing type argument is provided at compile-time by inference, and it should again be chosen using instantiate-to-bound. Note that instantiate-to-bound relies on the declaration of the given entity (here: the generic function that we're calling), which means that there is no difference between the results obtained dynamically and statically. Inference might use other sources of information for a given invocation, but when there is no additional information at the call site it relies exclusively on instantiate-to-bound. This means that the dynamic and static case use the same algorithm on the same data, hence producing the same result. DDC is right again.

If the CFE fills in Object as the bound for every type variable that has no explicit bound (and it doesn't preserve the information anywhere that this bound was synthetic) then I can't see how various back-ends using CFE could make the necessary distinction.

It would be nice if the CFE could simply fill in missing type variable bounds with dynamic, but this would have other effects that we probably do not want (and that we would certainly need to discuss rather than just having them happen by accident). In particular, it would mean that the source code in bodies of generic classes and functions would be subject to more forgiving static checking (foo<T>(T t) { t.whatever(42); } would be OK). There might be other unintended effects, but this is already enough for me to conclude that we shouldn't do that.

It looks like the CFE would need to somehow remember that these Object bounds are synthetic. Alternatively, if possible, all the work that needs to make this distinction (like computation of instantiate-to-bound) must be done and recorded in the kernel code, such that back-ends can do their job without knowing that some of these Object bounds are synthetic.

[jmesserly]

Edited the subject, as this sounds like a CFE bug.

[efortuna]

Yes, this is definitely a CFE bug. I just ran into a more obvious failure of the same thing:

foo<T extends num>(T bar, int p) {
  return bar + p;
}

confuse(x) {
  return x;
  return x;
}

main() {
  dynamic r = 4;
  r = confuse(r);
  print(foo(r, 5));
  print(foo<double>(2.0, 4));
}

Then if you dump the produced IR, you get:

 pkg/front_end/tool/fasta compile  --dump-ir foo.dartlibrary;
import self as self;
import "dart:core" as core;

static method foo<T extends core::num>(self::foo::T bar, core::int p) → dynamic {
  return bar.+(p);
}
static method confuse(dynamic x) → dynamic {
  return x;
  return x;
}
static method main() → dynamic {
  dynamic r = 4;
  r = self::confuse(r);
  core::print(self::foo<dynamic>(r, 5));
  core::print(self::foo<core::double>(2.0, 4));
}

Instead, that first invocation of self::foo should read:

core::print(self::foo<num>(r, 5));

because num is the lower bound in this case.

adding @kmillikin so he can assign to the proper person to fix this.
cc @sigmundch

[chloestefantsova]

Yep, sorry for that. I'm working on a fix in CL 53212.

Also, the following program that is currently rejected by the CFE should be compiled correctly after the fix:

class A<T extends num> {
  final T t;
  A(this.t);
}

dynamic foo(dynamic x) => x;

main() {
  var a = A(foo(42));
}

[chloestefantsova]

Ok, so I think there are two issues that are being discussed here:

1. The issue pointed out by the original post. I think it would be possible to resolve that after Proposal: store instantiate-to-bounds result in kernel #31581 is fixed. I'm putting my thoughts about how it can be done below.
2. The issue pointed out in Emily's CFE/Analyzer inconsistent behavior of instantiate to bounds #32483 (comment) and my CFE/Analyzer inconsistent behavior of instantiate to bounds #32483 (comment). And I'm a bit confused about these cases. It seems like the upwards inference suggests dynamic as the type argument here, so dynamic should be inferred, and then a compile-time error should be generated, because dynamic is not a subtype of num. However, I may misinterpret something, and the bound participates in the inference in some way that I'm not familiar with. @leafpetersen could you please take a look at these examples?

Now, I want to write how (1) could be resolved after #31581 is fixed. The idea is to give defaultType field to TypeParameter node from Kernel and to populate the field with the type from instantiate-to-bound. Since in the CFE we know if the bound was specified explicitly or was omitted, we may distinguish between the generated Object and the explicit Object and set defaultType accordingly. The following mock-up Kernel code illustrates this approach.

// The bound is omitted, so `defaultType` is `dynamic`.
void f<T extends Object = dynamic>() => print(T);

// The bound is specified explicitly, so `defaultType` equals to that.
void g<T extends dynamic = dynamic>() => print(T);

// The bound is specified explicitly, so `defaultType` equals to that.
void h<T extends Object = Object>() => print(T);
main() {
  print('Runtime behavior:');
  // `f` is a tear-off here, so no type arguments are provided by the CFE.
  (f as dynamic)();

  // Same as above: no type arguments are provided by the CFE to the tear-off.
  (g as dynamic)();

  // Same as above: no type arguments are provided by the CFE to the tear-off.
  (h as dynamic)();

  print('\nStatic behavior:');

  // Filled in by the CFE with the contents of `defaultType`, that is, `dynamic`.
  f<dynamic>();

  // Filled in by the CFE with the contents of `defaultType`, that is, `dynamic`.
  g<dynamic>();

  // Filled in by the CFE with the contents of `defaultType`, that is, `Object`.
  h<Object>();
}

Here the = <DartType> notation describes the defaultType field.

In order to achieve the desired run-time behavior, the back ends will need to fetch the contents of defaultType and use it as the corresponding type argument in dynamic invocations, that is, when the static type of the invoked target is dynamic. This way, the dynamic invocation of f will print dynamic, the dynamic invocation of g will print dynamic, and the dynamic invocation of h will print Object.

The reason why currently the raw types with type variables that have explicit Object as their bound receive dynamic as the result of instantiate-to-bound is, as Erik mentioned, that there's no way to distinguish generated Object and explicitly mentioned Object in Kernel. And currently we need to apply instantiate-to-bound to raw types that reference generic classes from compiled dill files as well as those from source files. For dill files, the information about the way Object appeared is lost. So, in order to be consistent in the cases when the generic classes (or typedefs) come from the source or from dill files, we need to treat Object as dynamic everywhere. But the proposed change is to populate defaultType when the information is still available in the CFE, and then serialize the contents of defaultType in the dill files, so there's a way to do the right thing consistently everywhere.

[leafpetersen]

@leafpetersen could you please take a look at these examples?

Yes, both of those examples should be inference failure errors (as the analyzer reports) since dynamic is not a valid instantiation.

T foo<T extends num>([T x]) => x;

class A<T extends num> {
  final T t;
  A([this.t]);
}

dynamic bar(dynamic x) => x;

void main() {
  dynamic r;
  var a = foo(r); // Failed inference
  var b = new A(bar(42)); // Failed inference
  var c = foo();  // Inferred as foo<num>
  var d = A(); // Inferred as A<num>
}

[chloestefantsova]

Small status update. #31581 is resolved, and the programs are now compiled into the form that allows backends to retrieve the default types (that is, the results from instantiate-to-bound) from type parameters and use them in place of the missing type arguments in dynamic invocations of generic functions.

The example script from the original message by @jmesserly is now compiled into the following representation in kernel:

library;
import self as self;
import "dart:core" as core;

static method f<T extends core::Object = dynamic>() → void
  return core::print(self::f::T);
static method g<T extends dynamic = dynamic>() → void
  return core::print(self::g::T);
static method h<T extends core::Object = core::Object>() → void
  return core::print(self::h::T);
static method main() → dynamic {
  core::print("Runtime behavior:");
  (self::f as dynamic).call();
  (self::g as dynamic).call();
  (self::h as dynamic).call();
  core::print("\nStatic behavior:");
  self::f<dynamic>();
  self::g<dynamic>();
  self::h<dynamic>();
}

In the listing above, h receives core::Object as the default type of its type parameter. Backends should pass it as the type argument in the dynamic invocations of h. The VM already does so:

$ out/ReleaseX64/dart --preview-dart-2 example.dart
Runtime behavior:
dynamic
dynamic
Object

Static behavior:
dynamic
dynamic
dynamic

Note that the static invocation still receives dynamic. This is addressed in CL 55897. I hope to get it reviewed and landed in the next week.

[chloestefantsova]

Ok, the static part of the fix has just landed: 2c9d892. With that patch in place, the original example is compiled into the following in strong mode:

library;
import self as self;
import "dart:core" as core;

static method f<T extends core::Object = dynamic>() → void
  return core::print(self::f::T);
static method g<T extends dynamic = dynamic>() → void
  return core::print(self::g::T);
static method h<T extends core::Object = core::Object>() → void
  return core::print(self::h::T);
static method main() → dynamic {
  core::print("Runtime behavior:");
  (self::f as dynamic).call();
  (self::g as dynamic).call();
  (self::h as dynamic).call();
  core::print("\nStatic behavior:");
  self::f<dynamic>();
  self::g<dynamic>();
  self::h<core::Object>();
}

And here is the output from running the script with the VM in Dart 2 mode:

$ out/ReleaseX64/dart --preview-dart-2 example.dart
Runtime behavior:
dynamic
dynamic
Object

Static behavior:
dynamic
dynamic
Object

The remaining part of the issue is to produce a compile-time error for the code from Emily's comment as suggested by Leaf.

[chloestefantsova]

Ok, I guess when you consider the title of this issue and the remaining issues discussed in the thread, you may see that they are slightly different. The issue in the code from Emily's comment is actually about checking the type arguments against their bounds after type inference is done, and is not about instantiate-to-bound.

So, I'm closing this issue and will be tracking the progress on the remaining in a new one: #33308.