On Thu, 14 Mar 2024 16:56:03 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> It is well described in a [JBS >> comment](https://bugs.openjdk.org/browse/JDK-8305072?focusedId=14610400&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14610400) >> by the submitter. >> >> Let's consider the failing case where: >> >> a = C:\Users<user>\Documents(true) >> b = C:\Users<user>(false) >> c = C:\Users<user>\Documents(false) >> >> The value in parentheses is `isSpecial()` flag. >> >> When `a` and `c` are compared, `special1 = true` and `special2 = false`, >> therefore the code flow enters this `if`-block. Because the `equals` method >> doesn't take into account the `isSpecial` flag, both `i1` and `i2` are 0. >> >> https://github.com/openjdk/jdk/blob/7bccc847fe9e8c5e640d8fa9c93ce32563a0ccfa/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolderManager2.java#L534-L535 >> >> Thus, `a` and `c` are considered equal, which breaks the general contract of >> [`Comparable.compareTo`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Comparable.html#compareTo(T)). >> >> If you look below this `if`-statement: >> >> https://github.com/openjdk/jdk/blob/7bccc847fe9e8c5e640d8fa9c93ce32563a0ccfa/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolderManager2.java#L545-L550 >> >> The comment and the code say that any special folder sorts above any regular >> folder. Therefore the code shouldn't enter that `if`-block above if *only >> one* of the folders is *special* — the order is already well-defined for >> such case. >> >> The purpose of the `if`-block is to make some of the special folders even >> more special. In other words, it ensures the following special >> folders—`getPersonal`, `getDesktop`, `getDrives`, `getNetwork`—are at the >> top of the list, above any other special folders. > > So, changing the condition in the `if`-block from `special1 || special2` to > `special1 && special2` makes the sorting order consistent because the > relation is now *transitive*: > > > a < b & b < c therefore a < c Hi, I'm the submitter of the original bug report. I can confirm that you got the reason for the change exactly right. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18126#discussion_r1526422961
