RFR: 8322477: order of subclasses in the permits clause can differ between compilations [v3]

[Jan Lahoda]

On Wed, 10 Jan 2024 03:48:42 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> This is a very interesting issue. Given code like:
>> 
>> sealed interface Sealed {
>>     record R1() implements Sealed {}
>>     record R2() implements Sealed {}
>> }
>> 
>> 
>> As we know javac will infer the `permits` clause of sealed interface `Sealed` logically the order should correspond to the order in which the permitted subclasses appear in the source code. Well it has been consistently observed by the reported of this bug, that some tools like Gradle while doing incremental compilation can make javac infer either `R1, R2` or `R2, R1` as permitted subclasses. The reason is not clear still under investigation on their side but the fact is that javac is generating inconsistent output for some classes with this shape. The proposed solution is to store the position of the permitted subclasses being discovered by javac so that the order of the permitted subclasses corresponds to the original order in the source file. Efforts to reduce the project where the issue was discovered to a small reproductor have been unsuccessful but the proposed patch have fixed the issue observed by the reporter.
>> 
>> TIA
>
> Vicente Romero has updated the pull request incrementally with one additional commit since the last revision:
> 
>   updating regression test

While it might be nicer to have the order implied, I can see that's not really easy to achieve in case of arbitrary order of completion. So, this looks OK to me. One suggestion for consideration slightly simplify the code. No re-review needed if the suggestion is accepted.

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java line 1334:

> 1332: 
> 1333:         public void addPermittedSubclass(ClassSymbol csym, int pos) {
> 1334:             if (isPermittedExplicit) {

If I understand it properly, the codepaths to add implicitly and explicitly declared permitted subclasses are completely different. And when the permitted subclasses are explicit, the `addPermittedSubclass` won't be called.

For consideration: just using `Assert.check(!isPermittedExplicit)`, drop the if.

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

Marked as reviewed by jlahoda (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17284#pullrequestreview-1813636466
PR Review Comment: https://git.openjdk.org/jdk/pull/17284#discussion_r1447632286