SortedMultiset.elementSet should return NavigableSet

[gissuebot]

Original issue created by wasserman.louis on 2012-03-17 at 08:15 PM

---

Related to issue 664. I'd prefer not to go down the route of the JDK in having separate Sorted/Navigable interfaces, but I'd like SortedMultiset.elementSet to return NavigableSet.

Would this break anyone's code? How can we migrate users?

[gissuebot]

Original comment posted by kevinb@google.com on 2012-03-18 at 04:12 AM

---

Changing a return type is ordinarily a binary-compatibility problem, but here I think we might get away with it because it's already a narrowed return type from the parent type (Multiset), so there's already a bridge method being created!

---

Status: Accepted

[gissuebot]

Original comment posted by wasserman.louis on 2012-03-18 at 12:35 PM

---

Normally we might have an issue with changing an interface spec, but I'm hoping that nobody's actually gone and implemented SortedMultiset for themselves...

[gissuebot]

Original comment posted by cpovirk@google.com on 2012-03-22 at 04:18 PM

---

GWT is a problem here, since it doesn't have a NavigableSet. We could add one, but it may bloat GWT code size. It's on my plate to analyze just how much and make a decision.

Louis has sent http://codereview.appspot.com/5867050/ (which otherwise LGTM).

[gissuebot]

Original comment posted by kevinb@google.com on 2012-03-22 at 04:32 PM

---

re:comment 2, as an interface it's @Beta. It's just the TreeMultiset implementation that's non-beta and really at issue here.

[gissuebot]

Original comment posted by wasserman.louis on 2012-03-22 at 04:40 PM

---

Hrrrrm.

I'd like to change the interface, ASAP, before Jane Random Coder decides to implement it herself and then gets broken by the change.

That said, I could settle for speccing TreeMultiset to return NavigableSet, and leaving the interface as-is for the moment. It strikes me that that would be easier, just in terms of GWT compatibility -- for example, we could have the GWT implementation spec'd to only return a SortedSet...I don't know if that'd work, but it might reduce the GWT complexity.

[gissuebot]

Original comment posted by cpovirk@google.com on 2012-03-22 at 06:01 PM

---

$ tail -n 999 *.java && javac *.java && javap SortedMultiset && javap TreeMultiset && javap -c Caller
==> Caller.java <==
public class Caller
{
  static void multiset() {
    Multiset m = new TreeMultiset();
    m.elementSet();
  }
  static void sortedMultiset() {
    SortedMultiset m = new TreeMultiset();
    m.elementSet();
  }
  static void treeMultiset() {
    TreeMultiset m = new TreeMultiset();
    m.elementSet();
  }
  public static void main(String[] args) {
    multiset();
    sortedMultiset();
    treeMultiset();
  }
}

==> Multiset.java <==
public interface Multiset {
  Set elementSet();
}

==> NavigableSet.java <==
public interface NavigableSet extends SortedSet {}

==> Set.java <==
public interface Set {}

==> SortedMultiset.java <==
public interface SortedMultiset extends Multiset {
  SortedSet elementSet();
}

==> SortedSet.java <==
public interface SortedSet extends Set {}

==> TreeMultiset.java <==
public class TreeMultiset implements SortedMultiset {
  public SortedSet elementSet() {
    return null;
  }
}
Compiled from "SortedMultiset.java"
public interface SortedMultiset extends Multiset{
    public abstract SortedSet elementSet();
}

Compiled from "TreeMultiset.java"
public class TreeMultiset extends java.lang.Object implements SortedMultiset{
    public TreeMultiset();
    public SortedSet elementSet();
    public Set elementSet();
}

Compiled from "Caller.java"
public class Caller extends java.lang.Object{
public Caller();
  Code:
   0: aload_0
   1: invokespecial #1; //Method java/lang/Object."<init>":()V
   4: return

static void multiset();
  Code:
   0: new #2; //class TreeMultiset
   3: dup
   4: invokespecial #3; //Method TreeMultiset."<init>":()V
   7: astore_0
   8: aload_0
   9: invokeinterface #4, 1; //InterfaceMethod Multiset.elementSet:()LSet;
   14: pop
   15: return

static void sortedMultiset();
  Code:
   0: new #2; //class TreeMultiset
   3: dup
   4: invokespecial #3; //Method TreeMultiset."<init>":()V
   7: astore_0
   8: aload_0
   9: invokeinterface #5, 1; //InterfaceMethod SortedMultiset.elementSet:()LSortedSet;
   14: pop
   15: return

static void treeMultiset();
  Code:
   0: new #2; //class TreeMultiset
   3: dup
   4: invokespecial #3; //Method TreeMultiset."<init>":()V
   7: astore_0
   8: aload_0
   9: invokevirtual #6; //Method TreeMultiset.elementSet:()LSortedSet;
   12: pop
   13: return

public static void main(java.lang.String[]);
  Code:
   0: invokestatic #7; //Method multiset:()V
   3: invokestatic #8; //Method sortedMultiset:()V
   6: invokestatic #9; //Method treeMultiset:()V
   9: return

}

If we make TreeMultiset return NavigableSet, we get the same methods, except that "SortedSet elementSet()" is now a bridge method (fine) and of course "NavigableSet elementSet()" exists (also fine). If I make this change, recompile only the Multiset classes, and run Caller, everything is fine.

If we make both TreeMultiset and SortedMultiset return NavigableSet, we see the SortedSet-returning method in each going away in favor of a NavigableSet-returning method (not good). If I make this change, recompile only the Multiset classes, and run Caller, I get an error:

$ javac Multiset.java Set.java && java Caller
Exception in thread "main" java.lang.NoSuchMethodError: SortedMultiset.elementSet()LSortedSet;
        at Caller.sortedMultiset(Caller.java:9)
        at Caller.main(Caller.java:17)

I should note that the problem exists with the call through the TreeMultiset API, too, not just through the @Beta SortedMultiset API. I confirmed this by commenting out the sortedMultiset() call:

Exception in thread "main" java.lang.NoSuchMethodError: TreeMultiset.elementSet()LSortedSet;
        at Caller.treeMultiset(Caller.java:13)
        at Caller.main(Caller.java:18)

I conclude that we can change TreeMultiset but not SortedMultiset -- ironic, given the @Beta situation.

Unless, of course, we're clever....

$ tail -n 999 Multiset.java
==> Multiset.java <==
public interface Multiset {
  Set elementSet();
}

==> SortedMultisetBridgeInjector.java <==
interface SortedMultisetBridgeInjector extends Multiset {
  SortedSet elementSet();
}

==> SortedMultiset.java <==
public interface SortedMultiset extends SortedMultisetBridgeInjector {
  NavigableSet elementSet();
}

==> TreeMultiset.java <==
public class TreeMultiset implements SortedMultiset {
  public NavigableSet elementSet() {
    return null;
  }
}

This works.

$ javap TreeMultiset
Compiled from "TreeMultiset.java"
public class TreeMultiset extends java.lang.Object implements SortedMultiset{
    public TreeMultiset();
    public NavigableSet elementSet();
    public SortedSet elementSet();
    public Set elementSet();
}

$ java Caller

[gissuebot]

Original comment posted by wasserman.louis on 2012-03-26 at 01:24 PM

---

I like the "clever solution."

[gissuebot]

Original comment posted by kevinb@google.com on 2012-04-03 at 05:40 PM

---

(No comment entered for this change.)

---

Labels: Type-Enhancement

[gissuebot]

Original comment posted by lowasser@google.com on 2012-12-10 at 05:25 PM

---

(No comment entered for this change.)

---

Status: Fixed
Labels: Milestone-Release14