Adapt type parameters of typed eta expansion according to expected variances #950
Conversation
|
/rebuild |
|
/rebuild |
|
https://scala-ci.typesafe.com/job/dotty-master-validate-partest/670/consoleFull the same test failed 3 times with same error. Does not look like random failure |
| case _ => NoSymbol | ||
| } | ||
| def adaptArg(arg: Type): Type = arg match { | ||
| case arg: TypeRef if arg.symbol.isLambdaTrait => |
There was a problem hiding this comment.
Unrelated to this change, but the definition of isLambdaTrait:
final def isLambdaTrait(implicit ctx: Context): Boolean =
isClass && name.startsWith(tpnme.LambdaPrefix)
Looks prone to falsely matching:
scala> class `Lambda|`
defined class Lambda$bar
There was a problem hiding this comment.
Right, we should restrict this to members of the scala package.
|
/rebuild |
3 similar comments
|
/rebuild |
|
/rebuild |
|
/rebuild |
|
Heisenbug hates me. Or at least is very good in hiding... |
|
@odersky Do you believe this bug is exposed by parallel test execution sharing a symbol table? Or is it a single-threaded non-determinism? Assuming the former: What sort of operations are supported concurrently? I've reviewed a little of the caching code. This is prone to race condition: private def memberCache(implicit ctx: Context): LRUCache[Name, PreDenotation] = {
if (myMemberCachePeriod != ctx.period) {
myMemberCache = new LRUCache
myMemberCachePeriod = ctx.period
}
myMemberCache
}Same story for def enter(key: Key, value: Value): Unit = {
keys(last) = key
values(last) = value
last = lastButOne
} |
|
The latest additions to diagnostics have finally produced something One other suggestion: The failures we are seeing happen when running the [info] *** missing method: On Fri, Nov 13, 2015 at 8:32 AM, Dmitry Petrashko [email protected]
Martin Odersky |
|
What is shared? The name table?
|
|
Yes, nametable is the only data structure that is shared. |
|
/rebuild |
1 similar comment
|
/rebuild |
When eta expanding a type `C` to `[vX] => C[X]` the variance `v` is now the variance of the expected type, not the variance of the type constructor `C`, as long as the variance of the expected type is compatible with the variance of `C`.
Exclude false positives such as `Lambda|` be requiring that lambda traits are defined in the Scala package.
|
rebased to master |
|
Wow. Is the Heisenbug slayed, finally? |
getSimpleName crashes on some module names created by scalac. May help finding the partest issue. (reverted from commit c11646c)
Adding parents signals (via SymDenotation.fullyDefined) that the class can now be frozen. So this should be done only after all members are entered.
Fix problems arising when hierarchies of classes are under completion at the same time. In this case it can happen that we see a subclass (e.g. Arrays.scala) which depends on a superclass (e.g. GenTraversableLike.scala) that itself does not have its parents defined yet. Previously, several things went wrong here - One of the base classes would be marked as frozen, even though it dod not have all members entered yet. This led to an error in finger printing. - The baseclasses and super class bits of the subclass would be computed before the parents of the middle class were known. The baseclasses would then be chached, leading to false results for isDerivedFrom. We need to refine the logic for computing base classes, super class bits, and fingerprints to account for that issue.
The previous commit made packages always fully completed. This is wrong - since new members can be added to packages at any time, packages are never fully completed.
Just noted that the previous scope might have been too small. We compute the bucket index with the table size before going into the synchronized. But that might mean we see a stale table size. Let's see what this gives us.
|
Removed all the added Heisenbug scaffolding for requiredMethod and force pushed. |
|
Two successes in a row! I am taking the liberty to merge so that others can profit. |
Adapt type parameters of typed eta expansion according to expected variances
Fixes #916. Review by @retronym?