Fix #3953: mistyping when using PartialFunction with HKT alias #3960
Conversation
The issue occurs due to a bug in scalac, where Defdef vparamss and the respective symbol have inconsistent types. For more information, please see: scala/bug#11884
|
You should definitely add the reproduction from the issue as a test case. ;) |
|
Have we assessed the feasibility of fixing this in scalac proper? I'm not a fan of simply postprocessing practically everything we generate. If this is really necessary, could we at least investigate a deeper integration that correctly translates an NB to reply to @errikos from #3960
IMHO this is the wrong way to look at it. We should not code in a way that it easy, but in a way that yields the easiest to understand and maintain result. The difficulty of the code and the review are secondary. A good end result is the primary goal. From the little I can see, we can create a |
Thanks for the feedback! I totally agree that this is far from the ideal way. That's why I said "easiest" instead of "better". But, it is the simplest one, since it only requires patching at a single code position and this is what I was going after, given that my familiarity with the Scala.js codebase is minimal.
I will look into this and see if it leads into a more elegant solution. As @sjrd said, ideally we would like that the bug is fixed in scalac. |
| val symParamTypes = | ||
| if (sym.paramss.isEmpty) Nil | ||
| else sym.paramss.head.map(p => toIRType(p.tpe)) |
There was a problem hiding this comment.
I believe you can just use sym.tpe.params.map(p => toIRType(p.tpe)).
| @@ -1906,6 +1921,38 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G) | |||
| newBody)(methodDef.optimizerHints, None)(methodDef.pos) | |||
| } | |||
|
|
|||
| private def patchTypesOfLocals(methodDef: js.MethodDef, | |||
| patches: mutable.Map[LocalName, ir.Types.Type]) = { | |||
There was a problem hiding this comment.
This should be an immutable Map. It's never mutated.
There was a problem hiding this comment.
Also, it's usually better to explicitly specify the result type of a function that is a bit long, even when it is private. It helps the reader.
| "symParams and vparams have different lengths") | ||
|
|
||
| val patchedParamTypes = collection.mutable.Map( | ||
| (params.map(encodeLocalSym(_).name) zip symParamTypes): _*) |
There was a problem hiding this comment.
You can use methodDefWithoutUselessVars.args instead of params here. That's better because we already have their LocalName, hence we don't need to re-encode the local syms.
Moreover, and more importantly, we should filter out the parameters for which the IR type is already correct (i.e., the (arg: js.ParamDef).ptpe == symParamType). If the resulting list is empty, which is the overwhelmingly common case, we should not call patchTypesOfLocals at all!
|
Superseded by #4533. |
The issue occurs due to a bug in scalac, where Defdef vparamss and the
respective symbol have inconsistent types. For more information, please
see: scala/bug#11884
This commit is a workaround for the aforementioned issue, until the latter
is fixed in the scala compiler.
It consists of a very dirty hack, where, after the IR Tree generation, we go
and patch the types of the locals to correspond to the types found in the
Defdef's symbol (which are the correct ones).