Closed scabug closed 13 years ago
Imported From: https://issues.scala-lang.org/browse/SI-3152?orig=1 Reporter: @retronym
@retronym said: Tested with: Scala compiler version 2.8.0.r21069-b20100305020157 -- Copyright 2002-2010, LAMP/EPFL
@retronym said: Here are two more datapoints, one positive, one negative.
trait Applicative[M[_]]
sealed trait MA[M[_], A] {
def sequence[N[_], B](implicit a: A <:< N[B], n: Applicative[N]): N[M[B]] = error("stub")
def sequence3[N[_], B]()(implicit a: A <:< N[B], n: Applicative[N]): N[M[B]] = error("stub")
}
object test {
implicit def ListMA[A](l: List[A]): MA[List, A] = error("stub")
implicit val ao: Applicative[Option] = error("stub")
/* This compiles OK:
(Nil: List[Option[Int]]).sequence3(): Option[List[Int]]
*/
// error: immutable is not an enclosing class
// !!! No line number is reported with the error
(Nil: List[Option[Int]]).sequence: Option[List[Int]]
}
@retronym said:
The crux of the problem appears to be in inferExpressionInstance
when called with these arguments:
tree = "test.this.ListMA[Option[Int]]((immutable.this.Nil: List[Option[Int]])).sequence[N, B]"
tparams = "List(type N, type B)"
pt = "Option[List[Int]]"
keepNothings = false
It gets to a point where:
detargs = "List(N, Int)"
detparams = "List(type B)"
Should there be an invariant that these lists are the same length?
This mutates tree to:
test.this.ListMA[Option[Int]]((immutable.this.Nil: List[Option[Int]])).sequence[N, N]
and returns:
undetparams$$3 = "List(type N)"
@adriaanm said: (In r21128) closes #3152: refactored adjustTypeArgs and methTypeArgs so that tparams are correctly split into ones that were inferred successfully, and that thus have a corresponding type argument, and those that weren't determined
I didn't investigate the exact cause of the final error message in the bug report, but Jason Zaugg's observations seems correct and I never liked that uninstantiated buffer in the first place.
review by odersky
@adriaanm said: (In r21129) slight (syntactic) cleanup of patch for see #3152 -- sorry, only realised when looking over my patch again
@adriaanm said: reopening because the "fix" caused #3477
@adriaanm said: the original patch addressed the mismatch between the list of type arguments (including dummy-Nothings) and the type params (where undetermined ones are dropped) by dropping both argument and parameter when they could not be determined (before, only the list of type parameters was pruned)
[the "dual" solution would be to stop dropping the parameter before doing the substitution (which implies undetermined parameters would get instantiated to themselves)]
undetermined type params are treated in two different ways:
1) they don't get instantiated: for undetermined type params, adjustTypeArgs returns the type that refers to the type parameter so substitution is a no-op
NOTE: eventually they do get substituted -- but where?!
2) when checking the bounds, we should pretend undetermined type parameters have been instantiated to Nothing
@harrah said: I came across this variant while porting some code to 2.8. In RC3 and r22181:
scala> val a = Array(1,2,3)
a: Array[Int] = Array(1, 2, 3)
scala> def x(a: Seq[Int]): Seq[Int] = a
x: (a: Seq[Int])Seq[Int]
scala> x(a)
res0: Seq[Int] = WrappedArray(1, 2, 3)
scala> x(a.toArray)
<console>:8: error: scala is not an enclosing class
x(a.toArray)
^
@adriaanm said: I can repro in one line. Who does better?
scala> Array(1).toArray : Seq[Int]
<console>:6: error: scala is not an enclosing class
Array(1).toArray : Seq[Int]
@adriaanm said: ok, a double whammy!
my current hypothesis is that something was wrong in mkAttributedQualifier: it generated a This(scala) tree instead of an Ident(scala) tree as the target of the selection scala.Predef... don't know if this is because the input type that seems to come from an ImplicitInfo in implicitss, was wrong, or whether mkAttributedQualifier was broken
current fix assumes the latter
@adriaanm said: meh, hypothesis wrong... will need to find other solution
@adriaanm said: Martin, reassigning to you as this one is beyond me.
typedThis does not allow trees like This(scala) but they are generated in TreeGen by calls from addImport
, which calls mkAttributedStableRef(pkg)
, as well as other code-paths that start in Implicits
this only comes up in this case because the Tree's symbol is set by
def mkAttributedThis(sym: Symbol): Tree =
This(sym.name) setSymbol sym setType sym.thisType
so typedThis
doesn't actually type check those (qualifiyingClass has a shortcut in case the tree's symbol is already set), but resetAllAttrs
, which is called in the adapt case I mentioned in an earlier comment, resets the type for these trees so that now they will have to be type checked
should typedThis be made more resilient? should these This(pkg)
trees never be generated in the first place? ([http://github.com/adriaanm/scala/commit/4d2432b48caaa41217fb46f11b6f90663ac146c7 I tried that], but a locker that does that fails in quick.lib in mysterious ways that probably have something to do with stepping on the toes of bootstrapping and Predef)
@adriaanm said: [urgh -- why can't I edit my comments... here's the text that shouldn't have gone in that code block]
so typedThis
doesn't actually type check those (qualifiyingClass has a shortcut in case the tree's symbol is already set), but resetAllAttrs
, which is called in the adapt case I mentioned in an earlier comment, resets the type for these trees so that now they will have to be type checked
should typedThis be made more resilient? should these This(pkg)
trees never be generated in the first place? ([http://github.com/adriaanm/scala/commit/4d2432b48caaa41217fb46f11b6f90663ac146c7 I tried that], but a locker that does that fails in quick.lib in mysterious ways that probably have something to do with stepping on the toes of bootstrapping and Predef)
@adriaanm said: (In r22523) cleaned up the mess that resulted from cobbling together fixes for see #3477 and see #3152 adjustTypeArgs and methTypeArgs now return a LinkedHashMap[Symbol, Option[Type]] TODO: check that type inference is still just as lightning fast as before
@jrudolph said: Args, now I'm at this point as well. Any news about what is happening here and how to get around it?
@adriaanm said: and another repro:
scala> def compare[T](x: Ordered[T], y: Ordered[T]) = error("")
compare: [T](x: Ordered[T],y: Ordered[T])Nothing
scala> def mkEx: Ordered[_] = error("")
mkEx: Ordered[_]
scala> compare(mkEx, mkEx)
<console>:8: error: math is not an enclosing class
compare(mkEx, mkEx)
^
<console>:8: error: math is not an enclosing class
compare(mkEx, mkEx)
@paulp said: Here's something weird. (Preface: apparently the addition of implicit def SeqDerived to Ordering means that adriaan's recent repro is now a diverging implicit expansion. Is that indicative of a bigger problem? Should we maybe pull that? So one has to get rid of that to see this.)
Why does ordered fail with the "enclosing class" error but Ordering completely differently?
object Weird {
def compare[T](x: Ordered[T], y: Ordered[T]) = error("")
def mkEx: Ordered[_] = error("")
compare(mkEx, mkEx)
def compare2[T](x: Ordering[T], y: Ordering[T]) = error("")
def mkEx2: Ordering[_] = error("")
compare2(mkEx2, mkEx2)
}
Failures:
weird.scala:4: error: math is not an enclosing class
compare(mkEx, mkEx)
^
weird.scala:4: error: math is not an enclosing class
compare(mkEx, mkEx)
^
weird.scala:8: error: type mismatch;
found : Ordering[_$$2] where type _$$2
required: Ordering[Any]
Note: _$$2 <: Any, but trait Ordering is invariant in type T.
You may wish to investigate a wildcard type such as `_ <: Any`. (SLS 3.2.10)
compare2(mkEx2, mkEx2)
^
weird.scala:8: error: type mismatch;
found : Ordering[_$$2] where type _$$2
required: Ordering[Any]
Note: _$$2 <: Any, but trait Ordering is invariant in type T.
You may wish to investigate a wildcard type such as `_ <: Any`. (SLS 3.2.10)
compare2(mkEx2, mkEx2)
^
four errors found
@paulp said: I'm going to raise some more tickets to high to make sure questions are resolved before 2.9 ships. In this case the high priority refers not to the bug but my most recent comment and the effect of implicit def SeqDerived. It is creating a divergent implicit situation where there was none before; why, and should we do something about it?
@odersky said: In fact the bug seems to be resolved now. I get none of the "is not an enclosing class" errors for any of the examples. Reassigning to Paul to close once the SeqDerived question is resolved.
@odersky said:
Paul showed me my previous comment was wrong. But it should be fixed for good in r24386. Test in pending/neg. The ticket is still open because SeqDerived
should be fixed.
@paulp said: (In r24419) Moved SeqDerived into an Ordering.Implicits object. Closes #3152 (only mopping up), no review.
This bug has been popping up regularly during Scalaz development for a few months, but I could never reproduce it in isolation.
It can usually be worked around by adding or removing type ascriptions, extracting sub-expressions to local vals, as demonstrated below.