Skip to content

Fix implicit conversion warnings #5886

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 12 commits into from
Feb 11, 2019
Merged
36 changes: 25 additions & 11 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -634,17 +634,31 @@ trait Checking {
* - it is defined in Predef
* - it is the scala.reflect.Selectable.reflectiveSelectable conversion
*/
def checkImplicitConversionUseOK(sym: Symbol, posd: Positioned)(implicit ctx: Context): Unit = {
val conversionOK =
!sym.exists ||
sym.is(Synthetic) ||
sym.info.finalResultType.classSymbols.exists(_.owner.isLinkedWith(sym.owner)) ||
defn.isPredefClass(sym.owner) ||
sym.name == nme.reflectiveSelectable && sym.maybeOwner.maybeOwner.maybeOwner == defn.ScalaPackageClass
if (!conversionOK)
checkFeature(defn.LanguageModuleClass, nme.implicitConversions,
i"Use of implicit conversion ${sym.showLocated}", NoSymbol, posd.sourcePos)
}
def checkImplicitConversionUseOK(sym: Symbol, posd: Positioned)(implicit ctx: Context): Unit =
if (sym.exists) {
val conv =
if (sym.is(Implicit)) sym
else {
assert(sym.name == nme.apply)
sym.owner
}
val conversionOK =
conv.is(Synthetic) ||
sym.info.finalResultType.classSymbols.exists(_.isLinkedWith(conv.owner)) ||
defn.isPredefClass(conv.owner) ||
conv.name == nme.reflectiveSelectable && conv.maybeOwner.maybeOwner.maybeOwner == defn.ScalaPackageClass
if (!conversionOK) {
val symToShow =
if (sym.is(Implicit)) sym
else {
assert(sym.name == nme.apply)
sym.owner
}
val clsSyms = sym.info.finalResultType.classSymbols.map(_.linkedClass)
checkFeature(defn.LanguageModuleClass, nme.implicitConversions,
i"Use of implicit conversion ${conv.showLocated}", NoSymbol, posd.sourcePos)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

symToShow and clsSyms are not used.


/** Issue a feature warning if feature is not enabled */
def checkFeature(base: ClassSymbol,
Expand Down
2 changes: 2 additions & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ class CompilationTests extends ParallelTesting {
compileFilesInDir("tests/neg-custom-args/fatal-warnings", defaultOptions.and("-Xfatal-warnings")) +
compileFilesInDir("tests/neg-custom-args/allow-double-bindings", allowDoubleBindings) +
compileDir("tests/neg-custom-args/impl-conv", defaultOptions.and("-Xfatal-warnings", "-feature")) +
compileFile("tests/neg-custom-args/implicit-conversions.scala", defaultOptions.and("-Xfatal-warnings", "-feature")) +
compileFile("tests/neg-custom-args/implicit-conversions-old.scala", defaultOptions.and("-Xfatal-warnings", "-feature")) +
compileFile("tests/neg-custom-args/i3246.scala", scala2Mode) +
compileFile("tests/neg-custom-args/overrideClass.scala", scala2Mode) +
compileFile("tests/neg-custom-args/autoTuplingTest.scala", defaultOptions.and("-language:noAutoTupling")) +
Expand Down
37 changes: 29 additions & 8 deletions library/src/scalaShadowing/language.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,10 @@ object language {
*/
@volatile implicit lazy val reflectiveCalls: reflectiveCalls = languageFeature.reflectiveCalls

/** Only where enabled, definitions of implicit conversions are allowed. An
* implicit conversion is an implicit value of unary function type `A => B`,
/** Only where enabled, definitions of legacy implicit conversions and certain uses
* of implicit conversions are allowed.
*
* A legacy implicit conversion is an implicit value of unary function type `A => B`,
* or an implicit method that has in its first parameter section a single,
* non-implicit parameter. Examples:
*
Expand All @@ -103,17 +105,36 @@ object language {
* implicit def listToX(xs: List[T])(implicit f: T => X): X = ...
* }}}
*
* implicit values of other types are not affected, and neither are implicit
* classes.
* Implicit values of other types are not affected, and neither are implicit
* classes. In particular, implied instances of the scala.Conversion class can be
* defined without having to import the language feature.
*
* The language import is also required to enable _uses_ of implicit conversions
* unless the conversion in question is co-defined with the type to which it maps.
* Co-defined means: defined in the companion object of the class of the result type.
* Examples:
*
* {{{
* class A
* class B
* object B {
* implied a2b for Conversion[A, B] { ... }
* }
* object C {
* implied b2a for Conversion[B, A] { ... }
* }
* import implied B._
* import implied C._
* val x: A = new B // language import required
* val x: B = new A // no import necessary since a2b is co-defined with B
* }}}
*
* '''Why keep the feature?''' Implicit conversions are central to many aspects
* of Scala’s core libraries.
*
* '''Why control it?''' Implicit conversions are known to cause many pitfalls
* if over-used. And there is a tendency to over-use them because they look
* very powerful and their effects seem to be easy to understand. Also, in
* most situations using implicit parameters leads to a better design than
* implicit conversions.
* if over-used. This holds in particular for implicit conversions defined after
* the fact between unrelated types.
*
* @group production
*/
Expand Down
23 changes: 23 additions & 0 deletions tests/neg-custom-args/implicit-conversions-old.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
class A
class B

object A {

implicit def a2b(x: A): B = ??? // error under -Xfatal-warnings -feature

implicit def b2a(x: B): A = ??? // error under -Xfatal-warnings -feature
}

class C

object D {
implicit def a2c(x: A): C = ??? // error under -Xfatal-warnings -feature
}

object Test {
import D._

val x1: A = new B
val x2: B = new A // error under -Xfatal-warnings -feature
val x3: C = new A // error under -Xfatal-warnings -feature
}
29 changes: 29 additions & 0 deletions tests/neg-custom-args/implicit-conversions.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
class A
class B

object A {

implied for Conversion[A, B] {
def apply(x: A): B = ???
}

implied for Conversion[B, A] {
def apply(x: B): A = ???
}
}

class C

object D {
implied for Conversion[A, C] {
def apply(x: A): C = ???
}
}

object Test {
import implied D._

val x1: A = new B
val x2: B = new A // error under -Xfatal-warnings -feature
val x3: C = new A // error under -Xfatal-warnings -feature
}