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

3 files changed, 106 insertions, 1 deletions

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