Skip to content

enable -Yexplicit-nulls for scala3-library #13729

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.

Sign up for GitHub

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 2 commits into from
Nov 8, 2021
Merged

enable -Yexplicit-nulls for scala3-library #13729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b2d1190
enable -Yexplicit-nulls for scala3-library
olhotak Oct 8, 2021
4a1c9f2
inline mapNull to avoid creating closures
olhotak Oct 12, 2021
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
31 changes: 18 additions & 13 deletions library/src/scala/IArray.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,54 +328,59 @@ object IArray:
extension [T, U >: T: ClassTag](x: T)
def +:(arr: IArray[U]): IArray[U] = genericArrayOps(arr).prepended(x)

// For backwards compatibility with code compiled without -Yexplicit-nulls
private inline def mapNull[A, B](a: A, inline f: B): B =
if((a: A|Null) == null) null.asInstanceOf[B] else f
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't use eq for nulls anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently not. The motivation comes from:
https://contributors.scala-lang.org/t/wip-scala-with-explicit-nulls/2761/4

I don't think this was discussed widely so if people feel strongly about it, we can discuss it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if A = X | Null (for some type X)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That shouldn't be a problem: an expression of type X | Null | Null is still allowed to be compared with null.


/** Conversion from IArray to immutable.ArraySeq */
implicit def genericWrapArray[T](arr: IArray[T]): ArraySeq[T] =
if arr eq null then null else ArraySeq.unsafeWrapArray(arr)
mapNull(arr, ArraySeq.unsafeWrapArray(arr))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapRefArray[T <: AnyRef](arr: IArray[T]): ArraySeq.ofRef[T] =
// Since the JVM thinks arrays are covariant, one 0-length Array[AnyRef]
// is as good as another for all T <: AnyRef. Instead of creating 100,000,000
// unique ones by way of this implicit, let's share one.
if (arr eq null) null
else if (arr.length == 0) ArraySeq.empty[AnyRef].asInstanceOf[ArraySeq.ofRef[T]]
else ArraySeq.ofRef(arr.asInstanceOf[Array[T]])
mapNull(arr,
if (arr.length == 0) ArraySeq.empty[AnyRef].asInstanceOf[ArraySeq.ofRef[T]]
else ArraySeq.ofRef(arr.asInstanceOf[Array[T]])
)

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapIntArray(arr: IArray[Int]): ArraySeq.ofInt =
if (arr ne null) new ArraySeq.ofInt(arr.asInstanceOf[Array[Int]]) else null
mapNull(arr, new ArraySeq.ofInt(arr.asInstanceOf[Array[Int]]))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapDoubleIArray(arr: IArray[Double]): ArraySeq.ofDouble =
if (arr ne null) new ArraySeq.ofDouble(arr.asInstanceOf[Array[Double]]) else null
mapNull(arr, new ArraySeq.ofDouble(arr.asInstanceOf[Array[Double]]))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapLongIArray(arr: IArray[Long]): ArraySeq.ofLong =
if (arr ne null) new ArraySeq.ofLong(arr.asInstanceOf[Array[Long]]) else null
mapNull(arr, new ArraySeq.ofLong(arr.asInstanceOf[Array[Long]]))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapFloatIArray(arr: IArray[Float]): ArraySeq.ofFloat =
if (arr ne null) new ArraySeq.ofFloat(arr.asInstanceOf[Array[Float]]) else null
mapNull(arr, new ArraySeq.ofFloat(arr.asInstanceOf[Array[Float]]))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapCharIArray(arr: IArray[Char]): ArraySeq.ofChar =
if (arr ne null) new ArraySeq.ofChar(arr.asInstanceOf[Array[Char]]) else null
mapNull(arr, new ArraySeq.ofChar(arr.asInstanceOf[Array[Char]]))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapByteIArray(arr: IArray[Byte]): ArraySeq.ofByte =
if (arr ne null) new ArraySeq.ofByte(arr.asInstanceOf[Array[Byte]]) else null
mapNull(arr, new ArraySeq.ofByte(arr.asInstanceOf[Array[Byte]]))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapShortIArray(arr: IArray[Short]): ArraySeq.ofShort =
if (arr ne null) new ArraySeq.ofShort(arr.asInstanceOf[Array[Short]]) else null
mapNull(arr, new ArraySeq.ofShort(arr.asInstanceOf[Array[Short]]))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapBooleanIArray(arr: IArray[Boolean]): ArraySeq.ofBoolean =
if (arr ne null) new ArraySeq.ofBoolean(arr.asInstanceOf[Array[Boolean]]) else null
mapNull(arr, new ArraySeq.ofBoolean(arr.asInstanceOf[Array[Boolean]]))

/** Conversion from IArray to immutable.ArraySeq */
implicit def wrapUnitIArray(arr: IArray[Unit]): ArraySeq.ofUnit =
if (arr ne null) new ArraySeq.ofUnit(arr.asInstanceOf[Array[Unit]]) else null
mapNull(arr, new ArraySeq.ofUnit(arr.asInstanceOf[Array[Unit]]))

