Skip to content

Comparing Refined Types can lead to invalid subtype reconstruction #15485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abgruszecki opened this issue Jun 20, 2022 · 3 comments
Open

Comparing Refined Types can lead to invalid subtype reconstruction #15485

abgruszecki opened this issue Jun 20, 2022 · 3 comments
Labels
area:gadt itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException)

Comments

@abgruszecki
Copy link
Contributor

abgruszecki commented Jun 20, 2022

Originally posted by @Linyxus in #15175 (comment)

Regarding the soundness holes caused by resetting approx state: yes, we have more soundness holes caused by this. Each time isSubType(S, T) (instead of recur(S, T)) is called inside isSubType, the approx state is reset, and it will be a potential soundness hole. For example, I find another callsite of isSubType here, and we can make a counter-example showing the unsoundness brought by this:

enum SUB[-A, +B]:
  case Refl[C]() extends SUB[C, C]

trait Tag { type T }

def foo[A, B, X <: Tag{type T <: A}](e: SUB[X, Tag{type T <: B}], x: A): B = e match {
  case SUB.Refl() =>
    // unsound GADT constr because of approx state resetting: A <: B
    x
}

def bad(x: Int): String =
  foo[Int, String, Tag{type T = Nothing}](SUB.Refl(), x)  // cast Int to String

In this example, we are trying to extract necessary constraint from X <: Tag{type T <: B}, where X is known to be a subtype of Tag{type T <: A}. During this we will try upcasting LHS and compare Tag{type T <: A} <: Tag{type T <: B} with LHS approximated. Since when comparing the refinement info the approx state is reset, we derive the unsound constraint A <: B.

@Linyxus
Copy link
Contributor

Linyxus commented Jun 22, 2022

More explanation: after having a closer inspection of the issue, the call site mentioned in the original comment is incorrect and misleading. What actually happens:

  • We are trying to reconstruct subtyping from X <: Tag{T <: B},
  • When comparing this relation, TypeComparer will end up calling hasMatchingMember(T, X, Tag{T<:B}), which asks whether the type member T or LHS is a subtype of the RHS.
  • Here in hasMatchingMember queries the member T of X, which implicitly upcasts X to its upper bound Tag{T <: A}, leading to the unsoundness.

@Linyxus
Copy link
Contributor

Linyxus commented Jun 22, 2022

To verify, I added the tracing expression trace.force(i"$tp1 . $name", show = true) { tp1.member(name) }. This is what it prints:

==> X . T?
<== X . T = type T

The query returns the type member T of the upper bound of X. This implicitly upcasts X.

@dwijnand dwijnand added the itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException) label Jul 6, 2022
@ValeriePe
Copy link

ValeriePe commented Aug 8, 2022

This issue was picked for the Issue Spree n° 19 of August 16th which takes place in a week from now. @dwijnand @nmcb will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:gadt itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants