From c335b1859cd3c4cd2bd4938d085b3020cc46672d Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Mon, 20 Jan 2025 15:05:41 -0500 Subject: [PATCH 1/3] Wrap unsafeNull members in flexible types during unpickling --- .../dotty/tools/dotc/core/JavaNullInterop.scala | 2 +- .../tools/dotc/core/tasty/TreeUnpickler.scala | 6 ++++++ .../test/dotty/tools/dotc/CompilationTests.scala | 14 ++++++++++++-- .../flexible-unpickle/Flexible_2.scala | 7 +++++++ .../flexible-unpickle/Unsafe_1.scala | 6 ++++++ 5 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 tests/explicit-nulls/flexible-unpickle/Flexible_2.scala create mode 100644 tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala diff --git a/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala b/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala index 46ce0d2d7852..ab73d1244aa7 100644 --- a/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala +++ b/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala @@ -53,7 +53,7 @@ object JavaNullInterop { */ def nullifyMember(sym: Symbol, tp: Type, isEnumValueDef: Boolean)(using Context): Type = { assert(ctx.explicitNulls) - assert(sym.is(JavaDefined), "can only nullify java-defined members") + // assert(sym.is(JavaDefined), "can only nullify java-defined members") // Some special cases when nullifying the type if isEnumValueDef || sym.name == nme.TYPE_ then diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index c07121a52191..b7ae6579bbd2 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -981,6 +981,12 @@ class TreeUnpickler(reader: TastyReader, sym.info = tpt.tpe ValDef(tpt) } + + // If explicit nulls is enabled, and the source file did not have explicit + // nulls enabled, nullify the member to allow for compatibility. + if (ctx.explicitNulls && !explicitNulls) then + sym.info = JavaNullInterop.nullifyMember(sym, sym.info, sym.is(Enum)) + goto(end) setSpan(start, tree) diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 70f55e569784..fe8f904f32f6 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -213,8 +213,18 @@ class CompilationTests { compileFilesInDir("tests/explicit-nulls/pos", explicitNullsOptions), compileFilesInDir("tests/explicit-nulls/flexible-types-common", explicitNullsOptions), compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-language:unsafeNulls" and "-Yno-flexible-types"), - ) - }.checkCompile() + ).checkCompile() + + locally { + val tests = List( + compileFile("tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala", explicitNullsOptions without "-Yexplicit-nulls"), + compileFile("tests/explicit-nulls/flexible-unpickle/Flexible_2.scala", explicitNullsOptions.withClasspath( + defaultOutputDir + testGroup + "/Unsafe_1/flexible-unpickle/Unsafe_1")), + ).map(_.keepOutput.checkCompile()) + + tests.foreach(_.delete()) + } + } @Test def explicitNullsWarn: Unit = { implicit val testGroup: TestGroup = TestGroup("explicitNullsWarn") diff --git a/tests/explicit-nulls/flexible-unpickle/Flexible_2.scala b/tests/explicit-nulls/flexible-unpickle/Flexible_2.scala new file mode 100644 index 000000000000..726a97b55f31 --- /dev/null +++ b/tests/explicit-nulls/flexible-unpickle/Flexible_2.scala @@ -0,0 +1,7 @@ +@main +def Flexible_2() = + val s2: String | Null = "foo" + val unsafe = new Unsafe_1() + val s: String = unsafe.foo(s2) + unsafe.foo("") + unsafe.foo(null) \ No newline at end of file diff --git a/tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala b/tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala new file mode 100644 index 000000000000..cf825934779d --- /dev/null +++ b/tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala @@ -0,0 +1,6 @@ +class Unsafe_1 { + def foo(s: String): String = { + if (s == null) then "nullString" + else s + } +} From d2544014e43878ad03ec76f30a731bdab5631591 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Sat, 25 Jan 2025 17:27:00 -0500 Subject: [PATCH 2/3] Do not nullify Modules --- compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala | 5 +++-- tests/explicit-nulls/flexible-unpickle/Flexible_2.scala | 3 +++ tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala | 6 ++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala b/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala index 1f78c91af2d4..239430e75f17 100644 --- a/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala +++ b/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala @@ -58,8 +58,9 @@ object JavaNullInterop { // assert(sym.is(JavaDefined), "can only nullify java-defined members") // Some special cases when nullifying the type - if isEnumValueDef || sym.name == nme.TYPE_ then - // Don't nullify the `TYPE` field in every class and Java enum instances + if isEnumValueDef || sym.name == nme.TYPE_ // Don't nullify the `TYPE` field in every class and Java enum instances + || sym.is(Flags.ModuleVal) // Don't nullify Modules + then tp else if sym.name == nme.toString_ || sym.isConstructor || hasNotNullAnnot(sym) then // Don't nullify the return type of the `toString` method. diff --git a/tests/explicit-nulls/flexible-unpickle/Flexible_2.scala b/tests/explicit-nulls/flexible-unpickle/Flexible_2.scala index 726a97b55f31..413e1a5f237c 100644 --- a/tests/explicit-nulls/flexible-unpickle/Flexible_2.scala +++ b/tests/explicit-nulls/flexible-unpickle/Flexible_2.scala @@ -1,3 +1,6 @@ +import unsafeNulls.Foo.* +import unsafeNulls.Unsafe_1 + @main def Flexible_2() = val s2: String | Null = "foo" diff --git a/tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala b/tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala index cf825934779d..77c3087fef70 100644 --- a/tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala +++ b/tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala @@ -1,6 +1,12 @@ +package unsafeNulls + class Unsafe_1 { def foo(s: String): String = { if (s == null) then "nullString" else s } } + +object Foo { + def bar = "bar!" +} \ No newline at end of file From 5a8790876f95a0885eb1dc0663670be033f20fe4 Mon Sep 17 00:00:00 2001 From: HarrisL2 Date: Sat, 8 Mar 2025 17:02:23 -0500 Subject: [PATCH 3/3] Refactor JavaNullInterop to reflect new usage --- ...ullInterop.scala => ImplicitNullInterop.scala} | 15 +++++++-------- compiler/src/dotty/tools/dotc/core/Types.scala | 1 + .../dotc/core/classfile/ClassfileParser.scala | 2 +- .../tools/dotc/core/tasty/TreeUnpickler.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Namer.scala | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) rename compiler/src/dotty/tools/dotc/core/{JavaNullInterop.scala => ImplicitNullInterop.scala} (93%) diff --git a/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala b/compiler/src/dotty/tools/dotc/core/ImplicitNullInterop.scala similarity index 93% rename from compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala rename to compiler/src/dotty/tools/dotc/core/ImplicitNullInterop.scala index 239430e75f17..783d373f8902 100644 --- a/compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala +++ b/compiler/src/dotty/tools/dotc/core/ImplicitNullInterop.scala @@ -35,7 +35,7 @@ import dotty.tools.dotc.core.Decorators.i * to handle the full spectrum of Scala types. Additionally, some kinds of symbols like constructors and * enum instances get special treatment. */ -object JavaNullInterop { +object ImplicitNullInterop { /** Transforms the type `tp` of Java member `sym` to be explicitly nullable. * `tp` is needed because the type inside `sym` might not be set when this method is called. @@ -55,7 +55,6 @@ object JavaNullInterop { */ def nullifyMember(sym: Symbol, tp: Type, isEnumValueDef: Boolean)(using Context): Type = trace(i"nullifyMember ${sym}, ${tp}"){ assert(ctx.explicitNulls) - // assert(sym.is(JavaDefined), "can only nullify java-defined members") // Some special cases when nullifying the type if isEnumValueDef || sym.name == nme.TYPE_ // Don't nullify the `TYPE` field in every class and Java enum instances @@ -81,14 +80,14 @@ object JavaNullInterop { * but the result type is not nullable. */ private def nullifyExceptReturnType(tp: Type)(using Context): Type = - new JavaNullMap(outermostLevelAlreadyNullable = true)(tp) + new ImplicitNullMap(outermostLevelAlreadyNullable = true)(tp) - /** Nullifies a Java type by adding `| Null` in the relevant places. */ + /** Nullifies a type by adding `| Null` in the relevant places. */ private def nullifyType(tp: Type)(using Context): Type = - new JavaNullMap(outermostLevelAlreadyNullable = false)(tp) + new ImplicitNullMap(outermostLevelAlreadyNullable = false)(tp) - /** A type map that implements the nullification function on types. Given a Java-sourced type, this adds `| Null` - * in the right places to make the nulls explicit in Scala. + /** A type map that implements the nullification function on types. Given a Java-sourced type or an + * implicitly null type, this adds `| Null` in the right places to make the nulls explicit. * * @param outermostLevelAlreadyNullable whether this type is already nullable at the outermost level. * For example, `Array[String] | Null` is already nullable at the @@ -98,7 +97,7 @@ object JavaNullInterop { * This is useful for e.g. constructors, and also so that `A & B` is nullified * to `(A & B) | Null`, instead of `(A | Null & B | Null) | Null`. */ - private class JavaNullMap(var outermostLevelAlreadyNullable: Boolean)(using Context) extends TypeMap { + private class ImplicitNullMap(var outermostLevelAlreadyNullable: Boolean)(using Context) extends TypeMap { def nullify(tp: Type): Type = if ctx.flexibleTypes then FlexibleType(tp) else OrNull(tp) /** Should we nullify `tp` at the outermost level? */ diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 2fcf628dbc01..8c28afed1097 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -3427,6 +3427,7 @@ object Types extends TypeUtils { // flexible type is always a subtype of the original type and the Object type. // It is not necessary according to the use cases, so we choose to use a simpler // rule. + assert(!tp.isInstanceOf[LazyType]) FlexibleType(OrNull(tp), tp) } } diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index cfbdc854a88f..4daf49aa8d2b 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -525,7 +525,7 @@ class ClassfileParser( denot.info = translateTempPoly(attrCompleter.complete(denot.info, isVarargs)) if (isConstructor) normalizeConstructorInfo() - if (ctx.explicitNulls) denot.info = JavaNullInterop.nullifyMember(denot.symbol, denot.info, isEnum) + if (ctx.explicitNulls) denot.info = ImplicitNullInterop.nullifyMember(denot.symbol, denot.info, isEnum) // seal java enums if (isEnum) { diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index b7ae6579bbd2..fe0fd908a1fe 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -985,7 +985,7 @@ class TreeUnpickler(reader: TastyReader, // If explicit nulls is enabled, and the source file did not have explicit // nulls enabled, nullify the member to allow for compatibility. if (ctx.explicitNulls && !explicitNulls) then - sym.info = JavaNullInterop.nullifyMember(sym, sym.info, sym.is(Enum)) + sym.info = ImplicitNullInterop.nullifyMember(sym, sym.info, sym.is(Enum)) goto(end) setSpan(start, tree) diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 21ef0fc5d123..9b7dfd339870 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -1879,7 +1879,7 @@ class Namer { typer: Typer => val mbrTpe = paramFn(checkSimpleKinded(typedAheadType(mdef.tpt, tptProto)).tpe) if (ctx.explicitNulls && mdef.mods.is(JavaDefined)) - JavaNullInterop.nullifyMember(sym, mbrTpe, mdef.mods.isAllOf(JavaEnumValue)) + ImplicitNullInterop.nullifyMember(sym, mbrTpe, mdef.mods.isAllOf(JavaEnumValue)) else mbrTpe }