/** Convert an array into an immutable array without copying, the original array
* must _not_ be mutated after this or the guaranteed immutablity of IArray will
Expand Down
4 changes: 2 additions & 2 deletions library/src/scala/reflect/Selectable.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ trait Selectable extends scala.Selectable:
final def selectDynamic(name: String): Any =
val rcls = selectedValue.getClass
try
val fld = rcls.getField(name)
val fld = rcls.getField(name).nn
ensureAccessible(fld)
fld.get(selectedValue)
catch case ex: NoSuchFieldException =>
Expand All @@ -35,7 +35,7 @@ trait Selectable extends scala.Selectable:
*/
final def applyDynamic(name: String, paramTypes: Class[_]*)(args: Any*): Any =
val rcls = selectedValue.getClass
val mth = rcls.getMethod(name, paramTypes: _*)
val mth = rcls.getMethod(name, paramTypes: _*).nn
ensureAccessible(mth)
mth.invoke(selectedValue, args.asInstanceOf[Seq[AnyRef]]: _*)

Expand Down
10 changes: 5 additions & 5 deletions library/src/scala/runtime/LazyVals.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@ package scala.runtime
*/
object LazyVals {
private[this] val unsafe: sun.misc.Unsafe =
classOf[sun.misc.Unsafe].getDeclaredFields.find { field =>
field.getType == classOf[sun.misc.Unsafe] && {
field.setAccessible(true)
classOf[sun.misc.Unsafe].getDeclaredFields.nn.find { field =>
field.nn.getType == classOf[sun.misc.Unsafe] && {
field.nn.setAccessible(true)
true
}
}
.map(_.get(null).asInstanceOf[sun.misc.Unsafe])
.map(_.nn.get(null).asInstanceOf[sun.misc.Unsafe])
Copy link

@h-vetinari h-vetinari Oct 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.nn.get(null) reads strange to me - it looks redundant contradictory?

Copy link
Member

@bishabosha bishabosha Oct 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here classOf[sun.misc.Unsafe].getDeclaredFields.nn is an Array[Field | Null] and then after calling find is an Option[Field | Null], so calling map would require asserting the inner value is also not null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.map is being called on Option[Field | Null].

The type of _ is thus Field | Null.

The type of _.nn is thus java.lang.reflect.Field. This type has a get(Object) method to read the value of the field. The API of this method says that for a static field, the parameter of type Object is ignored and should be null.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks for the explanation!

.getOrElse {
throw new ExceptionInInitializerError {
new IllegalStateException("Can't find instance of sun.misc.Unsafe")
}
}

private[this] val base: Int = {
val processors = java.lang.Runtime.getRuntime.availableProcessors()
val processors = java.lang.Runtime.getRuntime.nn.availableProcessors()
8 * processors * processors
}
private[this] val monitors: Array[Object] =
Expand Down
3 changes: 2 additions & 1 deletion library/src/scala/runtime/Scala3RunTime.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ object Scala3RunTime:
* Extracted to minimize the bytecode size at call site.
*/
def nn[T](x: T | Null): x.type & T =
if (x == null) throw new NullPointerException("tried to cast away nullability, but value is null")
val isNull = x == null
if (isNull) throw new NullPointerException("tried to cast away nullability, but value is null")
Comment on lines -18 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the benefit of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A workaround for #7882

else x.asInstanceOf[x.type & T]

end Scala3RunTime
4 changes: 2 additions & 2 deletions library/src/scala/util/FromDigits.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ object FromDigits {
case ex: NumberFormatException => throw MalformedNumber()
}
if (x.isInfinite) throw NumberTooLarge()
if (x == 0.0f && !zeroFloat.pattern.matcher(digits).matches) throw NumberTooSmall()
if (x == 0.0f && !zeroFloat.pattern.matcher(digits).nn.matches) throw NumberTooSmall()
x
}

Expand All @@ -153,7 +153,7 @@ object FromDigits {
case ex: NumberFormatException => throw MalformedNumber()
}
if (x.isInfinite) throw NumberTooLarge()
if (x == 0.0d && !zeroFloat.pattern.matcher(digits).matches) throw NumberTooSmall()
if (x == 0.0d && !zeroFloat.pattern.matcher(digits).nn.matches) throw NumberTooSmall()
x
}

Expand Down
1 change: 1 addition & 0 deletions project/Build.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ object Build {
(Compile / scalacOptions) ++= Seq(
// Needed so that the library sources are visible when `dotty.tools.dotc.core.Definitions#init` is called
"-sourcepath", (Compile / sourceDirectories).value.map(_.getAbsolutePath).distinct.mkString(File.pathSeparator),
"-Yexplicit-nulls",
),
)

Expand Down