From bf7eee08892aa38130dda75cfb6099fdc8b5bcd4 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Wed, 26 May 2010 11:52:11 +0000 Subject: also consider non-implicit locals when checking... also consider non-implicit locals when checking shadowing of implicits: closes #3453 nonImplicitSynonymInScope implements the predicate that is used in tryImplicit's checks for shadowing of locally defined implicits benchmarking shows the predicate does not significantly affect quick.comp+quick.lib (goes from 11min to 11min2s on my machine -- no optimisations) review by odersky --- .../scala/tools/nsc/typechecker/Implicits.scala | 20 ++++++- test/files/neg/t3453.check | 21 +++++++ test/files/neg/t3453.scala | 66 ++++++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/t3453.check create mode 100644 test/files/neg/t3453.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala index 20d367337f..3abaf4f337 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala @@ -571,6 +571,19 @@ self: Analyzer => /** A set containing names that are shadowed by implicit infos */ lazy val shadowed = new HashSet[Name]("shadowed", 512) + // #3453 + // in addition to the implicit symbols that may shadow the implicit with name `name`, + // this method tests whether there's a non-implicit symbol with name `name` in scope + // inspired by logic in typedIdent + def nonImplicitSynonymInScope(name: Name) = { + val defEntry = context.scope.lookupEntry(name) + (defEntry ne null) && + reallyExists(defEntry.sym) && + !defEntry.sym.isImplicit // the implicit ones are handled by the `shadowed` set above + // also, subsumes the test that defEntry.sym ne info.sym + // (the `info` that's in scope at the call to nonImplicitSynonymInScope in tryImplicit) + } + /** Is `sym' the standard conforms method in Predef? * Note: DON't replace this by sym == Predef_conforms, as Predef_conforms is a `def' * which does a member lookup (it can't be a lazy val because we might reload Predef @@ -592,7 +605,7 @@ self: Analyzer => def tryImplicit(info: ImplicitInfo): SearchResult = { incCounter(triedImplicits) if (info.isCyclicOrErroneous || - (isLocal && shadowed.contains(info.name)) || + (isLocal && (shadowed.contains(info.name) || nonImplicitSynonymInScope(info.name))) || (isView && isConformsMethod(info.sym)) || //@M this condition prevents no-op conversions, which are a problem (besides efficiency), // one example is removeNames in NamesDefaults, which relies on the type checker failing in case of ambiguity between an assignment/named arg @@ -615,6 +628,11 @@ self: Analyzer => applicable } + // #3453 -- alternative fix, seems not to be faster than encoding the set as the boolean predicate nonImplicitSynonymInScope + // in addition to the *implicit* symbols that may shadow the implicit with name `name` (added to shadowed by addAppInfos) + // add names of non-implicit symbols that are in scope (accessible without prefix) + // for(sym <- context.scope; if !sym.isImplicit) shadowed addEntry sym.name + var applicable = Map[ImplicitInfo, SearchResult]() for (is <- iss) applicable = addAppInfos(is, applicable) diff --git a/test/files/neg/t3453.check b/test/files/neg/t3453.check new file mode 100644 index 0000000000..52c948128c --- /dev/null +++ b/test/files/neg/t3453.check @@ -0,0 +1,21 @@ +t3453.scala:18: error: type mismatch; + found : A + required: B + new A + ^ +t3453.scala:36: error: type mismatch; + found : A + required: B + new A + ^ +t3453.scala:50: error: type mismatch; + found : A + required: B + new A + ^ +t3453.scala:64: error: type mismatch; + found : A + required: B + new A + ^ +four errors found diff --git a/test/files/neg/t3453.scala b/test/files/neg/t3453.scala new file mode 100644 index 0000000000..090b777151 --- /dev/null +++ b/test/files/neg/t3453.scala @@ -0,0 +1,66 @@ +// test shadowing of implicits by synonymous non-implicit symbols +// whether they be inherited, imported (explicitly or using a wildcard) or defined directly +class A +class B + +trait S { + implicit def aToB(a: A): B = new B +} + +class T1 extends S { + def x: B = { + val aToB = 3 + // ok: doesn't compile, because aToB method requires 'T.this.' prefix + //aToB(new A) + + // bug: compiles, using T.this.aToB, + // despite it not being accessible without a prefix + new A + } +} + +object O { + implicit def aToB(a: A): B = new B +} + +class T2a { + import O._ + + def x: B = { + val aToB = 3 + // ok: doesn't compile, because aToB method requires 'T.this.' prefix + //aToB(new A) + + // bug: compiles, using T.this.aToB, + // despite it not being accessible without a prefix + new A + } +} + +class T2b { + import O.aToB + + def x: B = { + val aToB = 3 + // ok: doesn't compile, because aToB method requires 'T.this.' prefix + //aToB(new A) + + // bug: compiles, using T.this.aToB, + // despite it not being accessible without a prefix + new A + } +} + +class T3 { + implicit def aToB(a: A): B = new B + + def x: B = { + val aToB = 3 + // ok: doesn't compile, because aToB method requires 'T.this.' prefix + //aToB(new A) + + // bug: compiles, using T.this.aToB, + // despite it not being accessible without a prefix + new A + } +} \ No newline at end of file -- cgit v1.2.3