Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

SI-6899, prohibit dangerous, useless implicit conversions. #2625

Merged
merged 1 commit into from

6 participants

@paulp

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.

@paulp paulp SI-6899, prohibit dangerous, useless implicit conversions.
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.
2f0e5ec
@xeno-by xeno-by commented on the diff
test/files/neg/t4158.check
@@ -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
@xeno-by Owner
xeno-by added a note

how about reformulating this in a bit lighter manner, e.g. nulls are ineligible for implicit conversion?

@paulp
paulp added a note

I find that misleading, because they're not, and not everything being prohibited here is a null.

// That's not a null.
scala> def f[T <: Null](x: T): Int = x
<console>:7: error: an expression of type Null is ineligible for implicit conversion
       def f[T <: Null](x: T): Int = x
                                     ^
// That IS a null.
scala> def g: Seq[Int] = (null: Array[Int])
g: Seq[Int]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@soc
soc commented

Although I'm concerned about introducing more special cases, this seems to be a huge win. LGTM.

@retronym
Owner

LGTM. But I think we should run this one past @odersky this week, too.

@gkossakowski

@retronym: did you have a chance to show this to @odersky? If not, what do we do about it?

@retronym
Owner

Nope, I forgot. But it definitely needs wider review. I'll email @odersky to see if I can evade his spam filter...

@gkossakowski

Ok. I'll try to help to get your message through.

@retronym
Owner

Oh right, he's still down your way. I designate you as the messenger!

@gkossakowski

Ok, I took the :envelope: and I'll see what can I do about it!

@adriaanm
Owner

LGTM by @odersky acquired by me

@gkossakowski gkossakowski merged commit e2aaf80 into scala:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 4, 2013
  1. @paulp

    SI-6899, prohibit dangerous, useless implicit conversions.

    paulp authored
    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.
This page is out of date. Refresh to see the latest.
View
28 src/compiler/scala/tools/nsc/typechecker/Implicits.scala
@@ -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()
@@ -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)
@@ -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
}
View
4 src/library/scala/Option.scala
@@ -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.
@@ -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.
View
31 src/library/scala/Predef.scala
@@ -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
@@ -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)
View
8 test/files/neg/no-implicit-to-anyref.check
@@ -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;
View
16 test/files/neg/t4158.check
@@ -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
@xeno-by Owner
xeno-by added a note

how about reformulating this in a bit lighter manner, e.g. nulls are ineligible for implicit conversion?

@paulp
paulp added a note

I find that misleading, because they're not, and not everything being prohibited here is a null.

// That's not a null.
scala> def f[T <: Null](x: T): Int = x
<console>:7: error: an expression of type Null is ineligible for implicit conversion
       def f[T <: Null](x: T): Int = x
                                     ^
// That IS a null.
scala> def g: Seq[Int] = (null: Array[Int])
g: Seq[Int]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
View
8 test/files/neg/t4727.check
@@ -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]
^
View
7 test/files/neg/t6889.check
@@ -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
View
18 test/files/neg/t6889.scala
@@ -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
+}
Something went wrong with that request. Please try again.