RFR: JDK-8315248: AssertionError in Name.compareTo

[Jonathan Gibbons]

On Wed, 30 Aug 2023 00:25:52 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

> Please review a small fix to avoid an `AssertionError` coming out of `CharSequence.compare` for two `CharSequence` objects obtained from different instances of the `javax.lang.model` API.
> 
> Full details in the JBS issue and comments.
> 
> There are some minor cleanup edits as well, and some improvements to the code to throw AssertionError in a few places.

> This definitely works. However, if a class `X` implements `Comparable`, by definition instances must be able to `compareTo()` themselves with any other instance of type `X` (even if `X` is an interface - so this is a pretty major imposition that any class or interface implementing `Comparable` imposes on all subtypes, present, past, and future).
> 
> In this case, it is `Name` that implements `Comparable`, and `Utf8NameTable.NameImpl`'s version of `compareTo()` is just as an overriding optimization of `Name.compareTo()`, which does in fact work in all cases.
> 
> So it seems like the "truly correct" thing to do here would be something like this:
> 
> ```java
> // Utf8NameTable.NameImpl.compareTo()
> @Override
> public int compareTo(Name name0) {
>     NameImpl name;
>     try {
>         name = (NameImpl)name0;
>     } catch (ClassCastException e) {
>         return super.compareTo(name0);
>     }
>     byte[] buf1 = getByteData();
>     ...etc...
> ```
> 
> This probably doesn't matter in practice, but just wanted to point out the theoretical argument.

Noted; thanks. Since we're editing the code, I'll fix it.
It's notable that the test works with all combinations of `Name` but it is protected by the code in `CharSequence.compare`

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15478#issuecomment-1699367173