[vm/compiler] Fix bad check in CompilerType::IsAssignableTo.

In order to know whether a value is assignable to a given type,
we need the fully instantiated type. Thus, return false if the
type given to CompilerType::IsAssignableTo is uninstantiated,
similar to CompilerType::IsInstanceOf.

TEST=vm/dart{,_2}/regress_b_230945329_test.dart

Bug: b/230945329
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try
Change-Id: I7ff9eb2214debc274f09580d51c80f0921f8f77d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244200
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>

runtime/tests/vm/dart/regress_b_230945329_test.dart

@@ -0,0 +1,39 @@
+// Copyright (c) 2022, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Regression test for b/230945329.
+//
+// Check that AssertAssignables for the same uninstantiated type, where the
+// instantiated types at runtime may differ, are not optimized away.
+//
+// VMOptions=--no-use-field-guards --no-use-osr --deterministic --optimization-counter-threshold=5
+
+void main() {
+  final bar = Box<dynamic>('a'); // T=dynamic
+  final barInt = Box<int>(1); // T=int
+
+  for (int i = 0; i < 5; ++i) {
+    bar.bar(bar);
+  }
+  try {
+    barInt.bar(bar);
+    throw 'that should have failed!';
+  } on TypeError catch (e, s) {}
+}
+
+class Box<T> {
+  final T v;
+  Box(this.v);
+
+  void bar(Box box) {
+    // The uninstantiated compile type of box.v is T, same as the uninstantiated
+    // compile type it's being checked against. It's only the instantiated
+    // versions at runtime that could differ: the first instance type argument
+    // of box (box.v) vs. the first instance type argument of this (T).
+    baz(box.v/*=T*/ as T/*=T*/);
+  }
+}
+
+@pragma('vm:never-inline')
+baz(e) {}

runtime/tests/vm/dart_2/regress_b_230945329_test.dart

@@ -0,0 +1,41 @@
+// Copyright (c) 2022, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Regression test for b/230945329.
+//
+// Check that AssertAssignables for the same uninstantiated type, where the
+// instantiated types at runtime may differ, are not optimized away.
+//
+// VMOptions=--no-use-field-guards --no-use-osr --deterministic --optimization-counter-threshold=5
+//
+// @dart=2.9
+
+void main() {
+  final bar = Box<dynamic>('a'); // T=dynamic
+  final barInt = Box<int>(1); // T=int
+
+  for (int i = 0; i < 5; ++i) {
+    bar.bar(bar);
+  }
+  try {
+    barInt.bar(bar);
+    throw 'that should have failed!';
+  } on TypeError catch (e, s) {}
+}
+
+class Box<T> {
+  final T v;
+  Box(this.v);
+
+  void bar(Box box) {
+    // The uninstantiated compile type of box.v is T, same as the uninstantiated
+    // compile type it's being checked against. It's only the instantiated
+    // versions at runtime that could differ: the first instance type argument
+    // of box (box.v) vs. the first instance type argument of this (T).
+    baz(box.v as T);
+  }
+}
+
+@pragma('vm:never-inline')
+baz(e) {}

runtime/vm/compiler/backend/type_propagator.cc

@@ -853,7 +853,20 @@ bool CompileType::IsAssignableTo(const AbstractType& other) {
   if (other.IsTopTypeForSubtyping()) {
     return true;
   }
-  if (IsNone()) {
+  // If we allow comparisons against an uninstantiated type, then we can
+  // end up incorrectly optimizing away AssertAssignables where the incoming
+  // value and outgoing value have CompileTypes that would return true to the
+  // subtype check below, but at runtime are instantiated with different type
+  // argument vectors such that the relation does not hold for the runtime
+  // instantiated values.
+  //
+  // We might consider using an approximation of the uninstantiated type,
+  // like the instantiation to bounds, and compare to that. However, in
+  // vm/dart_2/regress_b_230945329_test.dart we have a case where the compared
+  // uninstantiated type is the same as the one in the CompileType. Thus, no
+  // approach will be able to distinguish the two types, and so we fail the
+  // comparison in all cases.
+  if (IsNone() || !other.IsInstantiated()) {
     return false;
   }
   if (is_nullable() && !Instance::NullIsAssignableTo(other)) {