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

(reflect) getMethods returns default methods that are not members of the class

    Details

      Backports

        Description

        Dan writes:

        ---

        To illustrate, the JDK 7 implementation of getMethods includes the following:
        1) The class's/interface's declared (public) methods
        2) The getMethods() of the superclass (if this is a class), minus any that have a match in (1)
        3) The getMethods() of each superinterface, minus any that have a match in (1) or a _concrete_ match in (2)

        So, for example, as expected:
        interface I { void m(); }
        interface J extends I { void m(); }
        interface K extends J {}
        K.class.getMethods() = [ J.m ]

        But:
        interface I { void m(); }
        interface J extends I { void m(); }
        interface K extends ***I,*** J {}
        K.class.getMethods() = [ I.m, J.m ]

        This is consistent with a reasonable reading of JLS 7 (though it is vague)*, but conflicts with JLS 8. We changed the 8 spec because, unlike abstract methods, inheriting two conflicting default methods is a compiler error, and that should be avoided.

        [* There was a time when I thought the current behavior could be considered a 7 bug, but having experimented with it more, I no longer think that's the case. This is a new issue in 8.]

        An implementation of getMethods consistent with JLS 8 would include the following (see Lambda Spec, Part H, 9.4.1 and 8.4.8):
        1) The class's/interface's declared (public) methods
        2) The getMethods() of the superclass (if this is a class), minus any that have a match in (1)
        3) The getMethods() of each superinterface, minus any that have a match in (1) or a _concrete_ match in (2) ***or a match from a more-specific class/interface in (2) or (3)***

        However, there is a compatibility concern: the behavior of K.class.getMethods in the second example above changes. How much do we care about this? The spec change was made with the understanding that this was simply an internal change to an intermediate result, without observable consequences. Reflection exposes that intermediate result. But, even so, client code should not be depending on the rather vague notion of how many identical abstract methods are members of a class.

        ---

        That is, the spec changed in 8 but the implementation (which was correct up through 7) has not changed.

        The proposed fix for default methods as outlined by Dan:

        ---

        Given the sets of (1) declared methods, (2) superclass member methods, and (3) superinterface member methods, prune as follows:
        a) Remove methods from (2) and (3) that have a match in (1)
        b) Remove methods from (3) that have a _concrete_ match in (2)
        c) Remove methods declared in _interfaces_ from (2) and (3) that have a more-specific _default_ method in (2) or (3)

        The new logic is (c). We still allow a default method and its overriding abstract method to coexist in the same type, but these kinds of mixtures can already happen in a limited way (see Lambda Spec, 8.4.8.4), so that seems tolerable.

        I feel a little more comfortable with this particular ad hoc strategy given the guiding principle that we just want to ensure that methods that don't need to be implemented are excluded. (Thus, no need to consider all possible hierarchies and make arbitrary choices about correct behavior in each case.)

        This still has an impact on legacy clients (an abstract method might be made default, and then only one method turns up instead of two), but it's a narrow enough case that we should probably consider it far less risky.

        ---

        and:

        ---

        How do we feel about the compatibility risk of getMethod returning a different result that i) has the same name and descriptor, and ii) overrides the previously-returned result?

        Arguments for why this is okay:
        - The interface search order is completely unspecified and implementation-dependent
        - Depending on the declaring class/interface of a Method, when you got that method from a search like this, is crazy

        Arguments for why this is bad:
        - The accessibility of the method depends on its declaring interface (if that interface is package-access), possibly prompting an access error (but javac-generated bridges make this a very rare circumstance)

        Unlike getMethods, there is no incompatible change here (i.e., the new results are valid under the old javadoc), just a refinement of what results are considered acceptable.

        ---


          Attachments

            Issue Links

            There are no Sub-Tasks for this issue.

              Activity

                People

                • Assignee:
                  jfranck Joel Borggrén-Franck
                  Reporter:
                  jfranck Joel Borggrén-Franck
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  7 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: