Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8303170

Generate "potentially ambiguous overload" for two inherited methods

    XMLWordPrintable

Details

    • CSR
    • Resolution: Approved
    • P4
    • 21
    • tools
    • None
    • behavioral
    • low
    • Hide
      Code having a "potentially ambiguous overload" caused by two methods that are both inherited will now generate a warning, whereas previously it did not. Also, a conflicting method inherited in two different subtypes will now generate warnings in all cases instead of just the first. These additional warnings could cause compilations with -Xlint:overloads and -Werror to newly fail.
      Show
      Code having a "potentially ambiguous overload" caused by two methods that are both inherited will now generate a warning, whereas previously it did not. Also, a conflicting method inherited in two different subtypes will now generate warnings in all cases instead of just the first. These additional warnings could cause compilations with -Xlint:overloads and -Werror to newly fail.
    • JDK

    Description

      Summary

      This change fixes an omission in the generation of the "potentially ambiguous overload" warning in which the warning is not generated if both methods are inherited.

      Problem

      When -Xlint:overloads is enabled, the compiler warns about certain ambiguities that can cause problems for lambdas. For example, consider the interface Spliterator.OfInt, which declares these two methods:

      void forEachRemaining(Consumer<? super Integer> action);
      void forEachRemaining(IntConsumer action);

      Both methods have the same name, same number of parameters, and take a lambda with the same "shape" in the same argument position. This causes an ambiguity in any code that wants to do this:

          spliterator.forEachRemaining(x -> { ... });

      That code won't compile; instead, you'll get this error:

      Ambiguity.java:4: error: reference to forEachRemaining is ambiguous
              spliterator.forEachRemaining(x -> { });
                         ^
        both method forEachRemaining(IntConsumer) in OfInt and method forEachRemaining(Consumer<? super Integer>) in OfInt match

      The omission being fixed here is that the compiler fails to detect ambiguities which are created purely by inheritance, for example:

      interface ConsumerOfInteger {
          void foo(Consumer<Integer> c);
      }
      
      interface IntegerConsumer {
          void foo(IntConsumer c);
      }
      
      // We should get a warning here...
      interface Test extends ConsumerOfInteger, IntegerConsumer {
      }

      This change corrects this omission. As a result, the same source code may generate more warnings than before.

      There are also two other minor related changes:

      (1) Previously, warning about any method forever excluded it from any future warnings. This meant inherited methods were disqualified from participating in the analysis of other classes that also inherit them.

      As a result, for a class like the one below, the compiler was only generating one warning instead of the expected three:

      public interface SuperIface {
          void foo(Consumer<Integer> c);
      }
      
      public interface I1 extends SuperIface {
          void foo(IntConsumer c);        // warning was generated here
      }
      
      public interface I2 extends SuperIface {
          void foo(IntConsumer c);        // no warning was generated here
      }
      
      public interface I3 extends SuperIface {
          void foo(IntConsumer c);        // no warning was generated here
      }

      This change will correct this omission. As a result, the same source code may generate more warnings than before.

      (2) For inherited methods, the method signatures were being reported as they are declared, rather than in the context of the class being visited. As a result, when a methods is inherited from a generic supertype, the ambiguity is less clear. Here's an example:

      interface Upper<T> {
          void foo(T c);
      }
      
      interface Lower extends Upper<IntConsumer> {
          void foo(Consumer<Integer> c);
      }

      Currently, the error is reported as:

      warning: [overloads] foo(Consumer<Integer>) in Lower is potentially ambiguous with foo(T) in Upper

      This change will cause the method signatures in the context of the class being visited makes the ambiguity clearer:

      warning: [overloads] foo(Consumer<Integer>) in Lower is potentially ambiguous with foo(IntConsumer) in Upper

      This change will improve the error message wording. As a result, the wording of existing warnings may change.

      Solution

      Improve the algorithm of the compiler so that even when both methods are inherited a warning will be generated, but otherwise preserving the existing behavior as much as possible.

      Specification

      Consistent with previous behavior, the warning strives to only report conflicts that only exist by virtue of the class being analyzed. In other words, we only want to "blame" a class when that class itself, and not some supertype, is responsible for creating the ambiguity. For example, any interface I extending Spliterator.OfInt will automatically inherit the two ambiguities mentioned above, but these are not I's fault (so to speak), so no warning should be generated for I.

      More precisely, we generate the warning when there are two methods m1 and m2 in a class or interface C such that:

      • m1 and m2 constitute a "potentially ambiguous overload" (using the same definition as before)
      • There is no direct supertype T of C such that m1 and m2, or some methods they override, both exist in T and constitute a "potentially ambiguous overload" as members of T
      • We haven't already generated a warning for either m1 or m2 in class C

      There is also a minor issue of the diagnostic position (i.e., source code location) of the warning: If either method is declared in C, we locate the warning there, but when both methods are inherited, there's no method declaration to point at so the warning is instead located at the class declaration (this second possibility is new).

      Notes

      See PR#12645, which includes:

      • Changes to the compiler
      • Newly uncovered warnings in the JDK itself (addressed with @SuppressWarnings)
      • Regression test demonstrating the new behavior

      Attachments

        Issue Links

          Activity

            People

              acobbs Archie Cobbs
              vromero Vicente Arturo Romero Zaldivar
              Vicente Arturo Romero Zaldivar
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: