Can't call isSuptype on types whose Java types are not subtypes

[smillst]

No description provided.

[mernst]

This looks OK to me.

[wmdietl]

Wouldn't it be more concise to add the check for subtyping of the underlying type to isSubtype?
Is there ever a reason for not including this check? It looks like it is always occurring together. The only exception I saw was in visitIntersection_Intersection which could keep the additional check.

[smillst]

Is there ever a reason for not including this check?

Yes, for example isSubtype of Integer and int are not erased Java subtypes, so the check can't be added to isSubtype.

[wmdietl]

It would seem much simpler to add that special casing into isSubtype.
An int is never a subtype of Integer, because boxing/unboxing would apply first.
Making many invocations of isSubtype simpler and documenting the boxing special case in isSubtype seems beneficial and clearer to me.

[smillst]

As documented, the Java types of the arguments to isSubtype are not always in a subtype relationship.

It would seem much simpler to add that special casing into isSubtype.

It would not be correct to do so. For example, isSubtype(@C Cloneable & @A List<@B String, @A List<@B String>) if the isErasedSubtype is moved to isSubtype, then isSubtype called recursively on @C Cloneable and @A List<@B String> would return false which would cause isSubtype(@C Cloneable & @A List<@B String, @A List<@B String>) to return false as well.

[wmdietl]

I don't understand your example. At the moment the code is

for (alternatives) {
  if (isErasedSubtype(A, B) && isSubtype(A, B) {
    return true;
  }
}
return false;

How is it not semantically equivalent to write:

for (alternatives) {
  if (isSubytpe(A, B)) {
    return true;
  }
}
return false;

and strengthen isSubtype:

boolean isSubtype(A, B) {
   if (!isErasedSubtype(A, B) {
     return false;
   }
}

[smillst]

It would probably be good to do this, but it is not as simple as adding isErasedSubtype to isSubtype. Aside from primitives, isErasedSubtype fails for the following combinations to isSubtype (and there are probably more):

int[], Object[]
U extends Object & Iterable<Object>, Iterable<Object>
Throwable&Cloneable&Unions.MyInterface<String>,  Object&Cloneable&Unions.MyInterface<String>

So, if we would like to add this functionality, it should be done in a separate pull request.

[wmdietl]

Ok, if you think that is easier.
Did you have a test case for this pull request?

[smillst]

This is a refactoring, so there are already some test cases. More complex ones require asSuper to work correctly. See tests in all-systems with Union or Intersection in name.

[wmdietl]

Ok, so I'm not sure why splitting this up into two refactorings makes it easier, but if you prefer, let's do that. Please file an issue for the remaining work.
(The PR title made it sound like something doesn't work without this change.)

[smillst]

I have more test cases in asSuper that crash on master because of asSuper bugs that also crash on asSuper because of this issue.

[wmdietl]

Is there a test case that fails without this change?