8000 Fix #3953: mistyping when using PartialFunction with HKT alias by errikos · Pull Request #3960 · scala-js/scala-js · GitHub
[go: up one dir, main page]
Skip to content

Fix #3953: mistyping when using PartialFunction with HKT alias #3960

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Fix #3953: mistyping when using PartialFunction with HKT alias #3960

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1843,7 +1843,22 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
}
}

Some(methodDefWithoutUselessVars)
/* See: https://github.com/scala-js/scala-js/issues/3953 */
val methodDefWithPatchedTypes = {
val symParamTypes =
if (sym.paramss.isEmpty) Nil
else sym.paramss.head.map(p => toIRType(p.tpe))
Comment on lines +1848 to +1850
Copy link
Member

Choose a reason for hiding this comment

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

I believe you can just use sym.tpe.params.map(p => toIRType(p.tpe)).


assert(symParamTypes.length == params.length,
"symParams and vparams have different lengths")

val patchedParamTypes = collection.mutable.Map(
(params.map(encodeLocalSym(_).name) zip symParamTypes): _*)
Copy link
Member

Choose a reason for hiding this comment

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

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!


patchTypesOfLocals(methodDefWithoutUselessVars, patchedParamTypes)
}

Some(methodDefWithPatchedTypes)
}
}
}
Expand Down Expand Up @@ -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]) = {
Copy link
Member

Choose a reason for hiding this comment

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

This should be an immutable Map. It's never mutated.

Copy link
Member

Choose a reason for hiding this comment

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

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.


val js.MethodDef(flags, methodName, originalName, params, resultType, body) =
methodDef

val transformer = new ir.Transformers.Transformer {
override def transform(tree: js.Tree, isStat: Boolean): js.Tree = tree match {
case js.VarRef(ident) =>
val patchedTpe = patches.getOrElse(ident.name, tree.tpe)
js.VarRef(ident)(patchedTpe)(tree.pos)
case js.Closure(arrow, captureParams, params, body, captureValues) =>
val patchedCaptureValues = captureValues.map(transformExpr(_))
js.Closure(arrow, captureParams, params, body, patchedCaptureValues)(tree.pos)
6D71 case _ =>
super.transform(tree, isStat)
}
}

val newParams = params.map {
case pd @ ir.Trees.ParamDef(name, originalName, ptpe, mutable, rest) =>
val patchedPtpe = patches.getOrElse(name.name, ptpe)
ir.Trees.ParamDef(name, originalName, patchedPtpe, mutable, rest)(pd.pos)
}

val newBody = body.map(transformer.transform(_, isStat = resultType == jstpe.NoType))

js.MethodDef(flags, methodName, originalName, newParams, resultType,
newBody)(methodDef.optimizerHints, None)(methodDef.pos)
}


/** Moves all statements after the super constructor call.
*
* This is used for the primary constructor of a non-native JS
Expand Down
0