Skip to content
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

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

Merged
merged 1 commit into from Jun 19, 2013
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 23 additions & 5 deletions src/compiler/scala/tools/nsc/typechecker/Implicits.scala
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
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
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
@@ -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
@@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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]

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
@@ -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
@@ -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
@@ -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
}