Skip to content
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

Fix #4314: revert changes in #4299 #4333

Merged
merged 5 commits into from
Apr 20, 2018
Merged

Conversation

liufengyun
Copy link
Contributor

Fix #4314

  1. revert use ApproximatingTypeMap to simplify exhaustivity check code #4299: ApproximatingTypeMap produces Nothing, which is not what is needed.
  2. Avoid blind erasure which lose information about the pattern, see tests/patmat/i4314b.scala

1. ApproximatingTypeMap produces Nothing, which is not what is needed.
2. Avoid blind erasure which lose information about the pattern, see tests/patmat/i4314b.scala
Blind substitution of pattern bound type symbol with WildcardType
will cause t2425.scala fail, as the following holds:

    Array[_] <:< Array[Array[_]]

To walkaround the problem, we don't substitute pattern bound symbols
that are immediate parameters to `Array`.
@liufengyun
Copy link
Contributor Author

test performance with #fast please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/4333/ to see the changes.

Benchmarks is based on merging with master (ac84047)

@@ -356,7 +356,11 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {

val map = new TypeMap {
def apply(tp: Type) = tp match {
case tref: TypeRef if isPatternTypeSymbol(tref.typeSymbol) => WildcardType(tref.underlying.bounds)
case tp @ AppliedType(tycon, args) if tycon.isRef(defn.ArrayClass) =>
// walkaround `Array[_] <:< Array[Array[_]]`, see tests/patmat/t2425.scala
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment is true, the following code doesn't compile:

def foo(x: Array[_]): Array[Array[_]] = x

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underscore above expands to TypeBounds Array[<: Any :> Nothing] instead of Wildcards.

Here is the subtyping log:

  ==> Array[(?)] <:< Array[Array[(?)]]
    ==> scala.type <:< scala.type
    <== scala.type <:< scala.type   = true
    ==> Array[(?)] <:< (?)
      ==> Any <:< Any LoApprox
      <== Any <:< Any LoApprox  = true
    <== Array[(?)] <:< (?)   = true
    ==> (?) <:< Array[(?)]
    <== (?) <:< Array[(?)]   = true
  <== Array[(?)] <:< Array[Array[(?)]]   = true

And the behavior can be confirmed from the code in TypeComparer.

But thanks for raising the question, as by using TypeBounds instead WildcardType, we avoid the problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, WildcardType(lo, hi) <:< X is equivalent to lo <:< X, see TypeComparer

val map = new TypeMap {
def apply(tp: Type) = tp match {
case tp @ AppliedType(tycon, args) if tycon.isRef(defn.ArrayClass) =>
// walkaround `Array[_] <:< Array[Array[_]]`, see tests/patmat/t2425.scala
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/walkaround/work around/

Use TypeBounds instead of WildcardType. WildcardType has the
following unexpected behavior:

    Array[?] <:< Array[Array[?]]
@liufengyun liufengyun merged commit 78ab82d into scala:master Apr 20, 2018
@liufengyun liufengyun deleted the fix-4314 branch April 20, 2018 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants