8074803: Name clash

Javac incorrectly reports a name clash.

Reviewed-by: mcimadamore

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java

@@ -220,11 +220,7 @@ public class Flags {
      */
     public static final long UNION = 1L<<39;
 
-    /**
+    // Flag bit (1L << 40) is available.
-     * Flag that marks a special kind of bridge method (the ones that
-     * come from restricted supertype bounds).
-     */
-    public static final long OVERRIDE_BRIDGE = 1L<<40;
 
     /**
      * Flag that marks an 'effectively final' local variable.
@@ -383,7 +379,6 @@ public class Flags {
         HYPOTHETICAL(Flags.HYPOTHETICAL),
         PROPRIETARY(Flags.PROPRIETARY),
         UNION(Flags.UNION),
-        OVERRIDE_BRIDGE(Flags.OVERRIDE_BRIDGE),
         EFFECTIVELY_FINAL(Flags.EFFECTIVELY_FINAL),
         CLASH(Flags.CLASH),
         AUXILIARY(Flags.AUXILIARY),

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java

@@ -1528,9 +1528,33 @@ public abstract class Symbol extends AnnoConstruct implements Element {
          *  It is assumed that both symbols have the same name.  The static
          *  modifier is ignored for this test.
          *
+         *  A quirk in the works is that if the receiver is a method symbol for
+         *  an inherited abstract method we answer false summarily all else being
+         *  immaterial. Abstract "own" methods (i.e `this' is a direct member of
+         *  origin) don't get rejected as summarily and are put to test against the
+         *  suitable criteria.
+         *
          *  See JLS 8.4.6.1 (without transitivity) and 8.4.6.4
          */
         public boolean overrides(Symbol _other, TypeSymbol origin, Types types, boolean checkResult) {
+            return overrides(_other, origin, types, checkResult, true);
+        }
+
+        /** Does this symbol override `other' symbol, when both are seen as
+         *  members of class `origin'?  It is assumed that _other is a member
+         *  of origin.
+         *
+         *  Caveat: If `this' is an abstract inherited member of origin, it is
+         *  deemed to override `other' only when `requireConcreteIfInherited'
+         *  is false.
+         *
+         *  It is assumed that both symbols have the same name.  The static
+         *  modifier is ignored for this test.
+         *
+         *  See JLS 8.4.6.1 (without transitivity) and 8.4.6.4
+         */
+        public boolean overrides(Symbol _other, TypeSymbol origin, Types types, boolean checkResult,
+                                            boolean requireConcreteIfInherited) {
             if (isConstructor() || _other.kind != MTH) return false;
 
             if (this == _other) return true;
@@ -1550,7 +1574,7 @@ public abstract class Symbol extends AnnoConstruct implements Element {
             }
 
             // check for an inherited implementation
-            if ((flags() & ABSTRACT) != 0 ||
+            if (((flags() & ABSTRACT) != 0 && requireConcreteIfInherited) ||
                     ((other.flags() & ABSTRACT) == 0 && (other.flags() & DEFAULT) == 0) ||
                     !other.isOverridableIn(origin) ||
                     !this.isMemberOf(origin, types))

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java

@@ -86,7 +86,7 @@ public class TransTypes extends TreeTranslator {
         log = Log.instance(context);
         syms = Symtab.instance(context);
         enter = Enter.instance(context);
-        overridden = new HashMap<>();
+        bridgeSpans = new HashMap<>();
         types = Types.instance(context);
         make = TreeMaker.instance(context);
         resolve = Resolve.instance(context);
@@ -96,10 +96,12 @@ public class TransTypes extends TreeTranslator {
         annotate = Annotate.instance(context);
     }
 
-    /** A hashtable mapping bridge methods to the methods they override after
+    /** A hashtable mapping bridge methods to the pair of methods they bridge.
-     *  type erasure.
+     *  The bridge overrides the first of the pair after type erasure and deflects
+     *  to the second of the pair (which differs in type erasure from the one
+     *  it overrides thereby necessitating the bridge)
      */
-    Map<MethodSymbol,MethodSymbol> overridden;
+    Map<MethodSymbol, Pair<MethodSymbol, MethodSymbol>> bridgeSpans;
 
     /** Construct an attributed tree for a cast of expression to target type,
      *  unless it already has precisely that type.
@@ -294,9 +296,9 @@ public class TransTypes extends TreeTranslator {
             bridges.append(md);
         }
 
-        // Add bridge to scope of enclosing class and `overridden' table.
+        // Add bridge to scope of enclosing class and keep track of the bridge span.
         origin.members().enter(bridge);
-        overridden.put(bridge, meth);
+        bridgeSpans.put(bridge, new Pair<>(meth, impl));
     }
 
     private List<VarSymbol> createBridgeParams(MethodSymbol impl, MethodSymbol bridge,
@@ -344,12 +346,12 @@ public class TransTypes extends TreeTranslator {
         if (sym.kind == MTH &&
             sym.name != names.init &&
             (sym.flags() & (PRIVATE | STATIC)) == 0 &&
-            (sym.flags() & (SYNTHETIC | OVERRIDE_BRIDGE)) != SYNTHETIC &&
+            (sym.flags() & SYNTHETIC) != SYNTHETIC &&
             sym.isMemberOf(origin, types))
         {
             MethodSymbol meth = (MethodSymbol)sym;
             MethodSymbol bridge = meth.binaryImplementation(origin, types);
-            MethodSymbol impl = meth.implementation(origin, types, true, overrideBridgeFilter);
+            MethodSymbol impl = meth.implementation(origin, types, true);
             if (bridge == null ||
                 bridge == meth ||
                 (impl != null && !bridge.owner.isSubClass(impl.owner, types))) {
@@ -365,14 +367,19 @@ public class TransTypes extends TreeTranslator {
                     // reflection design error.
                     addBridge(pos, meth, impl, origin, false, bridges);
                 }
-            } else if ((bridge.flags() & (SYNTHETIC | OVERRIDE_BRIDGE)) == SYNTHETIC) {
+            } else if ((bridge.flags() & SYNTHETIC) == SYNTHETIC) {
-                MethodSymbol other = overridden.get(bridge);
+                final Pair<MethodSymbol, MethodSymbol> bridgeSpan = bridgeSpans.get(bridge);
+                MethodSymbol other = bridgeSpan == null ? null : bridgeSpan.fst;
                 if (other != null && other != meth) {
                     if (impl == null || !impl.overrides(other, origin, types, true)) {
-                        // Bridge for other symbol pair was added
+                        // Is bridge effectively also the bridge for `meth', if so no clash.
-                        log.error(pos, "name.clash.same.erasure.no.override",
+                        MethodSymbol target = bridgeSpan == null ? null : bridgeSpan.snd;
-                                  other, other.location(origin.type, types),
+                        if (target == null || !target.overrides(meth, origin, types, true, false)) {
-                                  meth,  meth.location(origin.type, types));
+                            // Bridge for other symbol pair was added
+                            log.error(pos, "name.clash.same.erasure.no.override",
+                                    other, other.location(origin.type, types),
+                                    meth, meth.location(origin.type, types));
+                        }
                     }
                 }
             } else if (!bridge.overrides(meth, origin, types, true)) {
@@ -388,11 +395,6 @@ public class TransTypes extends TreeTranslator {
         }
     }
     // where
-        private Filter<Symbol> overrideBridgeFilter = new Filter<Symbol>() {
-            public boolean accepts(Symbol s) {
-                return (s.flags() & (SYNTHETIC | OVERRIDE_BRIDGE)) != SYNTHETIC;
-            }
-        };
 
         /**
          * @param method The symbol for which a bridge might have to be added

langtools/test/tools/javac/NameClash/NameClashTest.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2015 Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8074803
+ * @summary Incorrect name clash error
+ *
+ * @compile NameClashTest.java
+ */
+
+public class NameClashTest {
+
+    String log = "";
+
+    interface A1 {
+        A1 m(String s);
+    }
+
+    abstract class A2 implements A1 {
+        public abstract A2 m(String s);
+    }
+
+    interface B1 {
+        A1 m(String s);
+    }
+
+    interface B2 extends B1 {
+        A2 m(String s);
+    }
+
+    abstract class C extends A2 implements B2 {}
+
+    class D extends C {
+
+        public A2 m(String s) {
+            log += s;
+            return null;
+        }
+    }
+
+    public static void main(String[] args) {
+        NameClashTest nct = new NameClashTest();
+        A1 a1 = nct.new D();
+        a1.m("A1.m ");
+        A2 a2 = nct.new D();
+        a2.m("A2.m ");
+        B1 b1 = nct.new D();
+        b1.m("B1.m ");
+        B2 b2 = nct.new D();
+        b2.m("B2.m ");
+        C c = nct.new D();
+        c.m("C.m ");
+        D d = nct.new D();
+        d.m("D.m ");
+        if (!nct.log.equals("A1.m A2.m B1.m B2.m C.m D.m "))
+            throw new AssertionError("unexpected output: " + nct.log);
+    }
+}