Skip to content

Commit

Permalink
SI-6899, prohibit dangerous, useless implicit conversions.
Browse files Browse the repository at this point in the history
Increase eligibility requirements for implicit conversions,
such that T => U is ineligible if

  T <: Null    <or>   AnyRef <: U

This has the salutary effect of allowing us to ditch 16
ridiculous implicits from Predef, since they existed solely
to work around the absence of this restriction.

There was one tiny impact on actual source code (one line
in one file) shown here, necessitated because the literal null
is not eligible to be implicitly converted to A via <:<.

  def f[A](implicit ev: Null <:< A): A = null     // before
  def f[A](implicit ev: Null <:< A): A = ev(null) // after

As impositions go it's on the tame side.
  • Loading branch information
paulp committed Jun 4, 2013
1 parent 803d451 commit 2f0e5ec
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 66 deletions.
28 changes: 23 additions & 5 deletions src/compiler/scala/tools/nsc/typechecker/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,15 @@ trait Implicits {
private val infoMapCache = new LinkedHashMap[Symbol, InfoMap]
private val improvesCache = perRunCaches.newMap[(ImplicitInfo, ImplicitInfo), Boolean]()

private def isInvalidConversionTarget(tpe: Type): Boolean = tpe match {
case Function1(_, out) => AnyRefClass.tpe <:< out
case _ => false
}
private def isInvalidConversionSource(tpe: Type): Boolean = tpe match {
case Function1(in, _) => in <:< NullClass.tpe
case _ => false
}

def resetImplicits() {
implicitsCache.clear()
infoMapCache.clear()
Expand Down Expand Up @@ -1357,10 +1366,10 @@ trait Implicits {

val wasAmbigious = result.isAmbiguousFailure // SI-6667, never search companions after an ambiguous error in in-scope implicits
result = materializeImplicit(pt)

// `materializeImplicit` does some preprocessing for `pt`
// is it only meant for manifests/tags or we need to do the same for `implicitsOfExpectedType`?
if (result.isFailure && !wasAmbigious) result = searchImplicit(implicitsOfExpectedType, isLocal = false)
if (result.isFailure && !wasAmbigious)
result = searchImplicit(implicitsOfExpectedType, isLocal = false)

if (result.isFailure) {
context.updateBuffer(previousErrs)
Expand All @@ -1370,9 +1379,18 @@ trait Implicits {
if (Statistics.canEnable) Statistics.incCounter(oftypeImplicitHits)
}
}

if (result.isFailure && settings.debug)
log("no implicits found for "+pt+" "+pt.typeSymbol.info.baseClasses+" "+implicitsOfExpectedType)
if (result.isSuccess && isView) {
if (isInvalidConversionTarget(pt)) {
context.issueAmbiguousError(AmbiguousImplicitTypeError(tree, "the result type of an implicit conversion must be more specific than AnyRef"))
result = SearchFailure
}
else if (isInvalidConversionSource(pt)) {
context.issueAmbiguousError(AmbiguousImplicitTypeError(tree, "an expression of type Null is ineligible for implicit conversion"))
result = SearchFailure
}
}
if (result.isFailure)
debuglog("no implicits found for "+pt+" "+pt.typeSymbol.info.baseClasses+" "+implicitsOfExpectedType)

result
}
Expand Down
4 changes: 2 additions & 2 deletions src/library/scala/Option.scala
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ sealed abstract class Option[+A] extends Product with Serializable {
* val textField = new JComponent(initalText.orNull,20)
* }}}
*/
@inline final def orNull[A1 >: A](implicit ev: Null <:< A1): A1 = this getOrElse null
@inline final def orNull[A1 >: A](implicit ev: Null <:< A1): A1 = this getOrElse ev(null)

