Skip to content

Commit

Permalink
Mark enums that cannot be extended as final
Browse files Browse the repository at this point in the history
Previously they were always recorded as `final`, which interfered with a downstream j2cl optimization.

Enums are still never recorded as `abstract`, which would require more analysis of supertypes and overrides to see if there were any un-implemented abstract methods, which is out of scope for turbine.

PiperOrigin-RevId: 638463654
  • Loading branch information
cushon authored and Javac Team committed May 30, 2024
1 parent fed74bf commit 70a8b3c
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 15 deletions.
39 changes: 30 additions & 9 deletions java/com/google/turbine/binder/CompUnitPreprocessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public static PreprocessedCompUnit preprocess(CompUnit unit) {
for (TyDecl decl : decls) {
ClassSymbol sym =
new ClassSymbol((!packageName.isEmpty() ? packageName + "/" : "") + decl.name());
int access = access(decl.mods(), decl.tykind());
int access = access(decl.mods(), decl);
ImmutableMap<String, ClassSymbol> children =
preprocessChildren(unit.source(), types, sym, decl.members(), access);
types.add(new SourceBoundClass(sym, null, children, access, decl));
Expand Down Expand Up @@ -167,24 +167,27 @@ private static ImmutableMap<String, ClassSymbol> preprocessChildren(
}

/** Desugars access flags for a class. */
public static int access(ImmutableSet<TurbineModifier> mods, TurbineTyKind tykind) {
public static int access(ImmutableSet<TurbineModifier> mods, TyDecl decl) {
int access = 0;
for (TurbineModifier m : mods) {
access |= m.flag();
}
switch (tykind) {
switch (decl.tykind()) {
case CLASS:
access |= TurbineFlag.ACC_SUPER;
break;
case INTERFACE:
access |= TurbineFlag.ACC_ABSTRACT | TurbineFlag.ACC_INTERFACE;
break;
case ENUM:
// Assuming all enums are final is safe, because nothing outside
// the compilation unit can extend abstract enums anyways, and
// refactoring an existing enum to implement methods in the container
// class instead of the constants is not a breaking change.
access |= TurbineFlag.ACC_SUPER | TurbineFlag.ACC_ENUM | TurbineFlag.ACC_FINAL;
// Assuming all enums are non-abstract is safe, because nothing outside
// the compilation unit can extend abstract enums, and refactoring an
// existing enum to implement methods in the container class instead
// of the constants is not a breaking change.
access |= TurbineFlag.ACC_SUPER | TurbineFlag.ACC_ENUM;
if (isEnumFinal(decl.members())) {
access |= TurbineFlag.ACC_FINAL;
}
break;
case ANNOTATION:
access |= TurbineFlag.ACC_ABSTRACT | TurbineFlag.ACC_INTERFACE | TurbineFlag.ACC_ANNOTATION;
Expand All @@ -196,9 +199,27 @@ public static int access(ImmutableSet<TurbineModifier> mods, TurbineTyKind tykin
return access;
}

/**
* If any enum constants have a class body (which is recorded in the parser by setting ENUM_IMPL),
* the class generated for the enum needs to not have ACC_FINAL set.
*/
private static boolean isEnumFinal(ImmutableList<Tree> declMembers) {
for (Tree t : declMembers) {
if (t.kind() != Tree.Kind.VAR_DECL) {
continue;
}
Tree.VarDecl var = (Tree.VarDecl) t;
if (!var.mods().contains(TurbineModifier.ENUM_IMPL)) {
continue;
}
return false;
}
return true;
}

/** Desugars access flags for an inner class. */
private static int innerClassAccess(int enclosing, TyDecl decl) {
int access = access(decl.mods(), decl.tykind());
int access = access(decl.mods(), decl);

// types declared in interfaces and annotations are implicitly public (JLS 9.5)
if ((enclosing & (TurbineFlag.ACC_INTERFACE | TurbineFlag.ACC_ANNOTATION)) != 0) {
Expand Down
3 changes: 3 additions & 0 deletions java/com/google/turbine/model/TurbineFlag.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ public final class TurbineFlag {
/** Default methods. */
public static final int ACC_DEFAULT = 1 << 16;

/** Enum constants with class bodies. */
public static final int ACC_ENUM_IMPL = 1 << 17;

/** Synthetic constructors (e.g. of inner classes and enums). */
public static final int ACC_SYNTH_CTOR = 1 << 18;

Expand Down
4 changes: 3 additions & 1 deletion java/com/google/turbine/parse/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -554,15 +554,17 @@ private ImmutableList<Tree> enumMembers(Ident enumName) {
if (token == Token.LPAREN) {
dropParens();
}
EnumSet<TurbineModifier> access = EnumSet.copyOf(ENUM_CONSTANT_MODIFIERS);
// TODO(cushon): consider desugaring enum constants later
if (token == Token.LBRACE) {
dropBlocks();
access.add(TurbineModifier.ENUM_IMPL);
}
maybe(Token.COMMA);
result.add(
new VarDecl(
position,
ENUM_CONSTANT_MODIFIERS,
access,
annos.build(),
new ClassTy(
position,
Expand Down
1 change: 1 addition & 0 deletions java/com/google/turbine/tree/Pretty.java
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ private void printModifiers(ImmutableSet<TurbineModifier> mods) {
case ACC_SYNTHETIC:
case ACC_BRIDGE:
case COMPACT_CTOR:
case ENUM_IMPL:
break;
}
}
Expand Down
3 changes: 2 additions & 1 deletion java/com/google/turbine/tree/TurbineModifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public enum TurbineModifier {
TRANSITIVE(TurbineFlag.ACC_TRANSITIVE),
SEALED(TurbineFlag.ACC_SEALED),
NON_SEALED(TurbineFlag.ACC_NON_SEALED),
COMPACT_CTOR(TurbineFlag.ACC_COMPACT_CTOR);
COMPACT_CTOR(TurbineFlag.ACC_COMPACT_CTOR),
ENUM_IMPL(TurbineFlag.ACC_ENUM_IMPL);

private final int flag;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public static Map<String, byte[]> canonicalize(Map<String, byte[]> in) {
for (ClassNode n : classes) {
removeImplementation(n);
removeUnusedInnerClassAttributes(infos, n);
makeEnumsFinal(all, n);
makeEnumsNonAbstract(all, n);
sortAttributes(n);
undeprecate(n);
removePreviewVersion(n);
Expand Down Expand Up @@ -187,17 +187,15 @@ private static boolean isDeprecated(List<AnnotationNode> visibleAnnotations) {
&& visibleAnnotations.stream().anyMatch(a -> a.desc.equals("Ljava/lang/Deprecated;"));
}

private static void makeEnumsFinal(Set<String> all, ClassNode n) {
private static void makeEnumsNonAbstract(Set<String> all, ClassNode n) {
n.innerClasses.forEach(
x -> {
if (all.contains(x.name) && (x.access & Opcodes.ACC_ENUM) == Opcodes.ACC_ENUM) {
x.access &= ~Opcodes.ACC_ABSTRACT;
x.access |= Opcodes.ACC_FINAL;
}
});
if ((n.access & Opcodes.ACC_ENUM) == Opcodes.ACC_ENUM) {
n.access &= ~Opcodes.ACC_ABSTRACT;
n.access |= Opcodes.ACC_FINAL;
}
}

Expand Down
30 changes: 30 additions & 0 deletions javatests/com/google/turbine/lower/testdata/enum_abstract.test
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,33 @@ class Test {
};
}
}
=== I.java ===
interface I {
void f();
}
=== EnumConstantImplementsInterface.java ===
enum EnumConstantImplementsInterface implements I {
ONE {
@Override
public void f() {}
};
}
=== EnumImplementsInterface.java ===
enum EnumImplementsInterface implements I {
ONE;

public void f() {}
}
=== EnumConstantImplementsMethod.java ===
enum EnumConstantImplementsMethod {
ONE {
@Override
public void f() {}
};
public void f() {}
}
=== EnumConstantEmptyBody.java ===
enum EnumConstantEmptyBody {
ONE {
};
}

0 comments on commit 70a8b3c

Please sign in to comment.