From b6e641fa28e58272d4982545f514bf57685bd779 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Fri, 24 Jun 2022 00:39:27 -0400 Subject: [PATCH 1/7] More consistant results in pickling --- .../dotty/tools/dotc/core/TypeComparer.scala | 13 ++-- .../tools/dotc/core/tasty/TreeUnpickler.scala | 26 ++++---- .../dotty/tools/dotc/typer/Nullables.scala | 2 +- .../dotty/tools/dotc/CompilationTests.scala | 2 +- .../i14947.scala | 0 .../pos-pickling}/i15097.scala | 1 + .../pos-pickling/match-case.scala | 59 +++++++++++++++++++ .../pos-pickling/other-pickling.scala | 35 +++++++++++ .../pos-pickling/try-catch.scala | 27 +++++++++ 9 files changed, 142 insertions(+), 23 deletions(-) rename tests/explicit-nulls/{pos-special => pos-pickling}/i14947.scala (100%) rename tests/{pos => explicit-nulls/pos-pickling}/i15097.scala (92%) create mode 100644 tests/explicit-nulls/pos-pickling/match-case.scala create mode 100644 tests/explicit-nulls/pos-pickling/other-pickling.scala create mode 100644 tests/explicit-nulls/pos-pickling/try-catch.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 6d79f377c84e..e5216f2305ac 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -2141,7 +2141,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling * @note We do not admit singleton types in or-types as lubs. */ def lub(tp1: Type, tp2: Type, canConstrain: Boolean = false, isSoft: Boolean = true): Type = /*>|>*/ trace(s"lub(${tp1.show}, ${tp2.show}, canConstrain=$canConstrain, isSoft=$isSoft)", subtyping, show = true) /*<|<*/ { - if (tp1 eq tp2) tp1 + if tp1 eq tp2 then tp1 else if !tp1.exists || (tp2 eq WildcardType) then tp1 else if !tp2.exists || (tp1 eq WildcardType) then tp2 else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || isBottom(tp2) then tp1 @@ -2157,16 +2157,19 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling if (hi1 & hi2).isEmpty then return orType(tp1, tp2) case none => case none => + + // Simplifying the super type is important to avoid + // inconsistant result in union type. val t1 = mergeIfSuper(tp1, tp2, canConstrain) - if (t1.exists) return t1 + if t1.exists then return t1.simplified val t2 = mergeIfSuper(tp2, tp1, canConstrain) - if (t2.exists) return t2 + if t2.exists then return t2.simplified - def widen(tp: Type) = if (widenInUnions) tp.widen else tp.widenIfUnstable + def widen(tp: Type) = if widenInUnions then tp.widen else tp.widenIfUnstable val tp1w = widen(tp1) val tp2w = widen(tp2) - if ((tp1 ne tp1w) || (tp2 ne tp2w)) lub(tp1w, tp2w, canConstrain = canConstrain, isSoft = isSoft) + if (tp1 ne tp1w) || (tp2 ne tp2w) then lub(tp1w, tp2w, canConstrain = canConstrain, isSoft = isSoft) else orType(tp1w, tp2w, isSoft = isSoft) // no need to check subtypes again } mergedLub(tp1.stripLazyRef, tp2.stripLazyRef) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 440871481114..b5c79209aec9 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -1194,10 +1194,6 @@ class TreeUnpickler(reader: TastyReader, res.withAttachment(SuppressedApplyToNone, ()) else res - def simplifyLub(tree: Tree): Tree = - tree.overwriteType(tree.tpe.simplified) - tree - def readLengthTerm(): Tree = { val end = readEnd() val result = @@ -1247,16 +1243,15 @@ class TreeUnpickler(reader: TastyReader, val tpt = ifBefore(end)(readTpt(), EmptyTree) Closure(Nil, meth, tpt) case MATCH => - simplifyLub( - if (nextByte == IMPLICIT) { - readByte() - InlineMatch(EmptyTree, readCases(end)) - } - else if (nextByte == INLINE) { - readByte() - InlineMatch(readTerm(), readCases(end)) - } - else Match(readTerm(), readCases(end))) + if (nextByte == IMPLICIT) { + readByte() + InlineMatch(EmptyTree, readCases(end)) + } + else if (nextByte == INLINE) { + readByte() + InlineMatch(readTerm(), readCases(end)) + } + else Match(readTerm(), readCases(end)) case RETURN => val from = readSymRef() val expr = ifBefore(end)(readTerm(), EmptyTree) @@ -1264,8 +1259,7 @@ class TreeUnpickler(reader: TastyReader, case WHILE => WhileDo(readTerm(), readTerm()) case TRY => - simplifyLub( - Try(readTerm(), readCases(end), ifBefore(end)(readTerm(), EmptyTree))) + Try(readTerm(), readCases(end), ifBefore(end)(readTerm(), EmptyTree)) case SELECTouter => val levels = readNat() readTerm().outerSelect(levels, SkolemType(readType())) diff --git a/compiler/src/dotty/tools/dotc/typer/Nullables.scala b/compiler/src/dotty/tools/dotc/typer/Nullables.scala index 9104418d406f..60f0530bdc33 100644 --- a/compiler/src/dotty/tools/dotc/typer/Nullables.scala +++ b/compiler/src/dotty/tools/dotc/typer/Nullables.scala @@ -28,7 +28,7 @@ object Nullables: ctx.explicitNulls && !ctx.mode.is(Mode.SafeNulls) private def needNullifyHi(lo: Type, hi: Type)(using Context): Boolean = - ctx.explicitNulls + ctx.mode.is(Mode.SafeNulls) && lo.isExactlyNull // only nullify hi if lo is exactly Null type && hi.isValueType // We cannot check if hi is nullable, because it can cause cyclic reference. diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 1abb1ffd9ea3..5a143f112783 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -249,9 +249,9 @@ class CompilationTests { compileFilesInDir("tests/explicit-nulls/pos", explicitNullsOptions), compileFilesInDir("tests/explicit-nulls/pos-separate", explicitNullsOptions), compileFilesInDir("tests/explicit-nulls/pos-patmat", explicitNullsOptions and "-Xfatal-warnings"), + compileFilesInDir("tests/explicit-nulls/pos-pickling", explicitNullsOptions and "-Ytest-pickler" and "-Xprint-types"), compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-language:unsafeNulls"), compileFile("tests/explicit-nulls/pos-special/i14682.scala", explicitNullsOptions and "-Ysafe-init"), - compileFile("tests/explicit-nulls/pos-special/i14947.scala", explicitNullsOptions and "-Ytest-pickler" and "-Xprint-types"), ) }.checkCompile() diff --git a/tests/explicit-nulls/pos-special/i14947.scala b/tests/explicit-nulls/pos-pickling/i14947.scala similarity index 100% rename from tests/explicit-nulls/pos-special/i14947.scala rename to tests/explicit-nulls/pos-pickling/i14947.scala diff --git a/tests/pos/i15097.scala b/tests/explicit-nulls/pos-pickling/i15097.scala similarity index 92% rename from tests/pos/i15097.scala rename to tests/explicit-nulls/pos-pickling/i15097.scala index 8b9cb2b5d893..4eab7a39ce8d 100644 --- a/tests/pos/i15097.scala +++ b/tests/explicit-nulls/pos-pickling/i15097.scala @@ -10,6 +10,7 @@ class C: if ??? then g else "" def f3 = + import scala.language.unsafeNulls (??? : Boolean) match case true => g case _ => "" diff --git a/tests/explicit-nulls/pos-pickling/match-case.scala b/tests/explicit-nulls/pos-pickling/match-case.scala new file mode 100644 index 000000000000..f1f481c6e72e --- /dev/null +++ b/tests/explicit-nulls/pos-pickling/match-case.scala @@ -0,0 +1,59 @@ +import scala.language.unsafeNulls + +def a(): String | Null = ??? +val b: String | Null = ??? + +val i: Int = ??? + +def f1 = i match { + case 0 => b + case _ => a() +} + +def f2 = i match { + case 0 => a() + case _ => b +} + +def f3 = i match { + case 0 => a() + case _ => "".trim +} + +def f4 = i match { + case 0 => b + case _ => "".trim +} + +def g1 = i match { + case 0 => a() + case 1 => "" + case _ => null +} + +def g2 = i match { + case 0 => "" + case 1 => null + case _ => b +} + +def g3 = i match { + case 0 => null + case 1 => b + case _ => "" +} + +def h1(i: Int) = i match + case 0 => 0 + case 1 => true + case 2 => i.toString + case _ => null + +// This test still fails. +// Even without explicit nulls, the type of Match +// is (0, true, "2"), which is wrong. +// def h2(i: Int) = i match +// case 0 => 0 +// case 1 => true +// case 2 => "2" +// case _ => null \ No newline at end of file diff --git a/tests/explicit-nulls/pos-pickling/other-pickling.scala b/tests/explicit-nulls/pos-pickling/other-pickling.scala new file mode 100644 index 000000000000..1177eef2958a --- /dev/null +++ b/tests/explicit-nulls/pos-pickling/other-pickling.scala @@ -0,0 +1,35 @@ +import scala.language.unsafeNulls + +def associatedFile: String | Null = ??? + +def source: String = { + val f = associatedFile + if (f != null) f else associatedFile +} + +def defines(raw: String): List[String] = { + val ds: List[(Int, Int)] = ??? + ds map { case (start, end) => raw.substring(start, end) } +} + +abstract class DeconstructorCommon[T >: Null <: AnyRef] { + var field: T = null + def get: this.type = this + def isEmpty: Boolean = field eq null + def isDefined = !isEmpty + def unapply(s: T): this.type ={ + field = s + this + } +} + +def genBCode = + val bsmArgs: Array[Object | Null] | Null = null + val implMethod = bsmArgs(3).asInstanceOf[Integer].toInt + implMethod + +val arrayApply = "a".split(" ")(2) + +val globdir: String = if (??? : Boolean) then associatedFile.replaceAll("[\\\\/][^\\\\/]*$", "") else "" + +def newInstOfC(c: Class[?]) = c.getConstructor().newInstance() \ No newline at end of file diff --git a/tests/explicit-nulls/pos-pickling/try-catch.scala b/tests/explicit-nulls/pos-pickling/try-catch.scala new file mode 100644 index 000000000000..40c670173b89 --- /dev/null +++ b/tests/explicit-nulls/pos-pickling/try-catch.scala @@ -0,0 +1,27 @@ +import scala.language.unsafeNulls + + +def s: String | Null = ??? + +def tryString = try s catch { + case _: NullPointerException => null + case _ => "" +} + +def tryString2 = try s catch { + case _: NullPointerException => "" + case _ => s +} + +def loadClass(classLoader: ClassLoader, name: String): Class[?] = + try classLoader.loadClass(name) + catch { + case _ => + throw new Exception() + } + +def loadClass2(classLoader: ClassLoader, name: String): Class[?] = + try classLoader.loadClass(name) + catch { + case _ => null + } \ No newline at end of file From 3bd0823477d572e5c1230108e4047c6eaef85588 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Tue, 5 Jul 2022 15:58:05 -0400 Subject: [PATCH 2/7] Simplify the types of trees before and after pickling --- .../dotty/tools/dotc/core/TypeComparer.scala | 13 ++++----- .../dotty/tools/dotc/transform/Pickler.scala | 27 +++++++++++++++++-- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index e5216f2305ac..6d79f377c84e 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -2141,7 +2141,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling * @note We do not admit singleton types in or-types as lubs. */ def lub(tp1: Type, tp2: Type, canConstrain: Boolean = false, isSoft: Boolean = true): Type = /*>|>*/ trace(s"lub(${tp1.show}, ${tp2.show}, canConstrain=$canConstrain, isSoft=$isSoft)", subtyping, show = true) /*<|<*/ { - if tp1 eq tp2 then tp1 + if (tp1 eq tp2) tp1 else if !tp1.exists || (tp2 eq WildcardType) then tp1 else if !tp2.exists || (tp1 eq WildcardType) then tp2 else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || isBottom(tp2) then tp1 @@ -2157,19 +2157,16 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling if (hi1 & hi2).isEmpty then return orType(tp1, tp2) case none => case none => - - // Simplifying the super type is important to avoid - // inconsistant result in union type. val t1 = mergeIfSuper(tp1, tp2, canConstrain) - if t1.exists then return t1.simplified + if (t1.exists) return t1 val t2 = mergeIfSuper(tp2, tp1, canConstrain) - if t2.exists then return t2.simplified + if (t2.exists) return t2 - def widen(tp: Type) = if widenInUnions then tp.widen else tp.widenIfUnstable + def widen(tp: Type) = if (widenInUnions) tp.widen else tp.widenIfUnstable val tp1w = widen(tp1) val tp2w = widen(tp2) - if (tp1 ne tp1w) || (tp2 ne tp2w) then lub(tp1w, tp2w, canConstrain = canConstrain, isSoft = isSoft) + if ((tp1 ne tp1w) || (tp2 ne tp2w)) lub(tp1w, tp2w, canConstrain = canConstrain, isSoft = isSoft) else orType(tp1w, tp2w, isSoft = isSoft) // no need to check subtypes again } mergedLub(tp1.stripLazyRef, tp2.stripLazyRef) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 4d9b42a36fe7..17ade99102a9 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -12,6 +12,7 @@ import Phases._ import Symbols._ import Flags.Module import reporting.{ThrowingReporter, Profile} +import typer.Nullables import collection.mutable import scala.concurrent.{Future, Await, ExecutionContext} import scala.concurrent.duration.Duration @@ -60,13 +61,15 @@ class Pickler extends Phase { val unit = ctx.compilationUnit pickling.println(i"unpickling in run ${ctx.runId}") + val typeSimplifier = new TypeSimplifyTransformer + for cls <- dropCompanionModuleClasses(topLevelClasses(unit.tpdTree)) tree <- sliceTopLevel(unit.tpdTree, cls) do val pickler = new TastyPickler(cls) if ctx.settings.YtestPickler.value then - beforePickling(cls) = tree.show + beforePickling(cls) = typeSimplifier.transform(tree).show picklers(cls) = pickler val treePkl = new TreePickler(pickler) treePkl.pickle(tree :: Nil) @@ -134,8 +137,11 @@ class Pickler extends Phase { cls -> unpickler } pickling.println("************* entered toplevel ***********") + + val typeSimplifier = new TypeSimplifyTransformer + for ((cls, unpickler) <- unpicklers) { - val unpickled = unpickler.rootTrees + val unpickled = typeSimplifier.transform(unpickler.rootTrees) testSame(i"$unpickled%\n%", beforePickling(cls), cls) } } @@ -151,4 +157,21 @@ class Pickler extends Phase { | | diff before-pickling.txt after-pickling.txt""".stripMargin) end testSame + + // Overwrite types of If, Match, and Try nodes with simplified types + // to avoid inconsistencies in unsafe nulls + class TypeSimplifyTransformer extends TreeMapWithPreciseStatContexts: + override def transform(tree: Tree)(using Context): Tree = + try tree match + case _: If | _: Match | _: Try if Nullables.unsafeNullsEnabled => + val newTree = super.transform(tree) + newTree.overwriteType(newTree.tpe.simplified) + newTree + case _ => + super.transform(tree) + catch + case ex: TypeError => + report.error(ex, tree.srcPos) + tree + end TypeSimplifyTransformer } From cbd747dd7f44cd5b28503078b919434ca9acad1e Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Tue, 5 Jul 2022 16:30:39 -0400 Subject: [PATCH 3/7] Place typeSimplifier as a member of Pickling --- compiler/src/dotty/tools/dotc/transform/Pickler.scala | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 17ade99102a9..24227bfd9703 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -50,6 +50,8 @@ class Pickler extends Phase { private val beforePickling = new mutable.HashMap[ClassSymbol, String] private val picklers = new mutable.HashMap[ClassSymbol, TastyPickler] + private val typeSimplifier = new TypeSimplifyTransformer + /** Drop any elements of this list that are linked module classes of other elements in the list */ private def dropCompanionModuleClasses(clss: List[ClassSymbol])(using Context): List[ClassSymbol] = { val companionModuleClasses = @@ -61,8 +63,6 @@ class Pickler extends Phase { val unit = ctx.compilationUnit pickling.println(i"unpickling in run ${ctx.runId}") - val typeSimplifier = new TypeSimplifyTransformer - for cls <- dropCompanionModuleClasses(topLevelClasses(unit.tpdTree)) tree <- sliceTopLevel(unit.tpdTree, cls) @@ -137,9 +137,6 @@ class Pickler extends Phase { cls -> unpickler } pickling.println("************* entered toplevel ***********") - - val typeSimplifier = new TypeSimplifyTransformer - for ((cls, unpickler) <- unpicklers) { val unpickled = typeSimplifier.transform(unpickler.rootTrees) testSame(i"$unpickled%\n%", beforePickling(cls), cls) From 83a60b668dd83e3df54cf54a3e46151f52410566 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Tue, 12 Jul 2022 02:27:30 -0400 Subject: [PATCH 4/7] Simplify the pickled tree after unpickler --- compiler/src/dotty/tools/dotc/transform/Pickler.scala | 11 +++++------ .../src/dotty/tools/dotc/transform/PostTyper.scala | 4 ++++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 24227bfd9703..d5c9bb33862c 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -12,7 +12,6 @@ import Phases._ import Symbols._ import Flags.Module import reporting.{ThrowingReporter, Profile} -import typer.Nullables import collection.mutable import scala.concurrent.{Future, Await, ExecutionContext} import scala.concurrent.duration.Duration @@ -50,7 +49,7 @@ class Pickler extends Phase { private val beforePickling = new mutable.HashMap[ClassSymbol, String] private val picklers = new mutable.HashMap[ClassSymbol, TastyPickler] - private val typeSimplifier = new TypeSimplifyTransformer + private val typeSimplifier = new TypeSimplifier /** Drop any elements of this list that are linked module classes of other elements in the list */ private def dropCompanionModuleClasses(clss: List[ClassSymbol])(using Context): List[ClassSymbol] = { @@ -69,7 +68,7 @@ class Pickler extends Phase { do val pickler = new TastyPickler(cls) if ctx.settings.YtestPickler.value then - beforePickling(cls) = typeSimplifier.transform(tree).show + beforePickling(cls) = tree.show picklers(cls) = pickler val treePkl = new TreePickler(pickler) treePkl.pickle(tree :: Nil) @@ -157,10 +156,10 @@ class Pickler extends Phase { // Overwrite types of If, Match, and Try nodes with simplified types // to avoid inconsistencies in unsafe nulls - class TypeSimplifyTransformer extends TreeMapWithPreciseStatContexts: + class TypeSimplifier extends TreeMapWithPreciseStatContexts: override def transform(tree: Tree)(using Context): Tree = try tree match - case _: If | _: Match | _: Try if Nullables.unsafeNullsEnabled => + case _: If | _: Match | _: Try => val newTree = super.transform(tree) newTree.overwriteType(newTree.tpe.simplified) newTree @@ -170,5 +169,5 @@ class Pickler extends Phase { case ex: TypeError => report.error(ex, tree.srcPos) tree - end TypeSimplifyTransformer + end TypeSimplifier } diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala index 8b7ac94675a1..ab3372ab7d53 100644 --- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala @@ -460,6 +460,10 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase ) case Block(_, Closure(_, _, tpt)) if ExpandSAMs.needsWrapperClass(tpt.tpe) => superAcc.withInvalidCurrentClass(super.transform(tree)) + case _: If | _: Match | _: Try => + val newTree = super.transform(tree) + newTree.overwriteType(newTree.tpe.simplified) + newTree case tree => super.transform(tree) } From bec2660e1d63ba78241fc95eec391f3c2ca9e69d Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 6 Jul 2022 17:27:13 -0400 Subject: [PATCH 5/7] Try to not add a transformer --- .../tools/dotc/core/tasty/TreeUnpickler.scala | 39 +++++++++++-------- .../dotty/tools/dotc/transform/Pickler.scala | 2 +- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index b5c79209aec9..fef9cfb8b359 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -1194,6 +1194,10 @@ class TreeUnpickler(reader: TastyReader, res.withAttachment(SuppressedApplyToNone, ()) else res + def simplifyLub(tree: Tree): Tree = + tree.overwriteType(tree.tpe.simplified) + tree + def readLengthTerm(): Tree = { val end = readEnd() val result = @@ -1232,26 +1236,28 @@ class TreeUnpickler(reader: TastyReader, val expansion = exprReader.readTerm() // need bindings in scope, so needs to be read before Inlined(call, bindings, expansion) case IF => - if (nextByte == INLINE) { - readByte() - InlineIf(readTerm(), readTerm(), readTerm()) - } - else - If(readTerm(), readTerm(), readTerm()) + simplifyLub( + if (nextByte == INLINE) { + readByte() + InlineIf(readTerm(), readTerm(), readTerm()) + } + else + If(readTerm(), readTerm(), readTerm())) case LAMBDA => val meth = readTerm() val tpt = ifBefore(end)(readTpt(), EmptyTree) Closure(Nil, meth, tpt) case MATCH => - if (nextByte == IMPLICIT) { - readByte() - InlineMatch(EmptyTree, readCases(end)) - } - else if (nextByte == INLINE) { - readByte() - InlineMatch(readTerm(), readCases(end)) - } - else Match(readTerm(), readCases(end)) + simplifyLub( + if (nextByte == IMPLICIT) { + readByte() + InlineMatch(EmptyTree, readCases(end)) + } + else if (nextByte == INLINE) { + readByte() + InlineMatch(readTerm(), readCases(end)) + } + else Match(readTerm(), readCases(end))) case RETURN => val from = readSymRef() val expr = ifBefore(end)(readTerm(), EmptyTree) @@ -1259,7 +1265,8 @@ class TreeUnpickler(reader: TastyReader, case WHILE => WhileDo(readTerm(), readTerm()) case TRY => - Try(readTerm(), readCases(end), ifBefore(end)(readTerm(), EmptyTree)) + simplifyLub( + Try(readTerm(), readCases(end), ifBefore(end)(readTerm(), EmptyTree))) case SELECTouter => val levels = readNat() readTerm().outerSelect(levels, SkolemType(readType())) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index d5c9bb33862c..fe9b297bdb17 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -137,7 +137,7 @@ class Pickler extends Phase { } pickling.println("************* entered toplevel ***********") for ((cls, unpickler) <- unpicklers) { - val unpickled = typeSimplifier.transform(unpickler.rootTrees) + val unpickled = unpickler.rootTrees testSame(i"$unpickled%\n%", beforePickling(cls), cls) } } From b983dbff18c15117a44093d45c4fada7a557edd4 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Tue, 12 Jul 2022 02:23:59 -0400 Subject: [PATCH 6/7] Simplify the pickled tree after unpickler --- .../tools/dotc/core/tasty/TreeUnpickler.scala | 39 ++++++++----------- .../dotty/tools/dotc/transform/Pickler.scala | 9 ++++- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index fef9cfb8b359..b5c79209aec9 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -1194,10 +1194,6 @@ class TreeUnpickler(reader: TastyReader, res.withAttachment(SuppressedApplyToNone, ()) else res - def simplifyLub(tree: Tree): Tree = - tree.overwriteType(tree.tpe.simplified) - tree - def readLengthTerm(): Tree = { val end = readEnd() val result = @@ -1236,28 +1232,26 @@ class TreeUnpickler(reader: TastyReader, val expansion = exprReader.readTerm() // need bindings in scope, so needs to be read before Inlined(call, bindings, expansion) case IF => - simplifyLub( - if (nextByte == INLINE) { - readByte() - InlineIf(readTerm(), readTerm(), readTerm()) - } - else - If(readTerm(), readTerm(), readTerm())) + if (nextByte == INLINE) { + readByte() + InlineIf(readTerm(), readTerm(), readTerm()) + } + else + If(readTerm(), readTerm(), readTerm()) case LAMBDA => val meth = readTerm() val tpt = ifBefore(end)(readTpt(), EmptyTree) Closure(Nil, meth, tpt) case MATCH => - simplifyLub( - if (nextByte == IMPLICIT) { - readByte() - InlineMatch(EmptyTree, readCases(end)) - } - else if (nextByte == INLINE) { - readByte() - InlineMatch(readTerm(), readCases(end)) - } - else Match(readTerm(), readCases(end))) + if (nextByte == IMPLICIT) { + readByte() + InlineMatch(EmptyTree, readCases(end)) + } + else if (nextByte == INLINE) { + readByte() + InlineMatch(readTerm(), readCases(end)) + } + else Match(readTerm(), readCases(end)) case RETURN => val from = readSymRef() val expr = ifBefore(end)(readTerm(), EmptyTree) @@ -1265,8 +1259,7 @@ class TreeUnpickler(reader: TastyReader, case WHILE => WhileDo(readTerm(), readTerm()) case TRY => - simplifyLub( - Try(readTerm(), readCases(end), ifBefore(end)(readTerm(), EmptyTree))) + Try(readTerm(), readCases(end), ifBefore(end)(readTerm(), EmptyTree)) case SELECTouter => val levels = readNat() readTerm().outerSelect(levels, SkolemType(readType())) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index fe9b297bdb17..108a088ccc9d 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -99,6 +99,13 @@ class Pickler extends Phase { println(i"**** pickled info of $cls") println(TastyPrinter.showContents(pickled, ctx.settings.color.value == "never")) } + // println(i"**** pickled info of $cls") + // println(TastyPrinter.showContents(pickled, ctx.settings.color.value == "never")) + if cls.show.contains("MainGenericRunner") then + import java.nio.file.{Paths, Files} + import java.nio.charset.StandardCharsets + + Files.write(Paths.get("debug.txt"), TastyPrinter.showContents(pickled, true).getBytes(StandardCharsets.UTF_8)) pickled }(using ExecutionContext.global) } @@ -137,7 +144,7 @@ class Pickler extends Phase { } pickling.println("************* entered toplevel ***********") for ((cls, unpickler) <- unpicklers) { - val unpickled = unpickler.rootTrees + val unpickled = typeSimplifier.transform(unpickler.rootTrees) testSame(i"$unpickled%\n%", beforePickling(cls), cls) } } From c322cf237f14162ff9fb61f714f1f637e7883988 Mon Sep 17 00:00:00 2001 From: noti0na1 Date: Wed, 6 Jul 2022 17:43:15 -0400 Subject: [PATCH 7/7] Enable explicit nulls for all compiling --- .../dotc/util/ClasspathFromClassloader.scala | 2 +- .../tools/dotc/util/GenericHashMap.scala | 2 +- .../src/dotty/tools/dotc/util/HashSet.scala | 13 +++++----- .../dotty/tools/dotc/util/ReadOnlyMap.scala | 26 ++++++++++++------- .../dotty/tools/dotc/util/WeakHashSet.scala | 2 +- .../tools/dotc/TastyBootstrapTests.scala | 2 +- .../tools/vulpix/TestConfiguration.scala | 4 +-- project/Build.scala | 6 ++--- .../GenericNumLits/EvenFromDigitsImpl_1.scala | 2 +- tests/pos-with-compiler/Fileish.scala | 6 ++--- tests/pos-with-compiler/Patterns.scala | 2 +- tests/pos-with-compiler/benchSets.scala | 2 +- tests/pos-with-compiler/lazyValsSepComp.scala | 2 +- tests/run-with-compiler/i14541.scala | 2 +- tests/run-with-compiler/scripting.scala | 2 +- 15 files changed, 40 insertions(+), 35 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/ClasspathFromClassloader.scala b/compiler/src/dotty/tools/dotc/util/ClasspathFromClassloader.scala index 25def103083d..25cefd767550 100644 --- a/compiler/src/dotty/tools/dotc/util/ClasspathFromClassloader.scala +++ b/compiler/src/dotty/tools/dotc/util/ClasspathFromClassloader.scala @@ -14,7 +14,7 @@ object ClasspathFromClassloader { * BEWARE: with exotic enough classloaders, this may not work at all or do * the wrong thing. */ - def apply(cl: ClassLoader): String = { + def apply(cl: ClassLoader | Null): String = { val classpathBuff = List.newBuilder[String] def collectClassLoaderPaths(cl: ClassLoader): Unit = { if (cl != null) { diff --git a/compiler/src/dotty/tools/dotc/util/GenericHashMap.scala b/compiler/src/dotty/tools/dotc/util/GenericHashMap.scala index fd6518fcc15c..213f08b2cc3d 100644 --- a/compiler/src/dotty/tools/dotc/util/GenericHashMap.scala +++ b/compiler/src/dotty/tools/dotc/util/GenericHashMap.scala @@ -133,7 +133,7 @@ abstract class GenericHashMap[Key, Value] val v1 = value v = v1 update(key, v1) - v.uncheckedNN + v private def addOld(key: Key, value: Value): Unit = Stats.record(statsItem("re-enter")) diff --git a/compiler/src/dotty/tools/dotc/util/HashSet.scala b/compiler/src/dotty/tools/dotc/util/HashSet.scala index a524dd39a594..84cd1157b16c 100644 --- a/compiler/src/dotty/tools/dotc/util/HashSet.scala +++ b/compiler/src/dotty/tools/dotc/util/HashSet.scala @@ -84,7 +84,7 @@ class HashSet[T](initialCapacity: Int = 8, capacityMultiple: Int = 2) extends Mu var idx = firstIndex(x) var e: T | Null = entryAt(idx) while e != null do - if isEqual(e.uncheckedNN, x) then return e + if isEqual(e, x) then return e idx = nextIndex(idx) e = entryAt(idx) null @@ -102,8 +102,7 @@ class HashSet[T](initialCapacity: Int = 8, capacityMultiple: Int = 2) extends Mu var idx = firstIndex(x) var e: T | Null = entryAt(idx) while e != null do - // TODO: remove uncheckedNN when explicit-nulls is enabled for regule compiling - if isEqual(e.uncheckedNN, x) then return e.uncheckedNN + if isEqual(e, x) then return e idx = nextIndex(idx) e = entryAt(idx) addEntryAt(idx, x) @@ -115,20 +114,20 @@ class HashSet[T](initialCapacity: Int = 8, capacityMultiple: Int = 2) extends Mu var idx = firstIndex(x) var e: T | Null = entryAt(idx) while e != null do - if isEqual(e.uncheckedNN, x) then + if isEqual(e, x) then var hole = idx while idx = nextIndex(idx) e = entryAt(idx) e != null do - val eidx = index(hash(e.uncheckedNN)) + val eidx = index(hash(e)) if isDense || index(eidx - (hole + 1)) > index(idx - (hole + 1)) // entry `e` at `idx` can move unless `index(hash(e))` is in // the (ring-)interval [hole + 1 .. idx] then - setEntry(hole, e.uncheckedNN) + setEntry(hole, e) hole = idx table(hole) = null used -= 1 @@ -156,7 +155,7 @@ class HashSet[T](initialCapacity: Int = 8, capacityMultiple: Int = 2) extends Mu var idx = 0 while idx < oldTable.length do val e: T | Null = oldTable(idx).asInstanceOf[T | Null] - if e != null then addOld(e.uncheckedNN) + if e != null then addOld(e) idx += 1 protected def growTable(): Unit = diff --git a/compiler/src/dotty/tools/dotc/util/ReadOnlyMap.scala b/compiler/src/dotty/tools/dotc/util/ReadOnlyMap.scala index 020303c18bc2..7271a47988bd 100644 --- a/compiler/src/dotty/tools/dotc/util/ReadOnlyMap.scala +++ b/compiler/src/dotty/tools/dotc/util/ReadOnlyMap.scala @@ -15,19 +15,25 @@ abstract class ReadOnlyMap[Key, Value]: def isEmpty: Boolean = size == 0 - def get(key: Key): Option[Value] = lookup(key) match - case null => None - case v => Some(v.uncheckedNN) - - def getOrElse(key: Key, value: => Value) = lookup(key) match - case null => value - case v => v.uncheckedNN + def get(key: Key): Option[Value] = + val v = lookup(key) + v match + case null => None + case _ => Some(v) + + def getOrElse(key: Key, value: => Value) = + val v = lookup(key) + v match + case null => value + case _ => v def contains(key: Key): Boolean = lookup(key) != null - def apply(key: Key): Value = lookup(key) match - case null => throw new NoSuchElementException(s"$key") - case v => v.uncheckedNN + def apply(key: Key): Value = + val v = lookup(key) + v match + case null => throw new NoSuchElementException(s"$key") + case _ => v def toArray: Array[(Key, Value)] = val result = new Array[(Key, Value)](size) diff --git a/compiler/src/dotty/tools/dotc/util/WeakHashSet.scala b/compiler/src/dotty/tools/dotc/util/WeakHashSet.scala index 3c23b181a041..af7719033b52 100644 --- a/compiler/src/dotty/tools/dotc/util/WeakHashSet.scala +++ b/compiler/src/dotty/tools/dotc/util/WeakHashSet.scala @@ -178,7 +178,7 @@ abstract class WeakHashSet[A <: AnyRef](initialCapacity: Int = 8, loadFactor: Do case null => addEntryAt(bucket, elem, h, oldHead) case _ => val entryElem = entry.get - if entryElem != null && isEqual(elem, entryElem) then entryElem.uncheckedNN + if entryElem != null && isEqual(elem, entryElem) then entryElem else linkedListLoop(entry.tail) } diff --git a/compiler/test/dotty/tools/dotc/TastyBootstrapTests.scala b/compiler/test/dotty/tools/dotc/TastyBootstrapTests.scala index 9e71b10b206d..2551a764ae4d 100644 --- a/compiler/test/dotty/tools/dotc/TastyBootstrapTests.scala +++ b/compiler/test/dotty/tools/dotc/TastyBootstrapTests.scala @@ -50,7 +50,7 @@ class TastyBootstrapTests { Properties.compilerInterface, Properties.scalaLibrary, Properties.scalaAsm, Properties.dottyInterfaces, Properties.jlineTerminal, Properties.jlineReader, ).mkString(File.pathSeparator), - Array("-Ycheck-reentrant", "-language:postfixOps", "-Xsemanticdb") + Array("-Ycheck-reentrant", "-language:postfixOps", "-Xsemanticdb", "-Yexplicit-nulls") ) val libraryDirs = List(Paths.get("library/src"), Paths.get("library/src-bootstrapped")) diff --git a/compiler/test/dotty/tools/vulpix/TestConfiguration.scala b/compiler/test/dotty/tools/vulpix/TestConfiguration.scala index 1f53add86667..47da30311533 100644 --- a/compiler/test/dotty/tools/vulpix/TestConfiguration.scala +++ b/compiler/test/dotty/tools/vulpix/TestConfiguration.scala @@ -66,7 +66,7 @@ object TestConfiguration { val defaultOptions = TestFlags(basicClasspath, commonOptions) val unindentOptions = TestFlags(basicClasspath, Array("-no-indent") ++ checkOptions ++ noCheckOptions ++ yCheckOptions) val withCompilerOptions = - defaultOptions.withClasspath(withCompilerClasspath).withRunClasspath(withCompilerClasspath) + defaultOptions.withClasspath(withCompilerClasspath).withRunClasspath(withCompilerClasspath) and "-Yexplicit-nulls" lazy val withStagingOptions = defaultOptions.withClasspath(withStagingClasspath).withRunClasspath(withStagingClasspath) lazy val withTastyInspectorOptions = @@ -82,7 +82,7 @@ object TestConfiguration { "-Yprint-pos-syms" ) val picklingWithCompilerOptions = - picklingOptions.withClasspath(withCompilerClasspath).withRunClasspath(withCompilerClasspath) + picklingOptions.withClasspath(withCompilerClasspath).withRunClasspath(withCompilerClasspath) and "-Yexplicit-nulls" val scala2CompatMode = defaultOptions.and("-source", "3.0-migration") val explicitUTF8 = defaultOptions and ("-encoding", "UTF8") val explicitUTF16 = defaultOptions and ("-encoding", "UTF16") diff --git a/project/Build.scala b/project/Build.scala index fb5dabf25080..8f52ff6c14e7 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -612,6 +612,9 @@ object Build { Compile / mainClass := Some("dotty.tools.dotc.Main"), + // Note: bench/profiles/projects.yml should be updated accordingly. + Compile / scalacOptions ++= Seq("-Yexplicit-nulls"), + scala := { val args: List[String] = spaceDelimited("").parsed.toList val externalDeps = externalCompilerClasspathTask.value @@ -774,9 +777,6 @@ object Build { ) }, - // Note: bench/profiles/projects.yml should be updated accordingly. - Compile / scalacOptions ++= Seq("-Yexplicit-nulls"), - repl := (Compile / console).value, Compile / console / scalacOptions := Nil, // reset so that we get stock REPL behaviour! E.g. avoid -unchecked being enabled ) diff --git a/tests/neg-with-compiler/GenericNumLits/EvenFromDigitsImpl_1.scala b/tests/neg-with-compiler/GenericNumLits/EvenFromDigitsImpl_1.scala index 2c8e0d088ea8..5096c360c97a 100644 --- a/tests/neg-with-compiler/GenericNumLits/EvenFromDigitsImpl_1.scala +++ b/tests/neg-with-compiler/GenericNumLits/EvenFromDigitsImpl_1.scala @@ -10,7 +10,7 @@ object EvenFromDigitsImpl: try evenFromDigits(ds) catch { case ex: FromDigits.FromDigitsException => - quotes.reflect.report.error(ex.getMessage) + quotes.reflect.report.error(ex.getMessage.nn) Even(0) } '{Even(${Expr(ev.n)})} diff --git a/tests/pos-with-compiler/Fileish.scala b/tests/pos-with-compiler/Fileish.scala index 8ba62efb3298..55ea758901a5 100644 --- a/tests/pos-with-compiler/Fileish.scala +++ b/tests/pos-with-compiler/Fileish.scala @@ -19,7 +19,7 @@ class Fileish(val path: Path, val input: () => InputStream) extends Streamable.C private lazy val pkgLines = lines() collect { case x if x startsWith "package " => x stripPrefix "package" trim } lazy val pkgFromPath = parent.path.replaceAll("""[/\\]""", ".") - lazy val pkgFromSource = pkgLines map (_ stripSuffix ";") mkString "." + lazy val pkgFromSource = pkgLines map (_.nn.stripSuffix(";")) mkString "." override def toString = path.path } @@ -32,7 +32,7 @@ class Fileish2(val path: Path, val input: () => InputStream) extends Streamable. private val pkgLines = lines() collect { case x if x startsWith "package " => x stripPrefix "package" trim } lazy val pkgFromPath = parent.path.replaceAll("""[/\\]""", ".") - lazy val pkgFromSource = pkgLines map (_ stripSuffix ";") mkString "." + lazy val pkgFromSource = pkgLines map (_.nn.stripSuffix(";")) mkString "." override def toString = path.path } @@ -46,7 +46,7 @@ class Fileish3(val path: Path, val input: () => InputStream) extends Streamable. private val pkgLines = lines() collect { case x if x startsWith "package " => x stripPrefix "package" trim } private val pkgFromPath = parent.path.replaceAll("""[/\\]""", ".") - private val pkgFromSource = pkgLines map (_ stripSuffix ";") mkString "." + private val pkgFromSource = pkgLines map (_.nn.stripSuffix(";")) mkString "." override def toString = path.path } diff --git a/tests/pos-with-compiler/Patterns.scala b/tests/pos-with-compiler/Patterns.scala index 6492ce6f8c72..d1791e7078f9 100644 --- a/tests/pos-with-compiler/Patterns.scala +++ b/tests/pos-with-compiler/Patterns.scala @@ -2,7 +2,7 @@ import dotty.tools.dotc.ast.Trees.* import dotty.tools.dotc.core.Types.* object Patterns { - val d: Object = null + val d: Object = null.asInstanceOf[Object] private def rebase(tp: NamedType): Type = { def rebaseFrom(prefix: Type): Type = ??? tp.prefix match { diff --git a/tests/pos-with-compiler/benchSets.scala b/tests/pos-with-compiler/benchSets.scala index 8619318956f9..99d9fafb3bb6 100644 --- a/tests/pos-with-compiler/benchSets.scala +++ b/tests/pos-with-compiler/benchSets.scala @@ -78,7 +78,7 @@ def testAnyRefMap = i += 1 while i > 0 do i -= 1 - val v = set.getOrNull(elems(i)) + val v: Elem | Null = set.getOrNull(elems(i)) if v != null then count += 1 iter += 1 diff --git a/tests/pos-with-compiler/lazyValsSepComp.scala b/tests/pos-with-compiler/lazyValsSepComp.scala index c7e97dc12142..43bd8575feb3 100644 --- a/tests/pos-with-compiler/lazyValsSepComp.scala +++ b/tests/pos-with-compiler/lazyValsSepComp.scala @@ -10,7 +10,7 @@ import dotty.tools.dotc.core.Contexts.* /** A test to trigger issue with separate compilation and lazy vals */ object Foo { - val definitions: Definitions = null + val definitions: Definitions = null.asInstanceOf[Definitions] def defn = definitions def go = defn.ScalaBoxedClasses } diff --git a/tests/run-with-compiler/i14541.scala b/tests/run-with-compiler/i14541.scala index 0fdfb89674d5..a609da132ed4 100644 --- a/tests/run-with-compiler/i14541.scala +++ b/tests/run-with-compiler/i14541.scala @@ -4,7 +4,7 @@ object Test: import dotty.tools.runner.RichClassLoader.* val classpath = dotty.tools.dotc.util.ClasspathFromClassloader(getClass.getClassLoader) def main(args: Array[String]): Unit = - getClass.getClassLoader.run("echo", List("hello", "raw", "world")) + getClass.getClassLoader.nn.run("echo", List("hello", "raw", "world")) // caution: uses "SCALA_OPTS" dotty.tools.MainGenericRunner.main(Array("--class-path", classpath, "echo", "hello", "run", "world")) diff --git a/tests/run-with-compiler/scripting.scala b/tests/run-with-compiler/scripting.scala index b42fd714e05a..88359a4feee1 100644 --- a/tests/run-with-compiler/scripting.scala +++ b/tests/run-with-compiler/scripting.scala @@ -1,7 +1,7 @@ object Test { def main(args: Array[String]): Unit = { val m = new javax.script.ScriptEngineManager(getClass().getClassLoader()) - val e = m.getEngineByName("scala") + val e = m.getEngineByName("scala").nn println(e.eval("42")) println(e.eval("Some(42)").asInstanceOf[Option[Int]].get) println(e.eval(new java.io.StringReader("42")))