From 78d917393ea03ef94f892549f87cbc2cabba8ac6 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 23 Feb 2017 15:44:37 -0800 Subject: SI-10206 tighten fix for SI-6889 There are more supertypes of `AnyRef` than you might think: `?{def clone: ?}` is one example... --- .../scala/tools/nsc/typechecker/Implicits.scala | 41 ++++++++++++---------- test/files/neg/t6889.check | 7 ++-- test/files/neg/t6889.scala | 1 + 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala index 509ce59104..2333c29b30 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala @@ -134,11 +134,6 @@ trait Implicits { private val improvesCache = perRunCaches.newMap[(ImplicitInfo, ImplicitInfo), Boolean]() private val implicitSearchId = { var id = 1 ; () => try id finally id += 1 } - private def isInvalidConversionSource(tpe: Type): Boolean = tpe match { - case Function1(in, _) => in <:< NullClass.tpe - case _ => false - } - def resetImplicits() { implicitsCache.clear() infoMapCache.clear() @@ -1388,27 +1383,32 @@ trait Implicits { } } if (result.isSuccess && isView) { - def maybeInvalidConversionError(msg: String) { + def maybeInvalidConversionError(msg: String): Boolean = { // We have to check context.ambiguousErrors even though we are calling "issueAmbiguousError" // which ostensibly does exactly that before issuing the error. Why? I have no idea. Test is pos/t7690. // AM: I would guess it's because ambiguous errors will be buffered in silent mode if they are not reported if (context.ambiguousErrors) context.issueAmbiguousError(AmbiguousImplicitTypeError(tree, msg)) + true } pt match { - case Function1(_, out) => - // must inline to avoid capturing result - def prohibit(sym: Symbol) = (sym.tpe <:< out) && { - maybeInvalidConversionError(s"the result type of an implicit conversion must be more specific than ${sym.name}") - true - } - if (prohibit(AnyRefClass) || (settings.isScala211 && prohibit(AnyValClass))) - result = SearchFailure - case _ => false - } - if (settings.isScala211 && isInvalidConversionSource(pt)) { - maybeInvalidConversionError("an expression of type Null is ineligible for implicit conversion") - result = SearchFailure + // SI-10206 don't use subtyping to rule out AnyRef/AnyVal: + // - there are several valid structural types that are supertypes of AnyRef (e.g., created by HasMember); + // typeSymbol will do the trick (AnyRef is a type alias for Object), while ruling out these structural types + // - also don't want to accidentally constrain type vars through using <:< + case Function1(in, out) => + val outSym = out.typeSymbol + + val fail = + if (out.annotations.isEmpty && (outSym == ObjectClass || (isScala211 && outSym == AnyValClass))) + maybeInvalidConversionError(s"the result type of an implicit conversion must be more specific than $out") + else if (isScala211 && in.annotations.isEmpty && in.typeSymbol == NullClass) + maybeInvalidConversionError("an expression of type Null is ineligible for implicit conversion") + else false + + if (fail) result = SearchFailure + + case _ => } } @@ -1418,6 +1418,9 @@ trait Implicits { result } + // this setting is expensive to check, actually.... + private[this] val isScala211 = settings.isScala211 + def allImplicits: List[SearchResult] = { def search(iss: Infoss, isLocalToCallsite: Boolean) = applicableInfos(iss, isLocalToCallsite).values ( diff --git a/test/files/neg/t6889.check b/test/files/neg/t6889.check index a77e8a010c..c14c3b09c0 100644 --- a/test/files/neg/t6889.check +++ b/test/files/neg/t6889.check @@ -1,7 +1,10 @@ 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 +t6889.scala:17: error: the result type of an implicit conversion must be more specific than Object + def f2(x: Dingo): Object = x // fail - no conversion to Object + ^ +t6889.scala:18: error: an expression of type Null is ineligible for implicit conversion var x: Int = null // fail - no conversion from Null ^ -two errors found +three errors found diff --git a/test/files/neg/t6889.scala b/test/files/neg/t6889.scala index ef1963669c..3fc235bf7e 100644 --- a/test/files/neg/t6889.scala +++ b/test/files/neg/t6889.scala @@ -14,5 +14,6 @@ object Test { trait Dingo extends Any with bippy.Bippy[foo.unrelated.Unrelated] def f(x: Dingo): AnyRef = x // fail - no conversion to AnyRef + def f2(x: Dingo): Object = x // fail - no conversion to Object var x: Int = null // fail - no conversion from Null } -- cgit v1.2.3 From 849d09b9e10d025df7f91494315f927f68bc9dd6 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 24 Feb 2017 09:37:20 +1000 Subject: Test case for SI-10206 Implicit search implements a restriction that the target type for an implicit view should be more specific that AnyRef or AnyVal. scala> def foo(s: String): AnyVal = s :12: error: the result type of an implicit conversion must be more specific than AnyVal def foo(s: String): AnyVal = s ^ Without this, an implicit value classes over `String` would be applied, which is unlikely to be what was intended. Implicit views are implemented as an implicit search for a function type with a structural type as its result. This structural type is created with: scala> val schema = analyzer.HasMethodMatching.apply(TermName("clone"), Nil, WildcardType) schema: $r.intp.global.analyzer.global.Type = ?{def clone(): ?} The quirk arises when, as above, we're seeking a member with the same name as a member of AnyRef. AnyRef is seen to be a subtype of the result type: scala> AnyRefClass.tpe <:< schema res23: Boolean = true Which leads to the implicit in the test case being disqualified. The typer opts to report the error about the inapplicability of the inherited clone method, so we don't even know why the implicit was discarded. --- test/files/pos/t10206.scala | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test/files/pos/t10206.scala diff --git a/test/files/pos/t10206.scala b/test/files/pos/t10206.scala new file mode 100644 index 0000000000..3ddd1ea2fd --- /dev/null +++ b/test/files/pos/t10206.scala @@ -0,0 +1,15 @@ +class Foo(val bar: String) + +object Foo { + implicit class Enrich(foo: Foo) { + def clone(x: Int, y: Int): Int = x + y + } +} + +object Main extends App { + val foo = new Foo("hello") + println(foo.clone(1, 2)) // <- does not compile + // the implicit view was being disqualified because a new check in the compiler + // that implicit views must not target Any or AnyRef considered an implicit search + // for `foo.type => ?{def clone: ?}` to targeted AnyRef. +} -- cgit v1.2.3