Prune private fields and methods from the output

--
MOS_MIGRATED_REVID=118713377

2 files changed, 78 insertions, 14 deletions

diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java
index 8a3041d6f0..acc09d021c 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java
@@ -27,7 +27,11 @@ import com.google.devtools.build.java.turbine.javac.ZipOutputFileManager.OutputF
 import com.sun.tools.javac.util.Context;
 
 import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
 
 import java.io.BufferedOutputStream;
 import java.io.IOException;
@@ -243,7 +247,7 @@ public class JavacTurbine implements AutoCloseable {
         String name = entry.getKey();
         byte[] bytes = entry.getValue().asBytes();
         if (name.endsWith(".class")) {
-          bytes = removeCode(bytes);
+          bytes = processBytecode(bytes);
         }
         ZipUtil.storeEntry(name, bytes, zipOut);
         hasEntries = true;
@@ -256,20 +260,58 @@ public class JavacTurbine implements AutoCloseable {
   }
 
   /**
-   * Strip any remaining code attributes.
+   * Remove code attributes and private members.
    *
    * <p>Most code will already have been removed after parsing, but the bytecode will still
    * contain e.g. lowered class and instance initializers.
    */
-  // TODO(cushon): add additional stripping to produce ijar-compatible output,
-  // e.g. removing private members.
-  private static byte[] removeCode(byte[] bytes) {
+  private static byte[] processBytecode(byte[] bytes) {
     ClassWriter cw = new ClassWriter(0);
     new ClassReader(bytes)
-        .accept(cw, ClassReader.SKIP_CODE | ClassReader.SKIP_FRAMES | ClassReader.SKIP_DEBUG);
+        .accept(
+            new PrivateMemberPruner(cw),
+            ClassReader.SKIP_CODE | ClassReader.SKIP_FRAMES | ClassReader.SKIP_DEBUG);
     return cw.toByteArray();
   }
 
+  /**
+   * Prune private members.
+   *
+   * <p>Like ijar, turbine prunes private fields and members to improve caching
+   * and reduce output size.
+   *
+   * <p>This is not always a safe optimization: it can prevent javac from emitting
+   * diagnostics e.g. when a public member is hidden by a private member which has
+   * then pruned. The impact of that is believed to be small, and as long as ijar
+   * continues to prune private members turbine should do the same for compatibility.
+   *
+   * <p>Some of this work could be done during tree pruning, but it's not completely
+   * trivial to detect private members at that point (e.g. with implicit modifiers).
+   */
+  static class PrivateMemberPruner extends ClassVisitor {
+    public PrivateMemberPruner(ClassVisitor cv) {
+      super(Opcodes.ASM5, cv);
+    }
+
+    @Override
+    public FieldVisitor visitField(
+        int access, String name, String desc, String signature, Object value) {
+      if ((access & Opcodes.ACC_PRIVATE) == Opcodes.ACC_PRIVATE) {
+        return null;
+      }
+      return super.visitField(access, name, desc, signature, value);
+    }
+
+    @Override
+    public MethodVisitor visitMethod(
+        int access, String name, String desc, String signature, String[] exceptions) {
+      if ((access & Opcodes.ACC_PRIVATE) == Opcodes.ACC_PRIVATE) {
+        return null;
+      }
+      return super.visitMethod(access, name, desc, signature, exceptions);
+    }
+  }
+
   /** Convert string elements of a classpath to {@link Path}s. */
   private static ImmutableList<Path> asPaths(Iterable<String> classpath) {
     ImmutableList.Builder<Path> result = ImmutableList.builder();
diff --git a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java
index d5fe382b32..1b4945b295 100644
--- a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java
+++ b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java
@@ -741,20 +741,12 @@ public class JavacTurbineTest {
       "  // access flags 0x4019",
       "  public final static enum LTheEnum; THREE",
       "",
-      "  // access flags 0x101A",
-      "  private final static synthetic [LTheEnum; $VALUES",
-      "",
       "  // access flags 0x9",
       "  public static values()[LTheEnum;",
       "",
       "  // access flags 0x9",
       "  public static valueOf(Ljava/lang/String;)LTheEnum;",
       "",
-      "  // access flags 0x2",
-      "  // signature ()V",
-      "  // declaration: void <init>()",
-      "  private <init>(Ljava/lang/String;I)V",
-      "",
       "  // access flags 0x8",
       "  static <clinit>()V",
       "}",
@@ -996,4 +988,34 @@ public class JavacTurbineTest {
     Map<String, byte[]> outputs = collectOutputs();
     assertThat(outputs.keySet()).containsExactly("Hello.class");
   }
+
+  @Test
+  public void privateMembers() throws Exception {
+    addSourceLines(
+        "Hello.java",
+        "class Hello {",
+        "  private void f() {}",
+        "  private int x;",
+        "}");
+
+    compile();
+
+    Map<String, byte[]> outputs = collectOutputs();
+
+    assertThat(outputs.keySet()).containsExactly("Hello.class");
+
+    String text = textify(outputs.get("Hello.class"));
+    String[] expected = {
+      "// class version 51.0 (51)",
+      "// access flags 0x20",
+      "class Hello {",
+      "",
+      "",
+      "  // access flags 0x0",
+      "  <init>()V",
+      "}",
+      ""
+    };
+    assertThat(text).isEqualTo(Joiner.on('\n').join(expected));
+  }
 }