8000 Merge pull request #4533 from sjrd/fix-param-types · renowncoder/scala-js@1874820 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1874820

Browse files
sjrdsjrd
authored
Merge pull request scala-js#4533 from sjrd/fix-param-types
Fix scala-js#3953: Patch the types of ParamDefs to match their method signature.
2 parents 9f70122 + 9ff67d5 commit 1874820

File tree

2 files changed

+122
-42
lines changed

2 files changed

+122
-42
lines changed

‎compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala

Lines changed: 101 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1747,9 +1747,37 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
17471747

17481748
// Generate a method -------------------------------------------------------
17491749

1750+
/** Maybe gen JS code for a method definition.
1751+
*
1752+
* Some methods are not emitted at all:
1753+
*
1754+
* - Primitives, since they are never actually called (with exceptions)
1755+
* - Abstract methods in non-native JS classes
1756+
* - Default accessor of a native JS constructor
1757+
* - Constructors of hijacked classes
1758+
* - Methods with the {{{@JavaDefaultMethod}}} annotation mixed in classes.
1759+
*/
17501760
def genMethod(dd: DefDef): Option[js.MethodDef] = {
1751-
withNewLocalNameScope {
1752-
genMethodWithCurrentLocalNameScope(dd)
1761+
val sym = dd.symbol
1762+
val isAbstract = isAbstractMethod(dd)
1763+
1764+
if (scalaPrimitives.isPrimitive(sym)) {
1765+
None
1766+
} else if (isAbstract && isNonNativeJSClass(currentClassSym)) {
1767+
// #4409: Do not emit abstract methods in non-native JS classes
1768+
None
1769+
} else if (isJSNativeCtorDefaultParam(sym)) {
1770+
None
1771+
} else if (sym.isClassConstructor && isHijackedClass(sym.owner)) {
1772+
None
1773+
} else if (scalaUsesImplClasses && !isImplClass(sym.owner) &&
1774+
!isAbstract && sym.hasAnnotation(JavaDefaultMethodAnnotation)) {
1775+
// Do not emit trait impl forwarders with @JavaDefaultMethod
1776+
None
1777+
} else {
1778+
withNewLocalNameScope {
1779+
Some(genMethodWithCurrentLocalNameScope(dd))
1780+
}
17531781
}
17541782
}
17551783

@@ -1758,42 +1786,30 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
17581786
* of the Scala method, as described in `JSEncoding`, to support
17591787
* overloading.
17601788
*
1761-
* Some methods are not emitted at all:
1762-
* * Primitives, since they are never actually called (with exceptions)
1763-
* * Abstract methods
1764-
* * Constructors of hijacked classes
1765-
* * Methods with the {{{@JavaDefaultMethod}}} annotation mixed in classes.
1766-
*
17671789
* Constructors are emitted by generating their body as a statement.
17681790
*
17691791
* Interface methods with the {{{@JavaDefaultMethod}}} annotation produce
17701792
* default methods forwarding to the trait impl class method.
17711793
*
17721794
* Other (normal) methods are emitted with `genMethodDef()`.
17731795
*/
1774-
def genMethodWithCurrentLocalNameScope(dd: DefDef): Option[js.MethodDef] = {
1796+
def genMethodWithCurrentLocalNameScope(dd: DefDef): js.MethodDef = {
17751797
implicit val pos = dd.pos
1776-
val DefDef(mods, name, _, vparamss, _, rhs) = dd
17771798
val sym = dd.symbol
17781799

17791800
withPerMethodBodyState(sym) {
1780-
assert(vparamss.isEmpty || vparamss.tail.isEmpty,
1781-
"Malformed parameter list: " + vparamss)
1782-
val params = if (vparamss.isEmpty) Nil else vparamss.head map (_.symbol)
1783-
17841801
val methodName = encodeMethodSym(sym)
17851802
val originalName = originalNameOfMethod(sym)
17861803

1787-
val isAbstract = isAbstractMethod(dd)
1788-
1789-
def jsParams = params.map(genParamDef(_))
1804+
val jsParams = {
1805+
val vparamss = dd.vparamss
1806+
assert(vparamss.isEmpty || vparamss.tail.isEmpty,
1807+
"Malformed parameter list: " + vparamss)
1808+
val params = if (vparamss.isEmpty) Nil else vparamss.head.map(_.symbol)
1809+
params.map(genParamDef(_))
1810+
}
17901811

1791-
if (scalaPrimitives.isPrimitive(sym)) {
1792-
None
1793-
} else if (isAbstract && isNonNativeJSClass(currentClassSym)) {
1794-
// #4409: Do not emit abstract methods in non-native JS classes
1795-
None
1796-
} else if (isAbstract) {
1812+
val jsMethodDef = if (isAbstractMethod(dd)) {
17971813
val body = if (scalaUsesImplClasses &&
17981814
sym.hasAnnotation(JavaDefaultMethodAnnotation)) {
17991815
/* For an interface method with @JavaDefaultMethod, make it a
@@ -1814,17 +1830,9 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
18141830
} else {
18151831
None
18161832
}
1817-
Some(js.MethodDef(js.MemberFlags.empty, methodName, originalName,
1833+
js.MethodDef(js.MemberFlags.empty, methodName, originalName,
18181834
jsParams, toIRType(sym.tpe.resultType), body)(
1819-
OptimizerHints.empty, None))
1820-
} else if (isJSNativeCtorDefaultParam(sym)) {
1821-
None
1822-
} else if (sym.isClassConstructor && isHijackedClass(sym.owner)) {
1823-
None
1824-
} else if (scalaUsesImplClasses && !isImplClass(sym.owner) &&
1825-
sym.hasAnnotation(JavaDefaultMethodAnnotation)) {
1826-
// Do not emit trait impl forwarders with @JavaDefaultMethod
1827-
None
1835+
OptimizerHints.empty, None)
18281836
} else {
18291837
def isTraitImplForwarder = dd.rhs match {
18301838
case app: Apply => isImplClass(app.symbol.owner)
@@ -1853,7 +1861,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
18531861
val namespace = js.MemberNamespace.Constructor
18541862
js.MethodDef(
18551863
js.MemberFlags.empty.withNamespace(namespace), methodName,
1856-
originalName, jsParams, jstpe.NoType, Some(genStat(rhs)))(
1864+
originalName, jsParams, jstpe.NoType, Some(genStat(dd.rhs)))(
18571865
optimizerHints, None)
18581866
} else {
18591867
val resultIRType = toIRType(sym.tpe.resultType)
@@ -1866,8 +1874,8 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
18661874
else js.MemberNamespace.Public
18671875
}
18681876
}
1869-
genMethodDef(namespace, methodName, originalName, params,
1870-
resultIRType, rhs, optimizerHints)
1877+
genMethodDef(namespace, methodName, originalName, jsParams,
1878+
resultIRType, dd.rhs, optimizerHints)
18711879
}
18721880
}
18731881

@@ -1889,7 +1897,25 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
18891897
}
18901898
}
18911899

1892-
Some(methodDefWithoutUselessVars)
1900+
methodDefWithoutUselessVars
1901+
}
1902+
1903+
/* #3953 Patch the param defs to have the type advertised by the method's type.
1904+
* This works around https://github.com/scala/bug/issues/11884, whose fix
1905+
* upstream is blocked because it is not binary compatible. The fix here
1906+
* only affects the inside of the js.MethodDef, so it is binary compat.
1907+
*/
1908+
val paramTypeRewrites = jsParams.zip(sym.tpe.paramTypes.map(toIRType(_))).collect {
1909+
case (js.ParamDef(name, _, tpe, _), sigType) if tpe != sigType => name.name -> sigType
1910+
}
1911+
if (paramTypeRewrites.isEmpty) {
1912+
// Overwhelmingly common case: all the types match, so there is nothing to do
1913+
jsMethodDef
1914+
} else {
1915+
devWarning(
1916+
"Working around https://github.com/scala/bug/issues/11884 " +
1917+
s"for ${sym.fullName} at ${sym.pos}")
1918+
patchTypeOfParamDefs(jsMethodDef, paramTypeRewrites.toMap)
18931919
}
18941920
}
18951921
}
@@ -1950,6 +1976,42 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
19501976
newBody)(methodDef.optimizerHints, None)(methodDef.pos)
19511977
}
19521978

1979+
/** Patches the type of selected param defs in a [[js.MethodDef]].
1980+
*
1981+
* @param patches
1982+
* Map from local name to new type. For param defs not in the map, the
1983+
* type is untouched.
1984+
*/
1985+
private def patchTypeOfParamDefs(methodDef: js.MethodDef,
1986+
patches: Map[LocalName, jstpe.Type]): js.MethodDef = {
1987+
1988+
def newType(name: js.LocalIdent, oldType: jstpe.Type): jstpe.Type =
1989+
patches.getOrElse(name.name, oldType)
1990+
1991+
val js.MethodDef(flags, methodName, originalName, params, resultType, body) =
1992+
methodDef
1993+
val newParams = for {
1994+
p @ js.ParamDef(name, originalName, ptpe, mutable) <- params
1995+
} yield {
1996+
js.ParamDef(name, originalName, newType(name, ptpe), mutable)(p.pos)
1997+
}
1998+
val transformer = new ir.Transformers.Transformer {
1999+
override def transform(tree: js.Tree, isStat: Boolean): js.Tree = tree match {
2000+
case tree @ js.VarRef(name) =>
2001+
js.VarRef(name)(newType(name, tree.tpe))(tree.pos)
2002+
case js.Closure(arrow, captureParams, params, restParam, body, captureValues) =>
2003+
js.Closure(arrow, captureParams, params, restParam, body,
2004+
captureValues.map(transformExpr))(tree.pos)
2005+
case _ =>
2006+
super.transform(tree, isStat)
2007+
}
2008+
}
2009+
val newBody = body.map(
2010+
b => transformer.transform(b, isStat = resultType == jstpe.NoType))
2011+
js.MethodDef(flags, methodName, originalName, newParams, resultType,
2012+
newBody)(methodDef.optimizerHints, None)(methodDef.pos)
2013+
}
2014+
19532015
/** Generates the JSNativeMemberDef of a JS native method. */
19542016
def genJSNativeMemberDef(tree: DefDef): js.JSNativeMemberDef = {
19552017
implicit val pos = tree.pos
@@ -1972,13 +2034,11 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
19722034
* `this`.
19732035
*/
19742036
def genMethodDef(namespace: js.MemberNamespace, methodName: js.MethodIdent,
1975-
originalName: OriginalName, paramsSyms: List[Symbol],
2037+
originalName: OriginalName, jsParams: List[js.ParamDef],
19762038
resultIRType: jstpe.Type, tree: Tree,
19772039
optimizerHints: OptimizerHints): js.MethodDef = {
19782040
implicit val pos = tree.pos
19792041

1980-
val jsParams = paramsSyms.map(genParamDef(_))
1981-
19822042
val bodyIsStat = resultIRType == jstpe.NoType
19832043

19842044
def genBodyWithinReturnableScope(): js.Tree = tree match {
@@ -5823,8 +5883,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
58235883
tryingToGenMethodAsJSFunction := true
58245884
) {
58255885
try {
5826-
genMethodWithCurrentLocalNameScope(applyDef).getOrElse(
5827-
abort(s"Oops, $applyDef did not produce a method"))
5886+
genMethodWithCurrentLocalNameScope(applyDef)
58285887
} catch {
58295888
case e: CancelGenMethodAsJSFunction =>
58305889
fail(e.getMessage)

‎test-suite/shared/src/test/scala/org/scalajs/testsuite/compiler/RegressionTest.scala

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,27 @@ class RegressionTest {
870870
assertEquals("lazily always", ex2.getMessage())
871871
}
872872

873+
@Test def paramDefWithWrongTypeWithHKTAndTypeAliases_Issue3953(): Unit = {
874+
assumeFalse("Scala/JVM 2.11.x produces wrong bytecode for this test",
875+
Platform.executingInJVM && Platform.scalaVersion.startsWith("2.11."))
876+
877+
import scala.language.higherKinds
878+
879+
sealed class StreamT[M[_]](val step: M[Step[StreamT[M]]])
880+
881+
sealed abstract class Step[S]
882+
883+
def mustMatch[A](actual: A)(f: PartialFunction[A, Boolean]): Boolean =
884+
f.applyOrElse(actual, (_: Any) => false)
885+
886+
type Id[A] = A
887+
888+
val result = mustMatch(new StreamT[Id](null).step) {
889+
case _ => true
890+
}
891+
assertTrue(result)
892+
}
893+
873894
}
874895

875896
object RegressionTest {

0 commit comments

Comments
 (0)
0