Skip to content

Fix #10563: Handle static methods in non-native JS classes. #10564

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.

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

Merged
merged 1 commit into from
Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 4 additions & 5 deletions compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1122,8 +1122,9 @@ class JSCodeGen()(using genCtx: Context) {
* type is Unit, then the body is emitted as a statement. Otherwise, it is
* emitted as an expression.
*
* Methods Scala.js-defined JS classes are compiled as static methods taking
* an explicit parameter for their `this` value.
* Instance methods in non-native JS classes are compiled as static methods
* taking an explicit parameter for their `this` value. Static methods in
* non-native JS classes are compiled as is, like methods in Scala classes.
*/
private def genMethodDef(namespace: js.MemberNamespace, methodName: js.MethodIdent,
originalName: OriginalName, paramsSyms: List[Symbol], resultIRType: jstpe.Type,
Expand All @@ -1137,13 +1138,11 @@ class JSCodeGen()(using genCtx: Context) {
else genExpr(tree)
}

if (!currentClassSym.isNonNativeJSClass) {
if (namespace.isStatic || !currentClassSym.isNonNativeJSClass) {
val flags = js.MemberFlags.empty.withNamespace(namespace)
js.MethodDef(flags, methodName, originalName, jsParams, resultIRType, Some(genBody()))(
optimizerHints, None)
} else {
assert(!namespace.isStatic, tree.span)

val thisLocalIdent = freshLocalIdent("this")
withScopedVars(
thisLocalVarIdent := Some(thisLocalIdent)
Expand Down
9 changes: 5 additions & 4 deletions compiler/src/dotty/tools/backend/sjs/JSEncoding.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import NameOps._
import Names._
import StdNames._

import dotty.tools.dotc.transform.sjs.JSSymUtils._

import org.scalajs.ir
import org.scalajs.ir.{Trees => js, Types => jstpe}
import org.scalajs.ir.Names.{LocalName, LabelName, FieldName, SimpleMethodName, MethodName, ClassName}
Expand All @@ -28,7 +30,6 @@ import org.scalajs.ir.UTF8String

import ScopedVar.withScopedVars
import JSDefinitions._
import JSInterop._

import dotty.tools.backend.jvm.DottyBackendInterface.symExtensions

Expand Down Expand Up @@ -199,7 +200,7 @@ object JSEncoding {

val paramTypeRefs0 = tpe.firstParamTypes.map(paramOrResultTypeRef(_))

val hasExplicitThisParameter = isScalaJSDefinedJSClass(sym.owner)
val hasExplicitThisParameter = !sym.is(JavaStatic) && sym.owner.isNonNativeJSClass
val paramTypeRefs =
if (!hasExplicitThisParameter) paramTypeRefs0
else encodeClassRef(sym.owner) :: paramTypeRefs0
Expand Down Expand Up @@ -243,7 +244,7 @@ object JSEncoding {

def encodeClassType(sym: Symbol)(using Context): jstpe.Type = {
if (sym == defn.ObjectClass) jstpe.AnyType
else if (isJSType(sym)) jstpe.AnyType
else if (sym.isJSType) jstpe.AnyType
else {
assert(sym != defn.ArrayClass,
"encodeClassType() cannot be called with ArrayClass")
Expand Down Expand Up @@ -303,7 +304,7 @@ object JSEncoding {

case typeRef: jstpe.ClassRef =>
val sym = typeRefInternal._2
if (sym == defn.ObjectClass || isJSType(sym))
if (sym == defn.ObjectClass || sym.isJSType)
jstpe.AnyType
else if (sym == defn.NothingClass)
jstpe.NothingType
Expand Down
4 changes: 0 additions & 4 deletions compiler/src/dotty/tools/backend/sjs/JSInterop.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ object JSInterop {
def isJSType(sym: Symbol)(using Context): Boolean =
sym.isJSType

/** Is this symbol a Scala.js-defined JS class, i.e., a non-native JS class? */
def isScalaJSDefinedJSClass(sym: Symbol)(using Context): Boolean =
sym.isNonNativeJSClass

/** Should this symbol be translated into a JS getter?
*
* This is true for any parameterless method, i.e., defined without `()`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,24 @@ package org.scalajs.testsuite.compiler
import org.junit.Assert._
import org.junit.Test

import scala.scalajs.js

class RegressionTestScala3 {
import RegressionTestScala3._

@Test def testRegressionDoubleDefinitionOfOuterPointerIssue10177(): Unit = {
assertEquals(6, new OuterClassIssue10177().foo(5))
}

@Test def testRegressionJSClassWithSyntheticStaticMethodsIssue10563(): Unit = {
val obj1 = new JSClassWithSyntheticStaticMethodsIssue10563(Some(3))
assertEquals(3, obj1.y)
assertEquals(8, obj1.foo(5))

val obj2 = new JSClassWithSyntheticStaticMethodsIssue10563(None)
assertEquals(-1, obj2.y)
assertEquals(4, obj2.foo(5))
}
}

object RegressionTestScala3 {
Expand All @@ -22,3 +34,10 @@ object RegressionTestScala3 {
def foo(x: Int): Int = new ChildClass().concreteMethod(x)
}
}

// This class needs to be at the top-level, not in an object, to reproduce the issue
class JSClassWithSyntheticStaticMethodsIssue10563(x: Option[Int]) extends js.Object {
val y: Int = x.getOrElse(-1) // lambda in constructor

def foo(z: Int): Int = x.getOrElse(-1) + z // lambda in method
}