Skip to content

Commit fd7baff

Browse files
authored
Merge pull request #4229 from dotty-staging/change-implicit-convs
Emit feature warnings for implicit conversions.
2 parents 4ed97e3 + 1f69d70 commit fd7baff

File tree

10 files changed

+91
-13
lines changed

10 files changed

+91
-13
lines changed

compiler/src/dotty/tools/dotc/core/StdNames.scala

+2
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,7 @@ object StdNames {
436436
val head: N = "head"
437437
val higherKinds: N = "higherKinds"
438438
val identity: N = "identity"
439+
val implicitConversions: N = "implicitConversions"
439440
val implicitly: N = "implicitly"
440441
val in: N = "in"
441442
val info: N = "info"
@@ -488,6 +489,7 @@ object StdNames {
488489
val productPrefix: N = "productPrefix"
489490
val readResolve: N = "readResolve"
490491
val reflect : N = "reflect"
492+
val reflectiveSelectable: N = "reflectiveSelectable"
491493
val reify : N = "reify"
492494
val rootMirror : N = "rootMirror"
493495
val run: N = "run"

compiler/src/dotty/tools/dotc/reporting/Reporter.scala

+7-4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import core.Decorators.PhaseListDecorator
88
import collection.mutable
99
import java.lang.System.currentTimeMillis
1010
import core.Mode
11-
import dotty.tools.dotc.core.Symbols.Symbol
11+
import dotty.tools.dotc.core.Symbols.{Symbol, NoSymbol}
1212
import diagnostic.messages._
1313
import diagnostic._
1414
import Message._
@@ -39,7 +39,7 @@ trait Reporting { this: Context =>
3939
def reportWarning(warning: Warning): Unit =
4040
if (!this.settings.silentWarnings.value) {
4141
if (this.settings.XfatalWarnings.value) reporter.report(warning.toError)
42-
else reporter.report(warning)
42+
else reporter.report(warning)
4343
}
4444

4545
def deprecationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
@@ -170,7 +170,10 @@ abstract class Reporter extends interfaces.ReporterResult {
170170
def errorsReported = hasErrors
171171

172172
private[this] var reportedFeaturesUseSites = Set[Symbol]()
173-
def isReportedFeatureUseSite(featureTrait: Symbol): Boolean = reportedFeaturesUseSites.contains(featureTrait)
173+
174+
def isReportedFeatureUseSite(featureTrait: Symbol): Boolean =
175+
featureTrait.ne(NoSymbol) && reportedFeaturesUseSites.contains(featureTrait)
176+
174177
def reportNewFeatureUseSite(featureTrait: Symbol): Unit = reportedFeaturesUseSites += featureTrait
175178

176179
val unreportedWarnings = new mutable.HashMap[String, Int] {
@@ -227,7 +230,7 @@ abstract class Reporter extends interfaces.ReporterResult {
227230
ctx.mode.is(Mode.Printing)
228231

229232
/** Does this reporter contain not yet reported errors or warnings? */
230-
def hasPending: Boolean = false
233+
def hasPendingErrors: Boolean = false
231234

232235
/** If this reporter buffers messages, remove and return all buffered messages. */
233236
def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] = Nil

compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala

+2-7
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,8 @@ class StoreReporter(outer: Reporter) extends Reporter {
2828
infos += m
2929
}
3030

31-
override def hasPending: Boolean = infos != null && {
32-
infos exists {
33-
case _: Error => true
34-
case _: Warning => true
35-
case _ => false
36-
}
37-
}
31+
override def hasPendingErrors: Boolean =
32+
infos != null && infos.exists(_.isInstanceOf[Error])
3833

3934
override def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] =
4035
if (infos != null) try infos.toList finally infos = null

compiler/src/dotty/tools/dotc/typer/Checking.scala

+46
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,49 @@ trait Checking {
577577
case _ =>
578578
}
579579

580+
/** If `sym` is an implicit conversion, check that implicit conversions are enabled.
581+
* @pre sym.is(Implicit)
582+
*/
583+
def checkImplicitConversionDefOK(sym: Symbol)(implicit ctx: Context): Unit = sym.info.stripPoly match {
584+
case mt @ MethodType(_ :: Nil)
585+
if !mt.isImplicitMethod && !sym.is(Synthetic) => // it's a conversion
586+
checkFeature(
587+
defn.LanguageModuleClass, nme.implicitConversions,
588+
i"Definition of implicit conversion $sym",
589+
ctx.owner.topLevelClass,
590+
sym.pos)
591+
case _ =>
592+
}
593+
594+
/** If `sym` is an implicit conversion, check that that implicit conversions are enabled, unless
595+
* - it is synthetic
596+
* - it is has the same owner as one of the classes it converts to (modulo companions)
597+
* - it is defined in Predef
598+
* - it is the scala.reflect.Selectable.reflectiveSelectable conversion
599+
*/
600+
def checkImplicitConversionUseOK(sym: Symbol, pos: Position)(implicit ctx: Context): Unit = {
601+
val conversionOK =
602+
!sym.exists ||
603+
sym.is(Synthetic) ||
604+
sym.info.finalResultType.classSymbols.exists(_.owner.isLinkedWith(sym.owner)) ||
605+
defn.isPredefClass(sym.owner) ||
606+
sym.name == nme.reflectiveSelectable && sym.maybeOwner.maybeOwner.maybeOwner == defn.ScalaPackageClass
607+
if (!conversionOK)
608+
checkFeature(defn.LanguageModuleClass, nme.implicitConversions,
609+
i"Use of implicit conversion ${sym.showLocated}", NoSymbol, pos)
610+
}
611+
612+
/** Issue a feature warning if feature is not enabled */
613+
def checkFeature(base: ClassSymbol,
614+
name: TermName,
615+
description: => String,
616+
featureUseSite: Symbol,
617+
pos: Position)(implicit ctx: Context): Unit =
618+
if (!ctx.featureEnabled(base, name))
619+
ctx.featureWarning(name.toString, description,
620+
isScala2Feature = base.isContainedIn(defn.LanguageModuleClass),
621+
featureUseSite, required = false, pos)
622+
580623
/** Check that `tp` is a class type and that any top-level type arguments in this type
581624
* are feasible, i.e. that their lower bound conforms to their upper bound. If a type
582625
* argument is infeasible, issue and error and continue with upper bound.
@@ -870,6 +913,8 @@ trait NoChecking extends ReChecking {
870913
override def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit = ()
871914
override def checkClassType(tp: Type, pos: Position, traitReq: Boolean, stablePrefixReq: Boolean)(implicit ctx: Context): Type = tp
872915
override def checkImplicitParamsNotSingletons(vparamss: List[List[ValDef]])(implicit ctx: Context): Unit = ()
916+
override def checkImplicitConversionDefOK(sym: Symbol)(implicit ctx: Context): Unit = ()
917+
override def checkImplicitConversionUseOK(sym: Symbol, pos: Position)(implicit ctx: Context): Unit = ()
873918
override def checkFeasibleParent(tp: Type, pos: Position, where: => String = "")(implicit ctx: Context): Type = tp
874919
override def checkInlineConformant(tree: Tree, isFinal: Boolean, what: => String)(implicit ctx: Context) = ()
875920
override def checkNoDoubleDeclaration(cls: Symbol)(implicit ctx: Context): Unit = ()
@@ -881,4 +926,5 @@ trait NoChecking extends ReChecking {
881926
override def checkCaseInheritance(parentSym: Symbol, caseCls: ClassSymbol, pos: Position)(implicit ctx: Context) = ()
882927
override def checkNoForwardDependencies(vparams: List[ValDef])(implicit ctx: Context): Unit = ()
883928
override def checkMembersOK(tp: Type, pos: Position)(implicit ctx: Context): Type = tp
929+
override def checkFeature(base: ClassSymbol, name: TermName, description: => String, featureUseSite: Symbol, pos: Position)(implicit ctx: Context): Unit = ()
884930
}

compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala

+6-1
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,12 @@ object ProtoTypes {
249249
targ = arg.withType(WildcardType)
250250
else {
251251
targ = typerFn(arg)
252-
if (!ctx.reporter.hasPending) {
252+
if (!ctx.reporter.hasPendingErrors) {
253+
// FIXME: This can swallow warnings by updating the typerstate from a nested
254+
// context that gets discarded later. But we do have to update the
255+
// typerstate if there are no errors. If we also omitted the next two lines
256+
// when warning were emitted, `pos/t1756.scala` would fail when run with -feature.
257+
// It would produce an orphan type parameter for CI when pickling.
253258
myTypedArg = myTypedArg.updated(arg, targ)
254259
evalState = evalState.updated(arg, (ctx.typerState, ctx.typerState.constraint))
255260
}

compiler/src/dotty/tools/dotc/typer/Typer.scala

+5-1
Original file line numberDiff line numberDiff line change
@@ -1405,7 +1405,10 @@ class Typer extends Namer
14051405
val tparams1 = tparams mapconserve (typed(_).asInstanceOf[TypeDef])
14061406
val vparamss1 = vparamss nestedMapconserve (typed(_).asInstanceOf[ValDef])
14071407
vparamss1.foreach(checkNoForwardDependencies)
1408-
if (sym is Implicit) checkImplicitParamsNotSingletons(vparamss1)
1408+
if (sym is Implicit) {
1409+
checkImplicitParamsNotSingletons(vparamss1)
1410+
checkImplicitConversionDefOK(sym)
1411+
}
14091412
val tpt1 = checkSimpleKinded(typedType(tpt))
14101413

14111414
var rhsCtx = ctx
@@ -2441,6 +2444,7 @@ class Typer extends Namer
24412444
if (ctx.mode.is(Mode.ImplicitsEnabled))
24422445
inferView(tree, pt) match {
24432446
case SearchSuccess(inferred, _, _) =>
2447+
checkImplicitConversionUseOK(inferred.symbol, tree.pos)
24442448
readapt(inferred)(ctx.retractMode(Mode.ImplicitsEnabled))
24452449
case failure: SearchFailure =>
24462450
if (pt.isInstanceOf[ProtoType] && !failure.isAmbiguous)

compiler/test/dotty/tools/dotc/CompilationTests.scala

+1
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ class CompilationTests extends ParallelTesting {
178178
compileFilesInDir("tests/neg-kind-polymorphism", defaultOptions and "-Ykind-polymorphism") +
179179
compileFilesInDir("tests/neg-custom-args/fatal-warnings", defaultOptions.and("-Xfatal-warnings")) +
180180
compileFilesInDir("tests/neg-custom-args/allow-double-bindings", allowDoubleBindings) +
181+
compileDir("tests/neg-custom-args/impl-conv", defaultOptions.and("-Xfatal-warnings", "-feature")) +
181182
compileFile("tests/neg-custom-args/i3246.scala", scala2Mode) +
182183
compileFile("tests/neg-custom-args/overrideClass.scala", scala2Mode) +
183184
compileFile("tests/neg-custom-args/autoTuplingTest.scala", defaultOptions.and("-language:noAutoTupling")) +

compiler/test/dotty/tools/dotc/reporting/UserDefinedErrorMessages.scala

+1
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ class UserDefinedErrorMessages extends ErrorMessagesTest {
8888
checkMessagesAfter("frontend") {
8989
"""
9090
|class C {
91+
| import scala.language.implicitConversions
9192
| @annotation.implicitAmbiguous("msg A=${A}")
9293
| implicit def f[A](x: Int): String = "f was here"
9394
| implicit def g(x: Int): String = "f was here"
+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package implConv
2+
3+
object A {
4+
5+
implicit def s2i(x: String): Int = Integer.parseInt(x) // error: feature
6+
7+
implicit class Foo(x: String) {
8+
def foo = x.length
9+
}
10+
11+
}
+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package implConv
2+
3+
object B {
4+
import A._
5+
6+
"".foo
7+
8+
val x: Int = "" // error: feature
9+
10+
}

0 commit comments

Comments
 (0)