Imported From: https://issues.scala-lang.org/browse/SI-7517?orig=1
Reporter: @harrah
Affected Versions: 2.10.2-RC1

@paulp said:
This one is 7e52fb910b . I think it's a certifiable miracle that it used to compile.

@paulp said:
Ha, look at paulp calling it in #7226. "and it's not a risk-free fix"

Of course one can usually take that position without extending one's neck too far (with the compiler, one always can) and yet, somehow it has the air of true prophecy.

@retronym said:
It will be interesting to trace this through at and just before scala/scala@0cde930b. 7e52fb910b was designed to restore the pre-0cde930b state of affairs.

@paulp said:
That's why I was surprised it used to compile!

@paulp said:
It compiles both before and after 0cde930b . The point between 2.9 and 2.10 when it started working was 658ba1b4e6, which makes sense given the test case:

// test/files/pos/hkrange.scala
class A {
  def f[CC[X] <: Traversable[X]](x: CC[Int]) = ()
  
  f(1 to 5)
}

 commit 658ba1b4e6
 Author: Paul Phillips <paulp@improving.org>
 Date:   1 year, 10 months ago
 
     Fixed adriaan's patch for type constructor infe...
     
     Fixed adriaan's patch for type constructor inference. The problem with
     haranguing people in bars about bugs is that the fixes with which they
     provide you may be flawed. Fortunately moors has this novelist on
     retainer. Review by moors.

@retronym said:
Marking as critical for 2.10.2, not for SBT, but because we need to characterize the nature of the regression more precisely. I'll do that in the coming days.

@retronym said:
One more data point: Applying 7e52fb910b immediately after 0cde930b compiles the test case successfully. Another change also appears to be in play.

@retronym said:
The actual regression (or progression, perhaps), comes from the use of dealiasWidenChain in unifyFull.

scala/scala@e5da30b843#L5L3176

// old
tpes = List(Composed.this.Split[A], K[[T]A[B[T]]])

//new 
tpe.dealiasWidenChain.toList = List(Composed.this.Split[A], K[[T]A[B[T]]])

@paulp said:
Have I gone blind, or are those the same lists of types?

@paulp said:
This fixes it:

 -        tpe.dealiasWidenChain exists unifySpecific
 +        val tpes = if (isLowerBound) tpe.dealiasWidenChain else tpe :: Nil
 +        tpes exists unifySpecific

@paulp said:
But, unsurprisingly, breaks other things.

% qscalac -d /tmp test/files/pos/t6846.scala 
test/files/pos/t6846.scala:6: error: polymorphic expression cannot be instantiated to expected type;
 found   : [M[_], A]Test.Arb[M[A]]
 required: Test.Arb[Test.ListInt]
  foo: Arb[ListInt]
  ^
test/files/pos/t6846.scala:25: error: polymorphic expression cannot be instantiated to expected type;
 found   : [M[_], A]Test2.Carb[M[A]]
 required: Test2.Carb[Test2.ListInt]
  bar: Carb[ListInt]
  ^
test/files/pos/t6846.scala:26: error: polymorphic expression cannot be instantiated to expected type;
 found   : [M[_], A]Test2.Carb[M[A]]
 required: Test2.Carb[Test2.ListSingletonX]
  bar: Carb[ListSingletonX]
  ^
test/files/pos/t6846.scala:27: error: polymorphic expression cannot be instantiated to expected type;
 found   : [M[_], A]Test2.Carb[M[A]]
 required: Test2.Carb[Test2.ListSingletonY]
  bar: Carb[ListSingletonY]
  ^
four errors found

@retronym said:
Sorry, I pasted the wrong snippet.

Here are all of the before/after pairs. I'm not sure about the difference between the first and second attempt to do the old ad-hoc version of dealiasWidenChain for Composed.this.Split[A].

tpes = List(Composed.this.Split[A], K[[T]A[B[T]]])
tpe.dealiasWidenChain.toList = List(Composed.this.Split[A], K[[T]A[B[T]]])

tpes = List(T)
tpe.dealiasWidenChain.toList = List(T)
tpes = List(Any)
tpe.dealiasWidenChain.toList = List(Any)
tpes = List(B[T])
tpe.dealiasWidenChain.toList = List(B[T])
tpes = List(B[T])
tpe.dealiasWidenChain.toList = List(B[T])
tpes = List(B[T])
tpe.dealiasWidenChain.toList = List(B[T])
tpes = List(B[T])
tpe.dealiasWidenChain.toList = List(B[T])

tpes = List(Composed.this.Split[A])
tpe.dealiasWidenChain.toList = List(Composed.this.Split[A], K[[T]A[B[T]]])

tpes = List(x)
tpe.dealiasWidenChain.toList = List(x)
tpes = List(B[T])
tpe.dealiasWidenChain.toList = List(B[T])
tpes = List(B[T])
tpe.dealiasWidenChain.toList = List(B[T])
tpes = List(B[T])
tpe.dealiasWidenChain.toList = List(B[T])
tpes = List(B[T])
tpe.dealiasWidenChain.toList = List(B[T])

@retronym said (edited on May 28, 2013 9:48:42 PM UTC):
Here's another idea.

-addBound(tp.typeConstructor)
-isSubArgs(lhs, rhs, params, AnyDepth)
+isSubArgs(lhs, rhs, params, AnyDepth) && {addBound(tp.typeConstructor); true}

With this patch, both this test and t6846.scala compile. A quick run of pt --pos --neg is also green.

What I'm trying to avoid, is registering both type constructors K and Split as bounds, which leads to them later being glb-ed in a seemingly nonsensical way. (What does RefinementType(parents = K, Split) mean, anyway?)

@retronym said:
Oh, and we should stay open to the idea of a conservative partial-revert for 2.10.2; leaving the a progression for 2.11.x. I'm not quite sure how that would play out as #6846 itself fixed a regression.

I suppose it would be good to try to construct a simpler test that exhibits the same problem to gauge where this lands on the esoteric-o-meter.

Paul, perhaps could you discuss this one with Adriaan?

@JamesIry said (edited on May 29, 2013 8:31:37 PM UTC):
I like the proposed fix (as opposed to reverting), but I'm not qualified to like it. I'll talk to Adriaan.

@adriaanm said:
I think Jason's fix makes sense.

More generally, I think this is a good reminder that we've hit the limit of what we can fix in 2.10.x, especially regarding type checking.
We should probably start targeting master with fixes by default, and stop merging 2.10.x into master when it becomes too burdensome.

@retronym said:
I'm about to submit a PR with this change. Just collecting a trace unify* for this test case to help to explain things.

@retronym said:
scala/scala#2607

@paulp said:
Also: scala/scala#2615