Skip to content

Commit 5ba69e1

Browse files
author
Vicente Romero
committed
8322477: order of subclasses in the permits clause can differ between compilations
Reviewed-by: jlahoda
1 parent c96cbe4 commit 5ba69e1

File tree

9 files changed

+106
-39
lines changed

9 files changed

+106
-39
lines changed

‎src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import java.lang.annotation.Annotation;
2929
import java.lang.annotation.Inherited;
30+
import java.util.ArrayList;
3031
import java.util.Collections;
3132
import java.util.EnumSet;
3233
import java.util.HashMap;
@@ -1303,10 +1304,12 @@ public static class ClassSymbol extends TypeSymbol implements TypeElement {
13031304
// sealed classes related fields
13041305
/** The classes, or interfaces, permitted to extend this class, or interface
13051306
*/
1306-
public List<Symbol> permitted;
1307+
private java.util.List<PermittedClassWithPos> permitted;
13071308

13081309
public boolean isPermittedExplicit = false;
13091310

1311+
private record PermittedClassWithPos(Symbol permittedClass, int pos) {}
1312+
13101313
public ClassSymbol(long flags, Name name, Type type, Symbol owner) {
13111314
super(TYP, flags, name, type, owner);
13121315
this.members_field = null;
@@ -1315,7 +1318,7 @@ public ClassSymbol(long flags, Name name, Type type, Symbol owner) {
13151318
this.sourcefile = null;
13161319
this.classfile = null;
13171320
this.annotationTypeMetadata = AnnotationTypeMetadata.notAnAnnotationType();
1318-
this.permitted = List.nil();
1321+
this.permitted = new ArrayList<>();
13191322
}
13201323

13211324
public ClassSymbol(long flags, Name name, Symbol owner) {
@@ -1327,6 +1330,37 @@ public ClassSymbol(long flags, Name name, Symbol owner) {
13271330
this.type.tsym = this;
13281331
}
13291332

1333+
public void addPermittedSubclass(ClassSymbol csym, int pos) {
1334+
Assert.check(!isPermittedExplicit);
1335+
// we need to insert at the right pos
1336+
PermittedClassWithPos element = new PermittedClassWithPos(csym, pos);
1337+
int index = Collections.binarySearch(permitted, element, java.util.Comparator.comparing(PermittedClassWithPos::pos));
1338+
if (index < 0) {
1339+
index = -index - 1;
1340+
}
1341+
permitted.add(index, element);
1342+
}
1343+
1344+
public boolean isPermittedSubclass(Symbol csym) {
1345+
for (PermittedClassWithPos permittedClassWithPos : permitted) {
1346+
if (permittedClassWithPos.permittedClass.equals(csym)) {
1347+
return true;
1348+
}
1349+
}
1350+
return false;
1351+
}
1352+
1353+
public void clearPermittedSubclasses() {
1354+
permitted.clear();
1355+
}
1356+
1357+
public void setPermittedSubclasses(List<Symbol> permittedSubs) {
1358+
permitted.clear();
1359+
for (Symbol csym : permittedSubs) {
1360+
permitted.add(new PermittedClassWithPos(csym, 0));
1361+
}
1362+
}
1363+
13301364
/** The Java source which this symbol represents.
13311365
*/
13321366
public String toString() {
@@ -1643,7 +1677,7 @@ public boolean isRecord() {
16431677

16441678
@DefinedBy(Api.LANGUAGE_MODEL)
16451679
public List<Type> getPermittedSubclasses() {
1646-
return permitted.map(s -> s.type);
1680+
return permitted.stream().map(s -> s.permittedClass().type).collect(List.collector());
16471681
}
16481682
}
16491683

‎src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1701,7 +1701,7 @@ private boolean areDisjoint(ClassSymbol ts, ClassSymbol ss) {
17011701
// permitted subtypes have to be disjoint with the other symbol
17021702
ClassSymbol sealedOne = ts.isSealed() ? ts : ss;
17031703
ClassSymbol other = sealedOne == ts ? ss : ts;
1704-
return sealedOne.permitted.stream().allMatch(sym -> areDisjoint((ClassSymbol)sym, other));
1704+
return sealedOne.getPermittedSubclasses().stream().allMatch(type -> areDisjoint((ClassSymbol)type.tsym, other));
17051705
}
17061706
return false;
17071707
}

‎src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5383,58 +5383,58 @@ void attribClass(ClassSymbol c) throws CompletionFailure {
53835383
if (c.isSealed() &&
53845384
!c.isEnum() &&
53855385
!c.isPermittedExplicit &&
5386-
c.permitted.isEmpty()) {
5386+
c.getPermittedSubclasses().isEmpty()) {
53875387
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.SealedClassMustHaveSubclasses);
53885388
}
53895389

53905390
if (c.isSealed()) {
53915391
Set<Symbol> permittedTypes = new HashSet<>();
53925392
boolean sealedInUnnamed = c.packge().modle == syms.unnamedModule || c.packge().modle == syms.noModule;
5393-
for (Symbol subTypeSym : c.permitted) {
5393+
for (Type subType : c.getPermittedSubclasses()) {
53945394
boolean isTypeVar = false;
5395-
if (subTypeSym.type.getTag() == TYPEVAR) {
5395+
if (subType.getTag() == TYPEVAR) {
53965396
isTypeVar = true; //error recovery
5397-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5398-
Errors.InvalidPermitsClause(Fragments.IsATypeVariable(subTypeSym.type)));
5397+
log.error(TreeInfo.diagnosticPositionFor(subType.tsym, env.tree),
5398+
Errors.InvalidPermitsClause(Fragments.IsATypeVariable(subType)));
53995399
}
5400-
if (subTypeSym.isAnonymous() && !c.isEnum()) {
5401-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree), Errors.LocalClassesCantExtendSealed(Fragments.Anonymous));
5400+
if (subType.tsym.isAnonymous() && !c.isEnum()) {
5401+
log.error(TreeInfo.diagnosticPositionFor(subType.tsym, env.tree), Errors.LocalClassesCantExtendSealed(Fragments.Anonymous));
54025402
}
5403-
if (permittedTypes.contains(subTypeSym)) {
5403+
if (permittedTypes.contains(subType.tsym)) {
54045404
DiagnosticPosition pos =
54055405
env.enclClass.permitting.stream()
5406-
.filter(permittedExpr -> TreeInfo.diagnosticPositionFor(subTypeSym, permittedExpr, true) != null)
5406+
.filter(permittedExpr -> TreeInfo.diagnosticPositionFor(subType.tsym, permittedExpr, true) != null)
54075407
.limit(2).collect(List.collector()).get(1);
5408-
log.error(pos, Errors.InvalidPermitsClause(Fragments.IsDuplicated(subTypeSym.type)));
5408+
log.error(pos, Errors.InvalidPermitsClause(Fragments.IsDuplicated(subType)));
54095409
} else {
5410-
permittedTypes.add(subTypeSym);
5410+
permittedTypes.add(subType.tsym);
54115411
}
54125412
if (sealedInUnnamed) {
5413-
if (subTypeSym.packge() != c.packge()) {
5414-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5413+
if (subType.tsym.packge() != c.packge()) {
5414+
log.error(TreeInfo.diagnosticPositionFor(subType.tsym, env.tree),
54155415
Errors.ClassInUnnamedModuleCantExtendSealedInDiffPackage(c)
54165416
);
54175417
}
5418-
} else if (subTypeSym.packge().modle != c.packge().modle) {
5419-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5418+
} else if (subType.tsym.packge().modle != c.packge().modle) {
5419+
log.error(TreeInfo.diagnosticPositionFor(subType.tsym, env.tree),
54205420
Errors.ClassInModuleCantExtendSealedInDiffModule(c, c.packge().modle)
54215421
);
54225422
}
5423-
if (subTypeSym == c.type.tsym || types.isSuperType(subTypeSym.type, c.type)) {
5424-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, ((JCClassDecl)env.tree).permitting),
5423+
if (subType.tsym == c.type.tsym || types.isSuperType(subType, c.type)) {
5424+
log.error(TreeInfo.diagnosticPositionFor(subType.tsym, ((JCClassDecl)env.tree).permitting),
54255425
Errors.InvalidPermitsClause(
5426-
subTypeSym == c.type.tsym ?
5426+
subType.tsym == c.type.tsym ?
54275427
Fragments.MustNotBeSameClass :
5428-
Fragments.MustNotBeSupertype(subTypeSym.type)
5428+
Fragments.MustNotBeSupertype(subType)
54295429
)
54305430
);
54315431
} else if (!isTypeVar) {
5432-
boolean thisIsASuper = types.directSupertypes(subTypeSym.type)
5432+
boolean thisIsASuper = types.directSupertypes(subType)
54335433
.stream()
54345434
.anyMatch(d -> d.tsym == c);
54355435
if (!thisIsASuper) {
5436-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5437-
Errors.InvalidPermitsClause(Fragments.DoesntExtendSealed(subTypeSym.type)));
5436+
log.error(TreeInfo.diagnosticPositionFor(subType.tsym, env.tree),
5437+
Errors.InvalidPermitsClause(Fragments.DoesntExtendSealed(subType)));
54385438
}
54395439
}
54405440
}
@@ -5469,7 +5469,7 @@ void attribClass(ClassSymbol c) throws CompletionFailure {
54695469

54705470
if (!c.type.isCompound()) {
54715471
for (ClassSymbol supertypeSym : sealedSupers) {
5472-
if (!supertypeSym.permitted.contains(c.type.tsym)) {
5472+
if (!supertypeSym.isPermittedSubclass(c.type.tsym)) {
54735473
log.error(TreeInfo.diagnosticPositionFor(c.type.tsym, env.tree), Errors.CantInheritFromSealed(supertypeSym));
54745474
}
54755475
}

‎src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -945,8 +945,8 @@ private Set<Symbol> allPermittedSubTypes(ClassSymbol root, Predicate<ClassSymbol
945945
current.complete();
946946

947947
if (current.isSealed() && current.isAbstract()) {
948-
for (Symbol sym : current.permitted) {
949-
ClassSymbol csym = (ClassSymbol) sym;
948+
for (Type t : current.getPermittedSubclasses()) {
949+
ClassSymbol csym = (ClassSymbol) t.tsym;
950950

951951
if (accept.test(csym)) {
952952
permittedSubtypesClosure = permittedSubtypesClosure.prepend(csym);

‎src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,7 @@ private void fillPermits(JCClassDecl tree, Env<AttrContext> baseEnv) {
919919
!supClass.isPermittedExplicit &&
920920
supClassEnv != null &&
921921
supClassEnv.toplevel == baseEnv.toplevel) {
922-
supClass.permitted = supClass.permitted.append(sym);
922+
supClass.addPermittedSubclass(sym, tree.pos);
923923
}
924924
}
925925
}
@@ -932,7 +932,7 @@ private void fillPermits(JCClassDecl tree, Env<AttrContext> baseEnv) {
932932
Type pt = attr.attribBase(permitted, baseEnv, false, false, false);
933933
permittedSubtypeSymbols.append(pt.tsym);
934934
}
935-
sym.permitted = permittedSubtypeSymbols.toList();
935+
sym.setPermittedSubclasses(permittedSubtypeSymbols.toList());
936936
}
937937
}
938938
}

‎src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,7 @@ protected void read(Symbol sym, int attrLen) {
12981298
for (int i = 0; i < numberOfPermittedSubtypes; i++) {
12991299
subtypes.add(poolReader.getClass(nextChar()));
13001300
}
1301-
((ClassSymbol)sym).permitted = subtypes.toList();
1301+
((ClassSymbol)sym).setPermittedSubclasses(subtypes.toList());
13021302
}
13031303
}
13041304
},
@@ -2930,7 +2930,7 @@ void readClass(ClassSymbol c) {
29302930
for (int i = 0; i < methodCount; i++) skipMember();
29312931
readClassAttrs(c);
29322932

2933-
if (c.permitted != null && !c.permitted.isEmpty()) {
2933+
if (!c.getPermittedSubclasses().isEmpty()) {
29342934
c.flags_field |= SEALED;
29352935
}
29362936

‎src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -922,11 +922,11 @@ private void listNested(Symbol sym, ListBuffer<ClassSymbol> seen) {
922922
/** Write "PermittedSubclasses" attribute.
923923
*/
924924
int writePermittedSubclassesIfNeeded(ClassSymbol csym) {
925-
if (csym.permitted.nonEmpty()) {
925+
if (csym.getPermittedSubclasses().nonEmpty()) {
926926
int alenIdx = writeAttr(names.PermittedSubclasses);
927-
databuf.appendChar(csym.permitted.size());
928-
for (Symbol c : csym.permitted) {
929-
databuf.appendChar(poolWriter.putClass((ClassSymbol) c));
927+
databuf.appendChar(csym.getPermittedSubclasses().size());
928+
for (Type t : csym.getPermittedSubclasses()) {
929+
databuf.appendChar(poolWriter.putClass((ClassSymbol) t.tsym));
930930
}
931931
endAttr(alenIdx);
932932
return 1;

‎src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1643,7 +1643,7 @@ public void visitClassDef(JCClassDecl node) {
16431643
originalAnnos.forEach(a -> visitAnnotation(a));
16441644
}
16451645
// we should empty the list of permitted subclasses for next round
1646-
node.sym.permitted = List.nil();
1646+
node.sym.clearPermittedSubclasses();
16471647
}
16481648
node.sym = null;
16491649
}

‎test/langtools/tools/javac/sealed/SealedDiffConfigurationsTest.java‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,6 @@ private void checkSealedClassFile(Path out, String cfName, List<String> expected
142142
} catch (ConstantPoolException ex) {
143143
}
144144
});
145-
subtypeNames.sort((s1, s2) -> s1.compareTo(s2));
146145
for (int i = 0; i < expectedSubTypeNames.size(); i++) {
147146
Assert.check(expectedSubTypeNames.get(0).equals(subtypeNames.get(0)));
148147
}
@@ -695,4 +694,38 @@ public void testSupertypePermitsLoop(Path base) throws Exception {
695694
.run()
696695
.writeAll();
697696
}
697+
698+
@Test
699+
public void testClientSwapsPermittedSubclassesOrder(Path base) throws Exception {
700+
Path src = base.resolve("src");
701+
Path foo = src.resolve("Foo.java");
702+
Path fooUser = src.resolve("FooUser.java");
703+
704+
tb.writeFile(foo,
705+
"""
706+
public sealed interface Foo {
707+
record R1() implements Foo {}
708+
record R2() implements Foo {}
709+
}
710+
""");
711+
712+
tb.writeFile(fooUser,
713+
"""
714+
public class FooUser {
715+
// see that the order of arguments differ from the order of subclasses of Foo in the source above
716+
// we need to check that the order of permitted subclasses of Foo in the class file corresponds to the
717+
// original order in the source code
718+
public void blah(Foo.R2 a, Foo.R1 b) {}
719+
}
720+
""");
721+
722+
Path out = base.resolve("out");
723+
Files.createDirectories(out);
724+
725+
new JavacTask(tb)
726+
.outdir(out)
727+
.files(fooUser, foo)
728+
.run();
729+
checkSealedClassFile(out, "Foo.class", List.of("Foo$R1", "Foo$R2"));
730+
}
698731
}

0 commit comments

Comments
 (0)