Skip to content

Union matching not exhaustive or refining #5970

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

Closed
milessabin opened this issue Feb 22, 2019 · 6 comments
Closed

Union matching not exhaustive or refining #5970

milessabin opened this issue Feb 22, 2019 · 6 comments

Comments

@milessabin
Copy link
Contributor

Compiling the following,

object Test extends App {
  case class Foo[T](t: T)

  def foo[T](ft: T|Foo[T]): T = {
    ft match {
      case Foo(t) => t
      case t => t
    }
  }

  foo(Foo(23))
}

results in,

-- [E007] Type Mismatch Error: tests/run/typeclass-derivation7.scala:6:21 ------
6 |      case Foo(t) => t
  |                     ^
  |          Found:    T$1(t)
  |          Required: T
  |          
  |          where:    T is a type in method foo with bounds >: Test.Foo[T$1]
-- [E007] Type Mismatch Error: tests/run/typeclass-derivation7.scala:7:16 ------
7 |      case t => t
  |                ^
  |                Found:    (T | Test.Foo[T])(t)
  |                Required: T
two errors found

Intuitively the pattern matcher ought to be able to see that the t's on the RHS's of the branches conform to T. If we add type tests,

object Test extends App {
  case class Foo[T](t: T)

  def foo[T](ft: T|Foo[T]): T = {
    ft match {
      case Foo(t: T) => t
      case t: T => t
    }
  }

  foo(Foo(23))
}

The match is reported as not exhaustive and the type tests are reported as uncheckable, despite being intuitively irrefutable.

Replacing the T in the union with Unit eliminates the non-exhaustiveness, but still results in a complaint about uncheckability,

object Test extends App {
  case class Foo[T](t: T)

  def foo[T](ft: Unit|Foo[T]): T = {
    ft match {
      case Foo(t) => t
      case () => ???
    }
  }

  foo(Foo(23))
}
-- Warning: tests/run/typeclass-derivation7.scala:6:11 -------------------------
6 |      case Foo(t) => t
  |           ^^^^^^
  |           the type test for Test.Foo[T] cannot be checked at runtime
one warning found
@liufengyun
Copy link
Contributor

I agree that the uncheckable warning in the last example is spurious, and should be fixed.

For the warnings & type checking errors in the first two examples, I am not so sure, giving the possibility of the following program:

object Test extends App {
  case class Foo[T](t: T)

  def foo[T](ft: T|Foo[T]): T = {
    ft match {
      case Foo(t: T) => t
      case t: T => t
    }
  }

  foo[Foo[Int]](Foo(23))
}

@milessabin
Copy link
Contributor Author

milessabin commented Feb 22, 2019

foo[Foo[Int]](Foo(23))

Good point.

Is there a way of capturing this idea without unsafe casts?

@liufengyun
Copy link
Contributor

@milessabin It seems to me the safe way is the standard sum types:

object Test extends App {
  case class Foo[T](t: T)

  def foo[T](ft: Either[T, Foo[T]]): T = {
    ft match {
      case Left(t) => t
      case Right(Foo(t)) => t
    }
  }

  foo(Left(Foo(23)))
}

(Typecase + union types + type parameters + erasure semantics) creates complexity.

@milessabin
Copy link
Contributor Author

Unfortunately the aim was to avoid boxing in the common, ie. non-Foo case ... Foo is similar to Left in the real code.

@LPTK
Copy link
Contributor

LPTK commented Feb 22, 2019

cf. #5826

@milessabin

Is there a way of capturing this idea without unsafe casts?

there is no way to achieve that because there is no way to prevent T from being instantiated to a subtype of Foo[_].

You could make Foo final and the type parameter T <: NotFoo where NotFoo is a trait that Foo does not extend, and in principle that should work. But I presume you don't want/can't have all your unboxed things extend NotFoo.

So you'll have to go with a similar approach to Sebastian's unboxed options (call it UnboxedEither). You'll need an opaque type, casts, and to be careful to actually box instances of Right(Left(...)) – because these ones can't be left unboxed for obvious reasons.

@milessabin
Copy link
Contributor Author

Closing this as a duplicate of #5826. I'll follow @LPTK's suggestion of investigating an approach similar to @sjrd's unboxed options.

liufengyun added a commit that referenced this issue Mar 20, 2019
Fix #5970: suppress spurious warning in isInstanceOf check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants