Skip to content

Commit

Permalink
Merge pull request #9767 from dotty-staging/sjs-various-js-interop-fixes
Browse files Browse the repository at this point in the history
Scala.js: Various JS interop fixes
  • Loading branch information
nicolasstucki authored Sep 17, 2020
2 parents 89db0f8 + 9584666 commit 0c8c115
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 43 deletions.
108 changes: 81 additions & 27 deletions compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import Symbols._
import Denotations._
import Phases._
import StdNames._
import TypeErasure.ErasedValueType

import dotty.tools.dotc.transform.Erasure
import dotty.tools.dotc.transform.{Erasure, ValueClasses}
import dotty.tools.dotc.transform.SymUtils._
import dotty.tools.dotc.util.SourcePosition
import dotty.tools.dotc.util.Spans.Span
import dotty.tools.dotc.report
Expand Down Expand Up @@ -282,7 +284,7 @@ class JSCodeGen()(using genCtx: Context) {

// Generate members (constructor + methods)

val generatedMethods = new mutable.ListBuffer[js.MethodDef]
val generatedNonFieldMembers = new mutable.ListBuffer[js.MemberDef]
val exportedSymbols = new mutable.ListBuffer[Symbol]

val tpl = td.rhs.asInstanceOf[Template]
Expand All @@ -297,13 +299,11 @@ class JSCodeGen()(using genCtx: Context) {
val sym = dd.symbol

val isExport = false //jsInterop.isExport(sym)
val isNamedExport = false /*isExport && sym.annotations.exists(
_.symbol == JSExportNamedAnnotation)*/

/*if (isNamedExport)
generatedMethods += genNamedExporterDef(dd)
else*/
generatedMethods ++= genMethod(dd)
if (sym.hasAnnotation(jsdefn.JSNativeAnnot))
generatedNonFieldMembers += genJSNativeMemberDef(dd)
else
generatedNonFieldMembers ++= genMethod(dd)

if (isExport) {
// We add symbols that we have to export here. This way we also
Expand All @@ -317,7 +317,7 @@ class JSCodeGen()(using genCtx: Context) {
}

// Generate fields and add to methods + ctors
val generatedMembers = genClassFields(td) ++ generatedMethods.toList
val generatedMembers = genClassFields(td) ++ generatedNonFieldMembers.toList

// Generate the exported members, constructors and accessors
val exports = {
Expand Down Expand Up @@ -529,7 +529,6 @@ class JSCodeGen()(using genCtx: Context) {

private def genClassInterfaces(sym: ClassSymbol)(
implicit pos: Position): List[js.ClassIdent] = {
import dotty.tools.dotc.transform.SymUtils._
for {
intf <- sym.directlyInheritedTraits
} yield {
Expand Down Expand Up @@ -677,7 +676,10 @@ class JSCodeGen()(using genCtx: Context) {
"genClassFields called with a ClassDef other than the current one")

// Term members that are neither methods nor modules are fields
classSym.info.decls.filter(f => !f.isOneOf(Method | Module) && f.isTerm).map({ f =>
classSym.info.decls.filter { f =>
!f.isOneOf(Method | Module) && f.isTerm
&& !f.hasAnnotation(jsdefn.JSNativeAnnot)
}.map({ f =>
implicit val pos = f.span

val name =
Expand Down Expand Up @@ -815,6 +817,17 @@ class JSCodeGen()(using genCtx: Context) {

// Generate a method -------------------------------------------------------

/** Generates the JSNativeMemberDef. */
def genJSNativeMemberDef(tree: DefDef): js.JSNativeMemberDef = {
implicit val pos = tree.span

val sym = tree.symbol
val flags = js.MemberFlags.empty.withNamespace(js.MemberNamespace.PublicStatic)
val methodName = encodeMethodSym(sym)
val jsNativeLoadSpec = computeJSNativeLoadSpecOfValDef(sym)
js.JSNativeMemberDef(flags, methodName, jsNativeLoadSpec)
}

private def genMethod(dd: DefDef): Option[js.MethodDef] = {
withScopedVars(
localNames := new LocalNameGenerator
Expand Down Expand Up @@ -1227,7 +1240,7 @@ class JSCodeGen()(using genCtx: Context) {
val sym = lhs0.symbol
if (sym.is(JavaStaticTerm))
throw new FatalError(s"Assignment to static member ${sym.fullName} not supported")
val genRhs = genExpr(rhs)
def genRhs = genExpr(rhs)
val lhs = lhs0 match {
case lhs: Ident => desugarIdent(lhs).getOrElse(lhs)
case lhs => lhs
Expand All @@ -1247,7 +1260,15 @@ class JSCodeGen()(using genCtx: Context) {

val genQual = genExpr(qualifier)

/*if (isScalaJSDefinedJSClass(sym.owner)) {
if (sym.hasAnnotation(jsdefn.JSNativeAnnot)) {
/* This is an assignment to a @js.native field. Since we reject
* `@js.native var`s as compile errors, this can only happen in
* the constructor of the enclosing object.
* We simply ignore the assignment, since the field will not be
* emitted at all.
*/
js.Skip()
} else /*if (isScalaJSDefinedJSClass(sym.owner)) {
val genLhs = if (isExposed(sym))
js.JSBracketSelect(genQual, js.StringLiteral(jsNameOf(sym)))
else
Expand All @@ -1256,12 +1277,12 @@ class JSCodeGen()(using genCtx: Context) {
ensureBoxed(genRhs,
enteringPhase(currentRun.posterasurePhase)(rhs.tpe))
js.Assign(genLhs, boxedRhs)
} else {*/
} else*/ {
js.Assign(
js.Select(genQual, encodeClassName(sym.owner),
encodeFieldSym(sym))(toIRType(sym.info)),
genRhs)
//}
}
case _ =>
js.Assign(
js.VarRef(encodeLocalSym(sym))(toIRType(sym.info)),
Expand Down Expand Up @@ -2119,6 +2140,8 @@ class JSCodeGen()(using genCtx: Context) {
genApplyJSMethodGeneric(sym, genExprOrGlobalScope(receiver), genActualJSArgs(sym, args), isStat)(tree.sourcePos)
/*else
genApplyJSClassMethod(genExpr(receiver), sym, genActualArgs(sym, args))*/
} else if (sym.hasAnnotation(jsdefn.JSNativeAnnot)) {
genJSNativeMemberCall(tree, isStat)
} else {
genApplyMethodMaybeStatically(genExpr(receiver), sym, genActualArgs(sym, args))
}
Expand Down Expand Up @@ -2286,6 +2309,26 @@ class JSCodeGen()(using genCtx: Context) {
(firstArg.asInstanceOf[js.Tree], args.tail)
}

/** Gen JS code for a call to a native JS def or val. */
private def genJSNativeMemberCall(tree: Apply, isStat: Boolean): js.Tree = {
val sym = tree.symbol
val Apply(_, args) = tree

implicit val pos = tree.span

val jsNativeMemberValue =
js.SelectJSNativeMember(encodeClassName(sym.owner), encodeMethodSym(sym))

val boxedResult =
if (sym.isJSGetter) jsNativeMemberValue
else js.JSFunctionApply(jsNativeMemberValue, genActualJSArgs(sym, args))

unbox(boxedResult, atPhase(elimErasedValueTypePhase) {
sym.info.resultType
})
}


/** Gen JS code for a call to a polymorphic method.
*
* The only methods that reach the back-end as polymorphic are
Expand Down Expand Up @@ -2599,10 +2642,10 @@ class JSCodeGen()(using genCtx: Context) {
case tpe if isPrimitiveValueType(tpe) =>
makePrimitiveBox(expr, tpe)

/*case tpe: ErasedValueType =>
val boxedClass = tpe.valueClazz
case tpe: ErasedValueType =>
val boxedClass = tpe.tycon.typeSymbol
val ctor = boxedClass.primaryConstructor
genNew(boxedClass, ctor, List(expr))*/
js.New(encodeClassName(boxedClass), encodeMethodSym(ctor), List(expr))

case _ =>
expr
Expand All @@ -2626,15 +2669,15 @@ class JSCodeGen()(using genCtx: Context) {
case tpe if isPrimitiveValueType(tpe) =>
makePrimitiveUnbox(expr, tpe)

/*case tpe: ErasedValueType =>
val boxedClass = tpe.valueClazz
val unboxMethod = boxedClass.derivedValueClassUnbox
case tpe: ErasedValueType =>
val boxedClass = tpe.tycon.typeSymbol.asClass
val unboxMethod = ValueClasses.valueClassUnbox(boxedClass)
val content = genApplyMethod(
genAsInstanceOf(expr, tpe), unboxMethod, Nil)
if (unboxMethod.tpe.resultType <:< tpe.erasedUnderlying)
js.AsInstanceOf(expr, encodeClassType(boxedClass)), unboxMethod, Nil)
if (unboxMethod.info.resultType <:< tpe.erasedUnderlying)
content
else
fromAny(content, tpe.erasedUnderlying)*/
unbox(content, tpe.erasedUnderlying)

case tpe =>
genAsInstanceOf(expr, tpe)
Expand Down Expand Up @@ -2889,7 +2932,11 @@ class JSCodeGen()(using genCtx: Context) {
case TYPEOF =>
// js.typeOf(arg)
val arg = genArgs1
genAsInstanceOf(js.JSUnaryOp(js.JSUnaryOp.typeof, arg), defn.StringType)
val typeofExpr = arg match {
case arg: js.JSGlobalRef => js.JSTypeOfGlobalRef(arg)
case _ => js.JSUnaryOp(js.JSUnaryOp.typeof, arg)
}
js.AsInstanceOf(typeofExpr, jstpe.ClassType(jsNames.BoxedStringClass))

case STRICT_EQ =>
// js.special.strictEquals(arg1, arg2)
Expand Down Expand Up @@ -3497,6 +3544,12 @@ class JSCodeGen()(using genCtx: Context) {
}
}

private def computeJSNativeLoadSpecOfValDef(sym: Symbol): js.JSNativeLoadSpec = {
atPhase(picklerPhase.next) {
computeJSNativeLoadSpecOfInPhase(sym)
}
}

private def computeJSNativeLoadSpecOfClass(sym: Symbol): Option[js.JSNativeLoadSpec] = {
if (sym.is(Trait) || sym.hasAnnotation(jsdefn.JSGlobalScopeAnnot)) {
None
Expand Down Expand Up @@ -3587,8 +3640,9 @@ class JSCodeGen()(using genCtx: Context) {

private def isPrimitiveValueType(tpe: Type): Boolean = {
tpe.widenDealias match {
case JavaArrayType(_) => false
case t => t.typeSymbol.asClass.isPrimitiveValueClass
case JavaArrayType(_) => false
case _: ErasedValueType => false
case t => t.typeSymbol.asClass.isPrimitiveValueClass
}
}

Expand Down
11 changes: 4 additions & 7 deletions compiler/src/dotty/tools/dotc/transform/Memoize.scala
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,10 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
case _ => ()
}

def removeUnwantedAnnotations(denot: SymDenotation): Unit =
def removeUnwantedAnnotations(denot: SymDenotation, metaAnnotSym: ClassSymbol): Unit =
if (sym.annotations.nonEmpty) {
val cpy = sym.copySymDenotation()
// Keep @deprecated annotation so that accessors can
// be marked as deprecated in the bytecode.
// TODO check the meta-annotations to know what to keep
cpy.filterAnnotations(_.matches(defn.DeprecatedAnnot))
cpy.filterAnnotations(_.symbol.hasAnnotation(metaAnnotSym))
cpy.installAfter(thisPhase)
}

Expand Down Expand Up @@ -141,7 +138,7 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
else transformFollowingDeep(ref(field))(using ctx.withOwner(sym))
val getterDef = cpy.DefDef(tree)(rhs = getterRhs)
addAnnotations(fieldDef.denot)
removeUnwantedAnnotations(sym)
removeUnwantedAnnotations(sym, defn.GetterMetaAnnot)
Thicket(fieldDef, getterDef)
else if sym.isSetter then
if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // This is intended as an assertion
Expand All @@ -162,7 +159,7 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
if (isErasableBottomField(field, tree.vparamss.head.head.tpt.tpe.classSymbol)) Literal(Constant(()))
else Assign(ref(field), adaptToField(field, ref(tree.vparamss.head.head.symbol)))
val setterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(using ctx.withOwner(sym)))
removeUnwantedAnnotations(sym)
removeUnwantedAnnotations(sym, defn.SetterMetaAnnot)
setterDef
else
// Curiously, some accessors from Scala2 have ' ' suffixes.
Expand Down
49 changes: 43 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/SelectStatic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,37 @@ import dotty.tools.dotc.core._
import dotty.tools.dotc.transform.MegaPhase._
import dotty.tools.dotc.transform.SymUtils._

/** Removes selects that would be compiled into GetStatic
* otherwise backend needs to be aware that some qualifiers need to be dropped.
* Similar transformation seems to be performed by flatten in nsc
* @author Dmytro Petrashko
/** Removes `Select`s that would be compiled into `GetStatic`.
*
* Otherwise, the backend needs to be aware that some qualifiers need to be
* dropped.
*
* A tranformation similar to what this phase does seems to be performed by
* flatten in nsc.
*
* The side effects of the qualifier of a dropped `Select` is normally
* retained. As an exception, the qualifier is completely dropped if it is
* a reference to a static owner (see `isStaticOwnerRef`). Concretely, this
* means that in
*
* {{{
* object Foo {
* println("side effects")
*
* object Bar
* class Baz
* }
*
* Foo.Bar
* new Foo.Baz()
* }}}
*
* the `Foo` qualifiers will be dropped, since it is a static object. The
* `println("side effects")` will therefore not be executed.
*
* This intended behavior is equivalent to what scalac does.
*
* @author Dmytro Petrashko
*/
class SelectStatic extends MiniPhase with IdentityDenotTransformer {
import ast.tpd._
Expand All @@ -29,13 +56,23 @@ class SelectStatic extends MiniPhase with IdentityDenotTransformer {
sym.isScalaStatic
val isStaticRef = !sym.is(Package) && !sym.maybeOwner.is(Package) && isStaticMember
val tree1 =
if (isStaticRef && !tree.qualifier.symbol.isAllOf(JavaModule) && !tree.qualifier.isType)
Block(List(tree.qualifier), ref(sym))
if isStaticRef && !tree.qualifier.symbol.isAllOf(JavaModule) && !tree.qualifier.isType then
if isStaticOwnerRef(tree.qualifier) then ref(sym)
else Block(List(tree.qualifier), ref(sym))
else tree

normalize(tree1)
}

private def isStaticOwnerRef(tree: Tree)(using Context): Boolean = tree match {
case Ident(_) =>
tree.symbol.is(Module) && tree.symbol.moduleClass.isStaticOwner
case Select(qual, _) =>
isStaticOwnerRef(qual) && tree.symbol.is(Module) && tree.symbol.moduleClass.isStaticOwner
case _ =>
false
}

private def normalize(t: Tree)(using Context) = t match {
case Select(Block(stats, qual), nm) =>
Block(stats, cpy.Select(t)(qual, nm))
Expand Down
9 changes: 7 additions & 2 deletions project/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,12 @@ object Build {
scalaJSLinkerConfig ~= { _.withSemantics(build.TestSuiteLinkerOptions.semantics _) },
scalaJSModuleInitializers in Test ++= build.TestSuiteLinkerOptions.moduleInitializers,

jsEnvInput in Test := {
val resourceDir = fetchScalaJSSource.value / "test-suite/js/src/test/resources"
val f = (resourceDir / "NonNativeJSTypeTestNatives.js").toPath
org.scalajs.jsenv.Input.Script(f) +: (jsEnvInput in Test).value
},

managedSources in Compile ++= {
val dir = fetchScalaJSSource.value / "test-suite/js/src/main/scala"
val filter = (
Expand All @@ -1032,7 +1038,7 @@ object Build {
++ (dir / "shared/src/test/require-jdk7" ** "*.scala").get

++ (dir / "js/src/test/scala/org/scalajs/testsuite/compiler" ** (("*.scala": FileFilter)
-- "InteroperabilityTest.scala" // compiler crash, related to value classes in JS interop
-- "InteroperabilityTest.scala" // nested native JS classes + JS exports
-- "OptimizerTest.scala" // non-native JS classes
-- "RegressionJSTest.scala" // non-native JS classes
-- "RuntimeTypesTest.scala" // compile errors: no ClassTag for Null and Nothing
Expand All @@ -1049,7 +1055,6 @@ object Build {
-- "ExportsTest.scala" // JS exports
-- "IterableTest.scala" // non-native JS classes
-- "JSExportStaticTest.scala" // JS exports
-- "JSNativeInPackage.scala" // #9785 tests fail due to js.typeOf(globalVar) being incorrect
-- "JSOptionalTest.scala" // non-native JS classes
-- "JSSymbolTest.scala" // non-native JS classes
-- "MiscInteropTest.scala" // non-native JS classes
Expand Down
11 changes: 11 additions & 0 deletions tests/run/no-init-enclosing-static-objects.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
InnerObject constructor
42
InnerClass constructor
5
42
InnerClass constructor
5
getEnclosing2()
Enclosing2 constructor
Enclosing2.InnerObject constructor
65
Loading

0 comments on commit 0c8c115

Please sign in to comment.