StrongTypeSystemImpl.isAssignableTo returns wrong result in one case

[bwilkerson]

The type system implementation of isAssignableTo is broken in the case where the first argument is Set<Null> and the second argument is a type parameter T whose bound is something like Set<int>. (The class Set can be replaced by anything, and the type int can be replaced by anything other than Null.)

This is blocking #35120.

[MichaelRFairhurst]

Assuming I'm creating a correct a code example to this...

class C<T extends Set<int>> {
  T t;
  f() {
    t = Set<Null>(); // This looks like it should be valid at first...but its not!
  }
}

This looks like it should be valid at first...but its not!

Proof by contradiction:

C<HashSet<int>> set;
set.f();

In this case, set.t will be a Set<Null>, but that is not a subtype of HashSet<int>. So the only types assignable to T in this case are T and Null.

[bwilkerson]

@lrhn I originally opened this issue because I thought this was a bug in analyzer that was preventing some of the set literal tests from passing (set_literal_test.dart and const_set_literal_test.dart). If Mike is right, then I think that some portions of those two tests are invalid.

[eernstg]

@MichaelRFairhurst is right, knowing only T <: Set<int> we cannot prove Set<Null> <: T. You could think of the subtype universe as a layered structure: Set<Null> is at the bottom of the layer which contains all the types of the form Set<S>, but we may have additional layers below, such as HashSet<_>. Of course, you could also use a layer which is always available: C<Null>; with that, you'd need Set<Null> <: Null and that won't fly, either.

You could have other subtypes with a bound: <S extends T>() {/* S is a subtype of T here */}, but otherwise we have no other lower bounds for a type variable than Null (and bottom).

So maybe this issue should be retargeted to correct

those two tests

[lrhn]

I agree that there is a problem with the tests. Similar tests worked for double literals, so I copied them, but those tests didn't get into generics.

It's true that S declared as <S extends Set<num>> does not allow Set<Null> to be assigned to it, and hence it does not introduce a set literal.
That's probably a good thing. The expression S x = {}; has no good element type that we can infer for the set. If we use num, then it's a run-time error if S is actually Set<int> at run-time (which it very well may be). So, we can't safely make it a set, so it should be an error. It's annoying that it becomes a " Map<dynamic,dynamic> is not assignable to S extends Set<num> error, but for now I think we will live with that.

We could change the rules to, say, make the empty-set/map-literal a set if there exists some type T so that Set<T> is assignable to the context type (and no types K and V so that Map<K, V> is).
That would allow this case because Set<num> is assignable to S by downcast, but it will also likely break if we try to run it because of that downcast. Not much saved, and forcing the compiler into a searching for a solution.

I'll fix the tests: https://dart-review.googlesource.com/c/sdk/+/85740

[lrhn]

I'm fixing the test now, but keeping the door open for changing the specification if the current one is too restrictive.

[bwilkerson]

Given that the analyzer's implementation was right and the tests were wrong, I'm closing this.