Skip to content

Commit ffbd08c

Browse files
committed
Address my self-review.
1 parent 03b5324 commit ffbd08c

File tree

5 files changed

+18
-21
lines changed

5 files changed

+18
-21
lines changed

compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -789,13 +789,11 @@ class JSCodeGen()(using genCtx: Context) {
789789
* initialized to an instance of the boxed representation, with
790790
* an underlying value set to the zero of its type. However we
791791
* cannot implement that, so we live with the discrepancy.
792-
* Anyway, scalac also has problems with uninitialized value
793-
* class values, if they come from a generic context.
794792
*
795-
* TODO Evaluate how much of this needs to be adapted for dotc,
796-
* which unboxes `null` to the zero of their underlying.
793+
* In dotc this is usually not an issue, because it unboxes `null` to
794+
* the zero of the underlying type, unlike scalac which throws an NPE.
797795
*/
798-
jstpe.ClassType(encodeClassName(tpe.valueClassSymbol))
796+
jstpe.ClassType(encodeClassName(tpe.tycon.typeSymbol))
799797

800798
case _ =>
801799
// Other types are not boxed, so we can initialized them to their true zero.

compiler/src/dotty/tools/backend/sjs/JSExportsGen.scala

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
652652
val isLiftedJSConstructor =
653653
sym.isClassConstructor && sym.owner.isNestedJSClass
654654

655+
// params: List[ParamSpec] ; captureParams and captureParamsBack: List[js.ParamDef]
655656
val (params, captureParamsFront, captureParamsBack) = {
656657
val paramNamessNow = sym.info.paramNamess
657658
val paramInfosNow = sym.info.paramInfoss.flatten
@@ -684,9 +685,10 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
684685
* by explicitouter, while the other capture params are added by
685686
* lambdalift (between elimErasedValueType and now).
686687
*
687-
* scalac: Note that lambdalift creates new symbols even for parameters that
688-
* are not the result of lambda lifting, but it preserves their
689-
* `name`s.
688+
* Since we cannot reliably get Symbols for parameters created by
689+
* intermediate phases, we have to perform some dance with the
690+
* paramNamess and paramInfoss observed at some phases, combined with
691+
* the paramSymss observed at elimRepeated.
690692
*/
691693

692694
val hasOuterParam = {
@@ -724,7 +726,8 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
724726
// For an anonymous JS class constructor, we put the capture parameters back as formal parameters.
725727
val allFormalParams = captureParamsFront.toIndexedSeq ++ formalParams ++ captureParamsBack.toIndexedSeq
726728
(allFormalParams, Nil, Nil)
727-
} else {*/ (formalParams, captureParamsFront, captureParamsBack)
729+
} else {*/
730+
(formalParams, captureParamsFront, captureParamsBack)
728731
//}
729732
}
730733
}

compiler/src/dotty/tools/dotc/core/TypeErasure.scala

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,6 @@ object TypeErasure {
9595
abstract case class ErasedValueType(tycon: TypeRef, erasedUnderlying: Type)
9696
extends CachedGroundType with ValueType {
9797
override def computeHash(bs: Hashable.Binders): Int = doHash(bs, tycon, erasedUnderlying)
98-
99-
final def valueClassSymbol(using Context): ClassSymbol = tycon.typeSymbol.asClass
10098
}
10199

102100
final class CachedErasedValueType(tycon: TypeRef, erasedUnderlying: Type)

compiler/src/dotty/tools/dotc/transform/sjs/ExplicitJSClasses.scala

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -250,20 +250,17 @@ class ExplicitJSClasses extends MiniPhase with InfoTransformer { thisPhase =>
250250

251251
override def changesMembers: Boolean = true // the phase adds fields for inner JS classes
252252

253-
/** Is the given symbol an owner for which this transformation applies?
253+
/** Is the given symbol an owner that might receive `\$jsclass` and/or `\$jsobject` fields?
254254
*
255255
* This applies if either or both of the following are true:
256256
* - It is not a static owner, or
257257
* - It is a non-native JS object.
258258
*
259259
* The latter is necessary for scala-js/scala-js#4086.
260260
*/
261-
private def isApplicableOwner(sym: Symbol)(using Context): Boolean = {
262-
!sym.isStaticOwner || (
263-
sym.is(ModuleClass) &&
264-
sym.hasAnnotation(jsdefn.JSTypeAnnot) &&
265-
!sym.hasAnnotation(jsdefn.JSNativeAnnot)
266-
)
261+
private def mayNeedJSClassOrJSObjectFields(sym: Symbol)(using Context): Boolean = {
262+
!sym.isStaticOwner
263+
|| (sym.is(ModuleClass) && sym.hasAnnotation(jsdefn.JSTypeAnnot) && !sym.hasAnnotation(jsdefn.JSNativeAnnot))
267264
}
268265

269266
/** Is the given symbol a JS class (that is not a trait nor an object)? */
@@ -329,7 +326,7 @@ class ExplicitJSClasses extends MiniPhase with InfoTransformer { thisPhase =>
329326
}
330327

331328
override def transformInfo(tp: Type, sym: Symbol)(using Context): Type = tp match {
332-
case tp @ ClassInfo(_, cls, _, decls, _) if !cls.is(JavaDefined) && isApplicableOwner(cls) =>
329+
case tp @ ClassInfo(_, cls, _, decls, _) if !cls.is(JavaDefined) && mayNeedJSClassOrJSObjectFields(cls) =>
333330
val innerJSClasses = decls.filter(isJSClass)
334331

335332
val innerObjectsForAdHocExposed =
@@ -451,7 +448,7 @@ class ExplicitJSClasses extends MiniPhase with InfoTransformer { thisPhase =>
451448
if (!cls.isJSType) tree.parents // fast path
452449
else tree.parents.mapConserve(unwrapWithContextualJSClassValue(_))
453450

454-
if (!isApplicableOwner(cls)) {
451+
if (!mayNeedJSClassOrJSObjectFields(cls)) {
455452
if (fixedParents eq tree.parents) tree
456453
else cpy.Template(tree)(parents = fixedParents)
457454
} else {

compiler/src/dotty/tools/dotc/transform/sjs/JSSymUtils.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,9 @@ object JSSymUtils {
5353
def isNonNativeJSClass(using Context): Boolean =
5454
sym.isJSType && !sym.hasAnnotation(jsdefn.JSNativeAnnot)
5555

56+
/** Is this symbol a nested JS class, i.e., an inner or local JS class? */
5657
def isNestedJSClass(using Context): Boolean =
57-
!sym.isStatic /*&& !isStaticModule(sym.originalOwner)*/ && sym.isJSType
58+
!sym.isStatic && sym.isJSType
5859

5960
/** Tests whether the given member is exposed, i.e., whether it was
6061
* originally a public or protected member of a non-native JS class.

0 commit comments

Comments
 (0)