Skip to content

Fix overriding in explicit nulls #13647

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.

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 6 commits into from
Oct 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 22 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,11 @@ object Types {

def isFromJavaObject(using Context): Boolean = typeSymbol eq defn.FromJavaObjectSymbol

def containsFromJavaObject(using Context): Boolean = this match
case tp: OrType => tp.tp1.containsFromJavaObject || tp.tp2.containsFromJavaObject
case tp: AndType => tp.tp1.containsFromJavaObject && tp.tp2.containsFromJavaObject
case _ => isFromJavaObject

/** True iff `symd` is a denotation of a class type parameter and the reference
* `<pre> . <symd>` is an actual argument reference, i.e. `pre` is not the
* ThisType of `symd`'s owner, or a reference to `symd`'s owner.'
Expand Down Expand Up @@ -4933,8 +4938,23 @@ object Types {
}

def & (that: TypeBounds)(using Context): TypeBounds =
if ((this.lo frozen_<:< that.lo) && (that.hi frozen_<:< this.hi)) that
else if ((that.lo frozen_<:< this.lo) && (this.hi frozen_<:< that.hi)) this
// This will try to preserve the FromJavaObjects type in upper bounds.
// For example, (? <: FromJavaObjects | Null) & (? <: Any),
// we want to get (? <: FromJavaObjects | Null) intead of (? <: Any),
// because we may check the result <:< (? <: Object | Null) later.
if this.hi.containsFromJavaObject
&& (this.hi frozen_<:< that.hi)
&& (that.lo frozen_<:< this.lo) then
// FromJavaObject in tp1.hi guarantees tp2.hi <:< tp1.hi
// prefer tp1 if FromJavaObject is in its hi
this
else if that.hi.containsFromJavaObject
&& (that.hi frozen_<:< this.hi)
&& (this.lo frozen_<:< that.lo) then
// Similarly, prefer tp2 if FromJavaObject is in its hi
that
else if (this.lo frozen_<:< that.lo) && (that.hi frozen_<:< this.hi) then that
else if (that.lo frozen_<:< this.lo) && (this.hi frozen_<:< that.hi) then this
else TypeBounds(this.lo | that.lo, this.hi & that.hi)

def | (that: TypeBounds)(using Context): TypeBounds =
Expand Down
6 changes: 5 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import Decorators._
import Denotations._, SymDenotations._
import TypeErasure.erasure
import DenotTransformers._
import NullOpsDecorator._

object ElimRepeated {
val name: String = "elimRepeated"
Expand Down Expand Up @@ -335,6 +336,9 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
val array = tp.translateFromRepeated(toArray = true) // Array[? <: T]
val element = array.elemType.hiBound // T

if element <:< defn.AnyRefType || element.typeSymbol.isPrimitiveValueClass then array

if element <:< defn.AnyRefType
|| ctx.mode.is(Mode.SafeNulls) && element.stripNull <:< defn.AnyRefType
|| element.typeSymbol.isPrimitiveValueClass then array
else defn.ArrayOf(TypeBounds.upper(AndType(element, defn.AnyRefType))) // Array[? <: T & AnyRef]
}
9 changes: 8 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,14 @@ object ResolveSuper {
// of the superaccessor's type, see i5433.scala for an example where this matters
val otherTp = other.asSeenFrom(base.typeRef).info
val accTp = acc.asSeenFrom(base.typeRef).info
if (!(otherTp.overrides(accTp, matchLoosely = true)))
// Since the super class can be Java defined,
// we use releaxed overriding check for explicit nulls if one of the symbols is Java defined.
// This forces `Null` being a subtype of reference types during override checking.
val relaxedCtxForNulls =
if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) then
ctx.retractMode(Mode.SafeNulls)
else ctx
if (!(otherTp.overrides(accTp, matchLoosely = true)(using relaxedCtxForNulls)))
report.error(IllegalSuperAccessor(base, memberName, targetName, acc, accTp, other.symbol, otherTp), base.srcPos)

bcs = bcs.tail
Expand Down
5 changes: 5 additions & 0 deletions tests/explicit-nulls/pos/i13040/Impl.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class Impl extends Intf:
override def test(x: Object | Null*): Unit = ???

class Impl2 extends Intf:
override def test(x: Object*): Unit = ???
3 changes: 3 additions & 0 deletions tests/explicit-nulls/pos/i13040/Intf.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
interface Intf {
void test(Object... x);
}
7 changes: 7 additions & 0 deletions tests/explicit-nulls/pos/i13486.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class MyPrintStream extends java.io.PrintStream(??? : java.io.OutputStream):
override def printf(format: String | Null, args: Array[? <: Object | Null])
: java.io.PrintStream | Null = ???

class MyPrintStream2 extends java.io.PrintStream(??? : java.io.OutputStream):
override def printf(format: String, args: Array[? <: Object])
: java.io.PrintStream = ???
3 changes: 3 additions & 0 deletions tests/explicit-nulls/pos/i13608.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import scala.util.control.NoStackTrace

case class ParseException(line: Int, character: Int, message: String) extends NoStackTrace