/** Returns a $some containing the result of applying $f to this $option's
* value if this $option is nonempty.
Expand Down Expand Up @@ -210,7 +210,7 @@ sealed abstract class Option[+A] extends Product with Serializable {
}

/** Tests whether the option contains a given value as an element.
*
*
* @param elem the element to test.
* @return `true` if the option has an element that is equal (as
* determined by `==`) to `elem`, `false` otherwise.
Expand Down
31 changes: 0 additions & 31 deletions src/library/scala/Predef.scala
Original file line number Diff line number Diff line change
Expand Up @@ -346,19 +346,6 @@ object Predef extends LowPriorityImplicits with DeprecatedPredef {
implicit def double2Double(x: Double) = java.lang.Double.valueOf(x)
implicit def boolean2Boolean(x: Boolean) = java.lang.Boolean.valueOf(x)

// These next eight implicits exist solely to exclude AnyRef methods from the
// eight implicits above so that primitives are not coerced to AnyRefs. They
// only create such conflict for AnyRef methods, so the methods on the java.lang
// boxed types are unambiguously reachable.
implicit def byte2ByteConflict(x: Byte) = new AnyRef
implicit def short2ShortConflict(x: Short) = new AnyRef
implicit def char2CharacterConflict(x: Char) = new AnyRef
implicit def int2IntegerConflict(x: Int) = new AnyRef
implicit def long2LongConflict(x: Long) = new AnyRef
implicit def float2FloatConflict(x: Float) = new AnyRef
implicit def double2DoubleConflict(x: Double) = new AnyRef
implicit def boolean2BooleanConflict(x: Boolean) = new AnyRef

implicit def Byte2byte(x: java.lang.Byte): Byte = x.byteValue
implicit def Short2short(x: java.lang.Short): Short = x.shortValue
implicit def Character2char(x: java.lang.Character): Char = x.charValue
Expand Down Expand Up @@ -481,24 +468,6 @@ private[scala] abstract class LowPriorityImplicits {
@inline implicit def doubleWrapper(x: Double) = new runtime.RichDouble(x)
@inline implicit def booleanWrapper(x: Boolean) = new runtime.RichBoolean(x)

// These eight implicits exist solely to exclude Null from the domain of
// the boxed types, so that e.g. "var x: Int = null" is a compile time
// error rather than a delayed null pointer exception by way of the
// conversion from java.lang.Integer. If defined in the same template as
// Integer2int, they would have higher priority because Null is a subtype
// of Integer. We balance that out and create conflict by moving the
// definition into the superclass.
//
// Caution: do not adjust tightrope tension without safety goggles in place.
implicit def Byte2byteNullConflict(x: Null): Byte = sys.error("value error")
implicit def Short2shortNullConflict(x: Null): Short = sys.error("value error")
implicit def Character2charNullConflict(x: Null): Char = sys.error("value error")
implicit def Integer2intNullConflict(x: Null): Int = sys.error("value error")
implicit def Long2longNullConflict(x: Null): Long = sys.error("value error")
implicit def Float2floatNullConflict(x: Null): Float = sys.error("value error")
implicit def Double2doubleNullConflict(x: Null): Double = sys.error("value error")
implicit def Boolean2booleanNullConflict(x: Null): Boolean = sys.error("value error")

implicit def genericWrapArray[T](xs: Array[T]): WrappedArray[T] =
if (xs eq null) null
else WrappedArray.make(xs)
Expand Down
8 changes: 1 addition & 7 deletions test/files/neg/no-implicit-to-anyref.check
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
no-implicit-to-anyref.scala:11: error: type mismatch;
found : Int(1)
required: AnyRef
Note: an implicit exists from scala.Int => java.lang.Integer, but
methods inherited from Object are rendered ambiguous. This is to avoid
a blanket implicit which would convert any scala.Int to any AnyRef.
You may wish to use a type ascription: `x: java.lang.Integer`.
no-implicit-to-anyref.scala:11: error: the result type of an implicit conversion must be more specific than AnyRef
1: AnyRef
^
no-implicit-to-anyref.scala:17: error: type mismatch;
Expand Down
16 changes: 2 additions & 14 deletions test/files/neg/t4158.check
Original file line number Diff line number Diff line change
@@ -1,19 +1,7 @@
t4158.scala:3: error: type mismatch;
found : Null(null)
required: Int
Note that implicit conversions are not applicable because they are ambiguous:
both method Integer2intNullConflict in class LowPriorityImplicits of type (x: Null)Int
and method Integer2int in object Predef of type (x: Integer)Int
are possible conversion functions from Null(null) to Int
t4158.scala:3: error: an expression of type Null is ineligible for implicit conversion
var y = null: Int
^
t4158.scala:2: error: type mismatch;
found : Null(null)
required: Int
Note that implicit conversions are not applicable because they are ambiguous:
both method Integer2intNullConflict in class LowPriorityImplicits of type (x: Null)Int
and method Integer2int in object Predef of type (x: Integer)Int
are possible conversion functions from Null(null) to Int
t4158.scala:2: error: an expression of type Null is ineligible for implicit conversion
var x: Int = null
^
two errors found
8 changes: 1 addition & 7 deletions test/files/neg/t4727.check
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
t4727.scala:5: error: type mismatch;
found : Null
required: Int
Note that implicit conversions are not applicable because they are ambiguous:
both method Integer2intNullConflict in class LowPriorityImplicits of type (x: Null)Int
and method Integer2int in object Predef of type (x: Integer)Int
are possible conversion functions from Null to Int
t4727.scala:5: error: an expression of type Null is ineligible for implicit conversion
Error occurred in an application involving default arguments.
new C[Int]
^
Expand Down
7 changes: 7 additions & 0 deletions test/files/neg/t6889.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
t6889.scala:16: error: the result type of an implicit conversion must be more specific than AnyRef
def f(x: Dingo): AnyRef = x // fail - no conversion to AnyRef
^
t6889.scala:17: error: an expression of type Null is ineligible for implicit conversion
var x: Int = null // fail - no conversion from Null
^
two errors found
18 changes: 18 additions & 0 deletions test/files/neg/t6889.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package bippy {
trait Bippy[A] extends Any
}
package foo {
package object unrelated {
implicit def bippyDingo[A](x: bippy.Bippy[A]): AnyRef = Nil
}
package unrelated {
trait Unrelated
}
}

object Test {
trait Dingo extends Any with bippy.Bippy[foo.unrelated.Unrelated]

def f(x: Dingo): AnyRef = x // fail - no conversion to AnyRef
var x: Int = null // fail - no conversion from Null
}

0 comments on commit 2f0e5ec

Please sign in to comment.