Use 'new Label()' rather than 'Code.newLabel()' to make it clear that allocating a label is a free operation, not depending on the current state of the Code instance.

9 files changed, 79 insertions, 49 deletions

diff --git a/src/main/java/com/google/dexmaker/Code.java b/src/main/java/com/google/dexmaker/Code.java
index 4ec2782..9afe0af 100644
--- a/src/main/java/com/google/dexmaker/Code.java
+++ b/src/main/java/com/google/dexmaker/Code.java
@@ -84,7 +84,8 @@ public final class Code {
         for (TypeId<?> parameter : method.parameters.types) {
             parameters.add(Local.get(this, parameter));
         }
-        this.currentLabel = newLabel();
+        this.currentLabel = new Label();
+        adopt(this.currentLabel);
         this.currentLabel.marked = true;
     }
 
@@ -167,19 +168,24 @@ public final class Code {
     // labels
 
     /**
-     * Creates a new label for use as a branch target. The new label must have
-     * code attached to it later by calling {@link #mark(Label)}.
+     * Assigns {@code target} to this code.
      */
-    public Label newLabel() {
-        Label result = new Label();
-        labels.add(result);
-        return result;
+    private void adopt(Label target) {
+        if (target.code == this) {
+            return; // already adopted
+        }
+        if (target.code != null) {
+            throw new IllegalArgumentException("Cannot adopt label; it belongs to another Code");
+        }
+        target.code = this;
+        labels.add(target);
     }
 
     /**
      * Start defining instructions for the named label.
      */
     public void mark(Label label) {
+        adopt(label);
         if (label.marked) {
             throw new IllegalStateException("already marked");
         }
@@ -191,6 +197,7 @@ public final class Code {
     }
 
     public void jump(Label target) {
+        adopt(target);
         addInstruction(new PlainInsn(Rops.GOTO, sourcePosition, null, RegisterSpecList.EMPTY),
                 target);
     }
@@ -199,6 +206,7 @@ public final class Code {
         if (catchTypes.contains(throwable)) {
             throw new IllegalArgumentException("Already caught: " + throwable);
         }
+        adopt(catchClause);
         catchTypes.add(throwable);
         catches = toTypeList(catchTypes);
         catchLabels.add(catchClause);
@@ -288,7 +296,8 @@ public final class Code {
      * @param catchLabels an immutable list of catch labels
      */
     private void splitCurrentLabel(Label alternateSuccessor, List<Label> catchLabels) {
-        Label newLabel = newLabel();
+        Label newLabel = new Label();
+        adopt(newLabel);
         currentLabel.primarySuccessor = newLabel;
         currentLabel.alternateSuccessor = alternateSuccessor;
         currentLabel.catchLabels = catchLabels;
@@ -366,9 +375,7 @@ public final class Code {
      * trueLabel}. If it is false, execution continues to the next instruction.
      */
     public <T> void compare(Comparison comparison, Label trueLabel, Local<T> a, Local<T> b) {
-        if (trueLabel == null) {
-            throw new IllegalArgumentException();
-        }
+        adopt(trueLabel);
         Rop rop = comparison.rop(StdTypeList.make(a.type.ropType, b.type.ropType));
         addInstruction(new PlainInsn(rop, sourcePosition, null,
                 RegisterSpecList.make(a.spec(), b.spec())), trueLabel);
diff --git a/src/main/java/com/google/dexmaker/DexMaker.java b/src/main/java/com/google/dexmaker/DexMaker.java
index 838bb69..d635465 100644
--- a/src/main/java/com/google/dexmaker/DexMaker.java
+++ b/src/main/java/com/google/dexmaker/DexMaker.java
@@ -25,7 +25,6 @@ import com.android.dx.dex.file.ClassDefItem;
 import com.android.dx.dex.file.DexFile;
 import com.android.dx.dex.file.EncodedField;
 import com.android.dx.dex.file.EncodedMethod;
-import com.android.dx.rop.code.AccessFlags;
 import static com.android.dx.rop.code.AccessFlags.ACC_CONSTRUCTOR;
 import com.android.dx.rop.code.LocalVariableInfo;
 import com.android.dx.rop.code.RopMethod;
@@ -36,6 +35,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Modifier;
 import static java.lang.reflect.Modifier.PRIVATE;
 import static java.lang.reflect.Modifier.STATIC;
 import java.util.LinkedHashMap;
@@ -157,7 +157,7 @@ import java.util.jar.JarOutputStream;
  *
  *   code.loadConstant(constant1, 1);
  *   code.loadConstant(constant2, 2);
- *   Label baseCase = code.newLabel();
+ *   Label baseCase = new Label();
  *   code.compare(Comparison.LT, baseCase, i, constant2);
  *   code.op(BinaryOp.SUBTRACT, a, i, constant1);
  *   code.op(BinaryOp.SUBTRACT, b, i, constant2);
@@ -201,10 +201,11 @@ public final class DexMaker {
         return result;
     }
 
-    // TODO: describe the legal flags without referring to a non-public API AccessFlags
-
     /**
-     * @param flags any flags masked by {@link com.android.dx.rop.code.AccessFlags#CLASS_FLAGS}.
+     * Declares {@code type}.
+     *
+     * @param flags a bitwise combination of {@link Modifier#PUBLIC}, {@link
+     *     Modifier#FINAL} and {@link Modifier#ABSTRACT}.
      */
     public void declare(TypeId<?> type, String sourceFile, int flags,
             TypeId<?> supertype, TypeId<?>... interfaces) {
@@ -220,14 +221,23 @@ public final class DexMaker {
     }
 
     /**
-     * @param flags any flags masked by {@link com.android.dx.rop.code.AccessFlags#METHOD_FLAGS}.
+     * Declares a constructor. The name of {@code method} must be "<init>",
+     * as it is on all instances returned by {@link TypeId#getConstructor}.
+     *
+     * @param flags a bitwise combination of {@link Modifier#PUBLIC}, {@link
+     *     Modifier#PRIVATE}, {@link Modifier#PROTECTED}, {@link Modifier#STATIC},
+     *     {@link Modifier#FINAL}, and {@link Modifier#VARARGS}.
      */
     public Code declareConstructor(MethodId<?, ?> method, int flags) {
         return declare(method, flags | ACC_CONSTRUCTOR);
     }
 
     /**
-     * @param flags any flags masked by {@link com.android.dx.rop.code.AccessFlags#METHOD_FLAGS}.
+     * Declares a method. The name of {@code method} must not be "<init>".
+     *
+     * @param flags a bitwise combination of {@link Modifier#PUBLIC}, {@link
+     *     Modifier#PRIVATE}, {@link Modifier#PROTECTED}, {@link Modifier#STATIC},
+     *     {@link Modifier#FINAL}, and {@link Modifier#VARARGS}.
      */
     public Code declare(MethodId<?, ?> method, int flags) {
         TypeDeclaration typeDeclaration = getTypeDeclaration(method.declaringType);
@@ -240,7 +250,12 @@ public final class DexMaker {
     }
 
     /**
-     * @param flags any flags masked by {@link AccessFlags#FIELD_FLAGS}.
+     * Declares a field.
+     *
+     * @param flags a bitwise combination of {@link Modifier#PUBLIC}, {@link
+     *     Modifier#PRIVATE}, {@link Modifier#PROTECTED}, {@link Modifier#STATIC},
+     *     {@link Modifier#FINAL}, {@link Modifier#VOLATILE}, and {@link
+     *     Modifier#TRANSIENT}.
      */
     public void declare(FieldId<?, ?> fieldId, int flags, Object staticValue) {
         TypeDeclaration typeDeclaration = getTypeDeclaration(fieldId.declaringType);
@@ -252,7 +267,7 @@ public final class DexMaker {
     }
 
     /**
-     * Returns a .dex formatted file.
+     * Generates a dex file and returns its bytes.
      */
     public byte[] generate() {
         DexOptions options = new DexOptions();
@@ -271,15 +286,14 @@ public final class DexMaker {
     }
 
     /**
-     * Loads the generated types into the current process.
+     * Generates a dex file and loads its types into the current process.
      *
-     * <p>All parameters are optional, you may pass {@code null} and suitable
+     * <p>All parameters are optional; you may pass {@code null} and suitable
      * defaults will be used.
      *
-     * <p>If you opt to provide your own output directories, take care to
-     * ensure that they are not world-readable, otherwise a malicious app will
-     * be able to inject code to run.  A suitable parameter for these output
-     * directories would be something like this:
+     * <p>If you opt to provide your own {@code dexDir}, take care to ensure
+     * that it is not world-writable, otherwise a malicious app may be able
+     * to inject code into your process.  A suitable parameter is:
      * {@code getApplicationContext().getDir("dx", Context.MODE_PRIVATE); }
      *
      * @param parent the parent ClassLoader to be used when loading
diff --git a/src/main/java/com/google/dexmaker/FieldId.java b/src/main/java/com/google/dexmaker/FieldId.java
index 17ebfec..42974e9 100644
--- a/src/main/java/com/google/dexmaker/FieldId.java
+++ b/src/main/java/com/google/dexmaker/FieldId.java
@@ -21,7 +21,7 @@ import com.android.dx.rop.cst.CstNat;
 import com.android.dx.rop.cst.CstString;
 
 /**
- * A field.
+ * Identifies a field.
  */
 public final class FieldId<D, V> {
     final TypeId<D> declaringType;
diff --git a/src/main/java/com/google/dexmaker/Label.java b/src/main/java/com/google/dexmaker/Label.java
index 401e2a0..b37b18c 100644
--- a/src/main/java/com/google/dexmaker/Label.java
+++ b/src/main/java/com/google/dexmaker/Label.java
@@ -31,6 +31,8 @@ public final class Label {
 
     final List<Insn> instructions = new ArrayList<Insn>();
 
+    Code code;
+
     boolean marked = false;
 
     /** an immutable list of labels corresponding to the types in the catch list */
@@ -44,7 +46,7 @@ public final class Label {
 
     int id = -1;
 
-    Label() {}
+    public Label() {}
 
     boolean isEmpty() {
         return instructions.isEmpty();
diff --git a/src/main/java/com/google/dexmaker/Local.java b/src/main/java/com/google/dexmaker/Local.java
index a993d5f..c89436d 100644
--- a/src/main/java/com/google/dexmaker/Local.java
+++ b/src/main/java/com/google/dexmaker/Local.java
@@ -19,7 +19,7 @@ package com.google.dexmaker;
 import com.android.dx.rop.code.RegisterSpec;
 
 /**
- * A temporary variable that holds a single value.
+ * A temporary variable that holds a single value of a known type.
  */
 public final class Local<T> {
     private final Code code;
diff --git a/src/main/java/com/google/dexmaker/MethodId.java b/src/main/java/com/google/dexmaker/MethodId.java
index ba67ff5..f437b90 100644
--- a/src/main/java/com/google/dexmaker/MethodId.java
+++ b/src/main/java/com/google/dexmaker/MethodId.java
@@ -23,7 +23,7 @@ import com.android.dx.rop.type.Prototype;
 import java.util.List;
 
 /**
- * A method or constructor.
+ * Identifies a method or constructor.
  */
 public final class MethodId<D, R> {
     final TypeId<D> declaringType;
@@ -55,6 +55,9 @@ public final class MethodId<D, R> {
         return returnType;
     }
 
+    /**
+     * Returns the method's name. This is "<init>" if this is a constructor.
+     */
     public String getName() {
         return name;
     }
diff --git a/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java b/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
index 7888a22..5970d82 100644
--- a/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
+++ b/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
@@ -385,7 +385,7 @@ public final class ProxyBuilder<T> {
 
             // if (proxy == null)
             code.loadConstant(nullHandler, null);
-            Label handlerNullCase = code.newLabel();
+            Label handlerNullCase = new Label();
             code.compare(Comparison.EQ, handlerNullCase, nullHandler, localHandler);
 
             // This code is what we execute when we have a valid proxy: delegate to invocation
diff --git a/src/test/java/com/google/dexmaker/DexMakerTest.java b/src/test/java/com/google/dexmaker/DexMakerTest.java
index b9b0ea2..9c347eb 100644
--- a/src/test/java/com/google/dexmaker/DexMakerTest.java
+++ b/src/test/java/com/google/dexmaker/DexMakerTest.java
@@ -493,8 +493,8 @@ public final class DexMakerTest extends TestCase {
         Local<Integer> localA = code.getParameter(0, TypeId.INT);
         Local<Integer> localB = code.getParameter(1, TypeId.INT);
         Local<Boolean> result = code.newLocal(TypeId.get(boolean.class));
-        Label afterIf = code.newLabel();
-        Label ifBody = code.newLabel();
+        Label afterIf = new Label();
+        Label ifBody = new Label();
         code.compare(comparison, ifBody, localA, localB);
         code.jump(afterIf);
 
@@ -1094,9 +1094,9 @@ public final class DexMakerTest extends TestCase {
         code.loadConstant(local2, 2);
         code.loadConstant(localResult, 1);
         code.loadConstant(localI, 0);
-        Label loopCondition = code.newLabel();
-        Label loopBody = code.newLabel();
-        Label afterLoop = code.newLabel();
+        Label loopCondition = new Label();
+        Label loopBody = new Label();
+        Label afterLoop = new Label();
         code.mark(loopCondition);
         code.compare(Comparison.LT, loopBody, localI, localCount);
         code.jump(afterLoop);
@@ -1135,9 +1135,9 @@ public final class DexMakerTest extends TestCase {
         Local<Integer> local2 = code.newLocal(TypeId.INT);
         code.loadConstant(localResult, 1);
         code.loadConstant(local2, 2);
-        Label loopCondition = code.newLabel();
-        Label loopBody = code.newLabel();
-        Label afterLoop = code.newLabel();
+        Label loopCondition = new Label();
+        Label loopBody = new Label();
+        Label afterLoop = new Label();
         code.mark(loopCondition);
         code.compare(Comparison.LT, loopBody, localResult, localMax);
         code.jump(afterLoop);
@@ -1178,9 +1178,9 @@ public final class DexMakerTest extends TestCase {
         Local<Integer> localA = code.getParameter(0, TypeId.INT);
         Local<Integer> localB = code.getParameter(1, TypeId.INT);
         Local<Integer> localC = code.getParameter(2, TypeId.INT);
-        Label aLessThanB = code.newLabel();
-        Label aLessThanC = code.newLabel();
-        Label bLessThanC = code.newLabel();
+        Label aLessThanB = new Label();
+        Label aLessThanC = new Label();
+        Label bLessThanC = new Label();
         code.compare(Comparison.LT, aLessThanB, localA, localB);
         code.compare(Comparison.LT, bLessThanC, localB, localC);
         code.returnValue(localC);
@@ -1224,7 +1224,7 @@ public final class DexMakerTest extends TestCase {
         Local<Integer> localX = code.newLocal(TypeId.INT);
         Local<Integer> localY = code.newLocal(TypeId.INT);
         Local<Integer> localResult = code.newLocal(TypeId.INT);
-        Label baseCase = code.newLabel();
+        Label baseCase = new Label();
         code.loadConstant(local1, 1);
         code.loadConstant(local2, 2);
         code.compare(Comparison.LT, baseCase, localA, local2);
@@ -1265,9 +1265,9 @@ public final class DexMakerTest extends TestCase {
         Code code = dexMaker.declare(methodId, PUBLIC | STATIC);
         Local<Integer> localI = code.getParameter(0, TypeId.INT);
         Local<String> result = code.newLocal(TypeId.STRING);
-        Label catchIae = code.newLabel();
-        Label catchIse = code.newLabel();
-        Label catchRe = code.newLabel();
+        Label catchIae = new Label();
+        Label catchIse = new Label();
+        Label catchRe = new Label();
 
         code.addCatchClause(TypeId.get(IllegalArgumentException.class), catchIae);
         code.addCatchClause(TypeId.get(IllegalStateException.class), catchIse);
@@ -1343,8 +1343,8 @@ public final class DexMakerTest extends TestCase {
         Local<Integer> localB = code.getParameter(1, TypeId.INT);
         Local<Integer> localC = code.getParameter(2, TypeId.INT);
         Local<String> localResult = code.newLocal(TypeId.STRING);
-        Label catchInner = code.newLabel();
-        Label catchOuter = code.newLabel();
+        Label catchInner = new Label();
+        Label catchOuter = new Label();
 
         TypeId<IllegalArgumentException> iaeType = TypeId.get(IllegalArgumentException.class);
         code.addCatchClause(iaeType, catchOuter);
@@ -1640,6 +1640,10 @@ public final class DexMakerTest extends TestCase {
 
     // TODO: attempt to generate an interface
 
+    // TODO: declare native method or abstract method
+
+    // TODO: synchronized or declared synchronized?
+
     private void addDefaultConstructor() {
         Code code = dexMaker.declareConstructor(GENERATED.getConstructor(), PUBLIC);
         Local<?> thisRef = code.getThis(GENERATED);
diff --git a/src/test/java/com/google/dexmaker/examples/FibonacciMaker.java b/src/test/java/com/google/dexmaker/examples/FibonacciMaker.java
index 91f743a..5e0461c 100644
--- a/src/test/java/com/google/dexmaker/examples/FibonacciMaker.java
+++ b/src/test/java/com/google/dexmaker/examples/FibonacciMaker.java
@@ -47,7 +47,7 @@ public final class FibonacciMaker {
         Local<Integer> result = code.newLocal(TypeId.INT);
         code.loadConstant(constant1, 1);
         code.loadConstant(constant2, 2);
-        Label baseCase = code.newLabel();
+        Label baseCase = new Label();
         code.compare(Comparison.LT, baseCase, i, constant2);
         code.op(BinaryOp.SUBTRACT, a, i, constant1);
         code.op(BinaryOp.SUBTRACT, b, i, constant2);