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

Changing "arbitrary" Name.compareTo() ordering breaks the regression suite

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • P4
    • 21
    • None
    • tools
    • jdk-21+11-25-g2fb1e3b7e72

    • b16

    Description

      This was discovered during investigation for JDK-8268622.

      The method Name.compareTo() looks like this:

          /** An arbitrary but consistent complete order among all Names.
           */
          public int compareTo(Name other) {
              return other.getIndex() - this.getIndex();
          }

      Unfortunately, the ordering can be "arbitrary" only if you don't mind breaking the regression suite.

      For example, if you apply this patch:

          --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Name.java
          +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Name.java
          @@ -105,7 +105,7 @@ public abstract class Name implements javax.lang.model.element.Name, PoolConstan
               /** An arbitrary but consistent complete order among all Names.
                */
               public int compareTo(Name other) {
          - return other.getIndex() - this.getIndex();
          + return this.getIndex() - other.getIndex();
               }

               /** Return true if this is the empty name.

      then the following tests will fail:

          TEST: tools/javac/8203436/T8203436b.java
          TEST: tools/javac/generics/7034019/T7034019c.java
          TEST: tools/javac/generics/7034019/T7034019d.java
          TEST: tools/javac/generics/diamond/neg/Neg21.java
          TEST: tools/javac/generics/diamond/neg/Neg22.java
          TEST: tools/javac/generics/inference/EagerReturnTypeResolution/EagerReturnTypeResolutionTestb.java
          TEST: tools/javac/patterns/InferenceUnitTest.java
          TEST: tools/javac/T8187978/FilterOutCandidatesForDiagnosticsTest.java
          TEST: tools/javac/varargs/6806876/T6806876.java

          ==============================
          Test summary
          ==============================
             TEST TOTAL PASS FAIL ERROR
          >> jtreg:test/langtools:langtools_javac 3718 3709 9 0 <<
          ==============================

      The javac regression suite should be robust against changes to something that can supposedly be "arbitrary".

      It turns out there is only one user of Name.compareTo(): the method TypeSymbol.precedes():

              /**
               * A partial ordering between type symbols that refines the
               * class inheritance graph.
               *
               * Type variables always precede other kinds of symbols.
               */
              public final boolean precedes(TypeSymbol that, Types types) {
                  if (this == that)
                      return false;
                  if (type.hasTag(that.type.getTag())) {
                      if (type.hasTag(CLASS)) {
                          return
                              types.rank(that.type) < types.rank(this.type) ||
                              types.rank(that.type) == types.rank(this.type) &&
                              that.getQualifiedName().compareTo(this.getQualifiedName()) < 0;
                      } else if (type.hasTag(TYPEVAR)) {
                          return types.isSubtype(this.type, that.type);
                      }
                  }
                  return type.hasTag(TYPEVAR);
              }

      If we want the above partial ordering to be stable, we need to specify how Name.compareTo() compares.

      An obvious choice would be to require that Name.compareTo() orders consistently with String.compareTo(), i.e., lexicographically on the Unicode characters in the name.

      Fortunately, this comparison can be done efficiently (i.e., without actually converting the Name into a String) because UTF-8 is "lexicographically consistent" with the characters it encodes, although Java's use of Modified UTF-8 means a special check for 0x0000 (which is encoded as two bytes) will be required.

      Attachments

        Issue Links

          Activity

            People

              acobbs Archie Cobbs
              acobbs Archie Cobbs
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: