diff options
Diffstat (limited to 'java/com/google/devtools')
16 files changed, 1751 insertions, 305 deletions
diff --git a/java/com/google/devtools/build/android/desugar/BitFlags.java b/java/com/google/devtools/build/android/desugar/BitFlags.java index 8542719..bb32c45 100644 --- a/java/com/google/devtools/build/android/desugar/BitFlags.java +++ b/java/com/google/devtools/build/android/desugar/BitFlags.java @@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.android.desugar; +import org.objectweb.asm.Opcodes; + /** * Convenience method for working with {@code int} bitwise flags. */ @@ -34,6 +36,18 @@ class BitFlags { return (flags & bitmask) == 0; } + public static boolean isInterface(int access) { + return isSet(access, Opcodes.ACC_INTERFACE); + } + + public static boolean isStatic(int access) { + return isSet(access, Opcodes.ACC_STATIC); + } + + public static boolean isSynthetic(int access) { + return isSet(access, Opcodes.ACC_SYNTHETIC); + } + // Static methods only private BitFlags() {} } diff --git a/java/com/google/devtools/build/android/desugar/BytecodeTypeInference.java b/java/com/google/devtools/build/android/desugar/BytecodeTypeInference.java new file mode 100644 index 0000000..777a4ab --- /dev/null +++ b/java/com/google/devtools/build/android/desugar/BytecodeTypeInference.java @@ -0,0 +1,1008 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.android.desugar; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.HashMap; +import org.objectweb.asm.Handle; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; + +/** + * Perform type inference for byte code (local variables and operand stack) with the help of stack + * map frames. + * + * <p>Note: This class only guarantees the correctness of reference types, but not the primitive + * types, though they might be correct too. + */ +public final class BytecodeTypeInference extends MethodVisitor { + + private boolean used = false; + private final ArrayList<InferredType> localVariableSlots; + private final ArrayList<InferredType> operandStack = new ArrayList<>(); + private FrameInfo previousFrame; + /** For debugging purpose. */ + private final String methodSignature; + /** + * Stores mapping from "uninitialized" value to concrete value. This is for the "new" instruction. + */ + private final HashMap<InferredType, InferredType> uninitializedToConcreteTypeMap = + new HashMap<>(); + + public BytecodeTypeInference(int access, String owner, String name, String methodDescriptor) { + super(Opcodes.ASM6); + localVariableSlots = createInitialLocalVariableTypes(access, owner, name, methodDescriptor); + previousFrame = FrameInfo.create(ImmutableList.copyOf(localVariableSlots), ImmutableList.of()); + this.methodSignature = owner + "." + name + methodDescriptor; + } + + public void setDelegateMethodVisitor(MethodVisitor visitor) { + mv = visitor; + } + + @Override + public void visitCode() { + checkState(!used, "Cannot reuse this method visitor."); + used = true; + super.visitCode(); + } + + /** Returns the type of a value in the operand. 0 means the top of the stack. */ + public InferredType getTypeOfOperandFromTop(int offsetFromTop) { + int index = operandStack.size() - 1 - offsetFromTop; + checkState( + index >= 0, + "Invalid offset %s in the list of size %s. The current method is %s", + offsetFromTop, + operandStack.size(), + methodSignature); + return operandStack.get(index); + } + + public String getOperandStackAsString() { + return operandStack.toString(); + } + + @Override + public void visitInsn(int opcode) { + switch (opcode) { + case Opcodes.NOP: + case Opcodes.INEG: + case Opcodes.LNEG: + case Opcodes.FNEG: + case Opcodes.DNEG: + case Opcodes.I2B: + case Opcodes.I2C: + case Opcodes.I2S: + case Opcodes.RETURN: + break; + case Opcodes.ACONST_NULL: + push(InferredType.NULL); + break; + case Opcodes.ICONST_M1: + case Opcodes.ICONST_0: + case Opcodes.ICONST_1: + case Opcodes.ICONST_2: + case Opcodes.ICONST_3: + case Opcodes.ICONST_4: + case Opcodes.ICONST_5: + push(InferredType.INT); + break; + case Opcodes.LCONST_0: + case Opcodes.LCONST_1: + push(InferredType.LONG); + push(InferredType.TOP); + break; + case Opcodes.FCONST_0: + case Opcodes.FCONST_1: + case Opcodes.FCONST_2: + push(InferredType.FLOAT); + break; + case Opcodes.DCONST_0: + case Opcodes.DCONST_1: + push(InferredType.DOUBLE); + push(InferredType.TOP); + break; + case Opcodes.IALOAD: + case Opcodes.BALOAD: + case Opcodes.CALOAD: + case Opcodes.SALOAD: + pop(2); + push(InferredType.INT); + break; + case Opcodes.LALOAD: + case Opcodes.D2L: + pop(2); + push(InferredType.LONG); + push(InferredType.TOP); + break; + case Opcodes.DALOAD: + case Opcodes.L2D: + pop(2); + push(InferredType.DOUBLE); + push(InferredType.TOP); + break; + case Opcodes.AALOAD: + InferredType arrayType = pop(2); + InferredType elementType = arrayType.getElementTypeIfArrayOrThrow(); + push(elementType); + break; + case Opcodes.IASTORE: + case Opcodes.BASTORE: + case Opcodes.CASTORE: + case Opcodes.SASTORE: + case Opcodes.FASTORE: + case Opcodes.AASTORE: + pop(3); + break; + case Opcodes.LASTORE: + case Opcodes.DASTORE: + pop(4); + break; + case Opcodes.POP: + case Opcodes.IRETURN: + case Opcodes.FRETURN: + case Opcodes.ARETURN: + case Opcodes.ATHROW: + case Opcodes.MONITORENTER: + case Opcodes.MONITOREXIT: + pop(); + break; + case Opcodes.POP2: + case Opcodes.LRETURN: + case Opcodes.DRETURN: + pop(2); + break; + case Opcodes.DUP: + push(top()); + break; + case Opcodes.DUP_X1: + { + InferredType top = pop(); + InferredType next = pop(); + push(top); + push(next); + push(top); + break; + } + case Opcodes.DUP_X2: + { + InferredType top = pop(); + InferredType next = pop(); + InferredType bottom = pop(); + push(top); + push(bottom); + push(next); + push(top); + break; + } + case Opcodes.DUP2: + { + InferredType top = pop(); + InferredType next = pop(); + push(next); + push(top); + push(next); + push(top); + break; + } + case Opcodes.DUP2_X1: + { + InferredType top = pop(); + InferredType next = pop(); + InferredType bottom = pop(); + push(next); + push(top); + push(bottom); + push(next); + push(top); + break; + } + case Opcodes.DUP2_X2: + { + InferredType t1 = pop(); + InferredType t2 = pop(); + InferredType t3 = pop(); + InferredType t4 = pop(); + push(t2); + push(t1); + push(t4); + push(t3); + push(t2); + push(t1); + break; + } + case Opcodes.SWAP: + { + InferredType top = pop(); + InferredType next = pop(); + push(top); + push(next); + break; + } + case Opcodes.IADD: + case Opcodes.ISUB: + case Opcodes.IMUL: + case Opcodes.IDIV: + case Opcodes.IREM: + case Opcodes.ISHL: + case Opcodes.ISHR: + case Opcodes.IUSHR: + case Opcodes.IAND: + case Opcodes.IOR: + case Opcodes.IXOR: + case Opcodes.L2I: + case Opcodes.D2I: + case Opcodes.FCMPL: + case Opcodes.FCMPG: + pop(2); + push(InferredType.INT); + break; + + case Opcodes.LADD: + case Opcodes.LSUB: + case Opcodes.LMUL: + case Opcodes.LDIV: + case Opcodes.LREM: + case Opcodes.LAND: + case Opcodes.LOR: + case Opcodes.LXOR: + pop(4); + push(InferredType.LONG); + push(InferredType.TOP); + break; + + case Opcodes.LSHL: + case Opcodes.LSHR: + case Opcodes.LUSHR: + pop(3); + push(InferredType.LONG); + push(InferredType.TOP); + break; + case Opcodes.I2L: + case Opcodes.F2L: + pop(); + push(InferredType.LONG); + push(InferredType.TOP); + break; + case Opcodes.I2F: + pop(); + push(InferredType.FLOAT); + break; + + case Opcodes.LCMP: + case Opcodes.DCMPG: + case Opcodes.DCMPL: + pop(4); + push(InferredType.INT); + break; + + case Opcodes.I2D: + case Opcodes.F2D: + pop(); + push(InferredType.DOUBLE); + push(InferredType.TOP); + break; + case Opcodes.F2I: + case Opcodes.ARRAYLENGTH: + pop(); + push(InferredType.INT); + break; + case Opcodes.FALOAD: + case Opcodes.FADD: + case Opcodes.FSUB: + case Opcodes.FMUL: + case Opcodes.FDIV: + case Opcodes.FREM: + case Opcodes.L2F: + case Opcodes.D2F: + pop(2); + push(InferredType.FLOAT); + break; + + case Opcodes.DADD: + case Opcodes.DSUB: + case Opcodes.DMUL: + case Opcodes.DDIV: + case Opcodes.DREM: + pop(4); + push(InferredType.DOUBLE); + push(InferredType.TOP); + break; + default: + throw new RuntimeException("Unhandled opcode " + opcode); + } + super.visitInsn(opcode); + } + + @Override + public void visitIntInsn(int opcode, int operand) { + switch (opcode) { + case Opcodes.BIPUSH: + case Opcodes.SIPUSH: + push(InferredType.INT); + break; + case Opcodes.NEWARRAY: + pop(); + switch (operand) { + case Opcodes.T_BOOLEAN: + pushDescriptor("[Z"); + break; + case Opcodes.T_CHAR: + pushDescriptor("[C"); + break; + case Opcodes.T_FLOAT: + pushDescriptor("[F"); + break; + case Opcodes.T_DOUBLE: + pushDescriptor("[D"); + break; + case Opcodes.T_BYTE: + pushDescriptor("[B"); + break; + case Opcodes.T_SHORT: + pushDescriptor("[S"); + break; + case Opcodes.T_INT: + pushDescriptor("[I"); + break; + case Opcodes.T_LONG: + pushDescriptor("[J"); + break; + default: + throw new RuntimeException("Unhandled operand value: " + operand); + } + break; + default: + throw new RuntimeException("Unhandled opcode " + opcode); + } + super.visitIntInsn(opcode, operand); + } + + @Override + public void visitVarInsn(int opcode, int var) { + switch (opcode) { + case Opcodes.ILOAD: + push(InferredType.INT); + break; + case Opcodes.LLOAD: + push(InferredType.LONG); + push(InferredType.TOP); + break; + case Opcodes.FLOAD: + push(InferredType.FLOAT); + break; + case Opcodes.DLOAD: + push(InferredType.DOUBLE); + push(InferredType.TOP); + break; + case Opcodes.ALOAD: + push(getLocalVariableType(var)); + break; + case Opcodes.ISTORE: + case Opcodes.FSTORE: + case Opcodes.ASTORE: + { + InferredType type = pop(); + setLocalVariableTypes(var, type); + break; + } + case Opcodes.LSTORE: + case Opcodes.DSTORE: + { + InferredType type = pop(2); + setLocalVariableTypes(var, type); + setLocalVariableTypes(var + 1, InferredType.TOP); + break; + } + case Opcodes.RET: + throw new RuntimeException("The instruction RET is not supported"); + default: + throw new RuntimeException("Unhandled opcode " + opcode); + } + super.visitVarInsn(opcode, var); + } + + @Override + public void visitTypeInsn(int opcode, String type) { + String descriptor = convertToDescriptor(type); + switch (opcode) { + case Opcodes.NEW: + pushDescriptor(descriptor); // This should be UNINITIALIZED(label). Okay for type inference. + break; + case Opcodes.ANEWARRAY: + pop(); + pushDescriptor('[' + descriptor); + break; + case Opcodes.CHECKCAST: + pop(); + pushDescriptor(descriptor); + break; + case Opcodes.INSTANCEOF: + pop(); + push(InferredType.INT); + break; + default: + throw new RuntimeException("Unhandled opcode " + opcode); + } + super.visitTypeInsn(opcode, type); + } + + @Override + public void visitFieldInsn(int opcode, String owner, String name, String desc) { + switch (opcode) { + case Opcodes.GETSTATIC: + pushDescriptor(desc); + break; + case Opcodes.PUTSTATIC: + popDescriptor(desc); + break; + case Opcodes.GETFIELD: + pop(); + pushDescriptor(desc); + break; + case Opcodes.PUTFIELD: + popDescriptor(desc); + pop(); + break; + default: + throw new RuntimeException( + "Unhandled opcode " + opcode + ", owner=" + owner + ", name=" + name + ", desc" + desc); + } + super.visitFieldInsn(opcode, owner, name, desc); + } + + @Override + public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) { + if (opcode == Opcodes.INVOKESPECIAL && "<init>".equals(name)) { + int argumentSize = (Type.getArgumentsAndReturnSizes(desc) >> 2); + InferredType receiverType = getTypeOfOperandFromTop(argumentSize - 1); + if (receiverType.isUninitialized()) { + InferredType realType = InferredType.create('L' + owner + ';'); + uninitializedToConcreteTypeMap.put(receiverType, realType); + replaceUninitializedTypeInStack(receiverType, realType); + } + } + switch (opcode) { + case Opcodes.INVOKESPECIAL: + case Opcodes.INVOKEVIRTUAL: + case Opcodes.INVOKESTATIC: + case Opcodes.INVOKEINTERFACE: + popDescriptor(desc); + if (opcode != Opcodes.INVOKESTATIC) { + pop(); // Pop receiver. + } + pushDescriptor(desc); + break; + default: + throw new RuntimeException( + String.format( + "Unhandled opcode %s, owner=%s, name=%s, desc=%s, itf=%s", + opcode, owner, name, desc, itf)); + } + super.visitMethodInsn(opcode, owner, name, desc, itf); + } + + @Override + public void visitInvokeDynamicInsn(String name, String desc, Handle bsm, Object... bsmArgs) { + popDescriptor(desc); + pushDescriptor(desc); + super.visitInvokeDynamicInsn(name, desc, bsm, bsmArgs); + } + + @Override + public void visitJumpInsn(int opcode, Label label) { + switch (opcode) { + case Opcodes.IFEQ: + case Opcodes.IFNE: + case Opcodes.IFLT: + case Opcodes.IFGE: + case Opcodes.IFGT: + case Opcodes.IFLE: + pop(); + break; + case Opcodes.IF_ICMPEQ: + case Opcodes.IF_ICMPNE: + case Opcodes.IF_ICMPLT: + case Opcodes.IF_ICMPGE: + case Opcodes.IF_ICMPGT: + case Opcodes.IF_ICMPLE: + case Opcodes.IF_ACMPEQ: + case Opcodes.IF_ACMPNE: + pop(2); + break; + case Opcodes.GOTO: + break; + case Opcodes.JSR: + throw new RuntimeException("The JSR instruction is not supported."); + case Opcodes.IFNULL: + case Opcodes.IFNONNULL: + pop(1); + break; + default: + throw new RuntimeException("Unhandled opcode " + opcode); + } + super.visitJumpInsn(opcode, label); + } + + @Override + public void visitLdcInsn(Object cst) { + if (cst instanceof Integer) { + push(InferredType.INT); + } else if (cst instanceof Float) { + push(InferredType.FLOAT); + } else if (cst instanceof Long) { + push(InferredType.LONG); + push(InferredType.TOP); + } else if (cst instanceof Double) { + push(InferredType.DOUBLE); + push(InferredType.TOP); + } else if (cst instanceof String) { + pushDescriptor("Ljava/lang/String;"); + } else if (cst instanceof Type) { + pushDescriptor(((Type) cst).getDescriptor()); + } else if (cst instanceof Handle) { + pushDescriptor("Ljava/lang/invoke/MethodHandle;"); + } else { + throw new RuntimeException("Cannot handle constant " + cst + " for LDC instruction"); + } + super.visitLdcInsn(cst); + } + + @Override + public void visitIincInsn(int var, int increment) { + setLocalVariableTypes(var, InferredType.INT); + super.visitIincInsn(var, increment); + } + + @Override + public void visitTableSwitchInsn(int min, int max, Label dflt, Label... labels) { + pop(); + super.visitTableSwitchInsn(min, max, dflt, labels); + } + + @Override + public void visitLookupSwitchInsn(Label dflt, int[] keys, Label[] labels) { + pop(); + super.visitLookupSwitchInsn(dflt, keys, labels); + } + + @Override + public void visitMultiANewArrayInsn(String desc, int dims) { + pop(dims); + pushDescriptor(desc); + super.visitMultiANewArrayInsn(desc, dims); + } + + @Override + public void visitFrame(int type, int nLocal, Object[] local, int nStack, Object[] stack) { + switch (type) { + case Opcodes.F_NEW: + // Expanded form. + previousFrame = + FrameInfo.create( + convertTypesInStackMapFrame(nLocal, local), + convertTypesInStackMapFrame(nStack, stack)); + break; + case Opcodes.F_SAME: + // This frame type indicates that the frame has exactly the same local variables as the + // previous frame and that the operand stack is empty. + previousFrame = FrameInfo.create(previousFrame.locals(), ImmutableList.of()); + break; + case Opcodes.F_SAME1: + // This frame type indicates that the frame has exactly the same local variables as the + // previous frame and that the operand stack has one entry. + previousFrame = + FrameInfo.create(previousFrame.locals(), convertTypesInStackMapFrame(nStack, stack)); + break; + case Opcodes.F_APPEND: + // This frame type indicates that the frame has the same locals as the previous frame except + // that k additional locals are defined, and that the operand stack is empty. + previousFrame = + FrameInfo.create( + appendArrayToList(previousFrame.locals(), nLocal, local), ImmutableList.of()); + break; + case Opcodes.F_CHOP: + // This frame type indicates that the frame has the same local variables as the previous + // frame except that the last k local variables are absent, and that the operand stack is + // empty. + previousFrame = + FrameInfo.create( + removeBackFromList(previousFrame.locals(), nLocal), ImmutableList.of()); + break; + case Opcodes.F_FULL: + previousFrame = + FrameInfo.create( + convertTypesInStackMapFrame(nLocal, local), + convertTypesInStackMapFrame(nStack, stack)); + break; + default: + // continue below + } + // Update types for operand stack and local variables. + operandStack.clear(); + operandStack.addAll(previousFrame.stack()); + localVariableSlots.clear(); + localVariableSlots.addAll(previousFrame.locals()); + + super.visitFrame(type, nLocal, local, nStack, stack); + } + + private static String convertToDescriptor(String type) { + char firstChar = type.charAt(0); + switch (firstChar) { + case 'Z': + case 'B': + case 'C': + case 'S': + case 'I': + case 'J': + case 'D': + case 'F': + case '[': + return type; + default: + return 'L' + type + ';'; + } + } + + private void push(InferredType type) { + operandStack.add(type); + } + + private void replaceUninitializedTypeInStack(InferredType oldType, InferredType newType) { + checkArgument(oldType.isUninitialized(), "The old type is NOT uninitialized. %s", oldType); + for (int i = 0, size = operandStack.size(); i < size; ++i) { + InferredType type = operandStack.get(i); + if (type == oldType) { + operandStack.set(i, newType); + } + } + } + + private final void pushDescriptor(String desc) { + int index = desc.charAt(0) == '(' ? desc.indexOf(')') + 1 : 0; + switch (desc.charAt(index)) { + case 'V': + return; + case 'Z': + case 'C': + case 'B': + case 'S': + case 'I': + push(InferredType.INT); + break; + case 'F': + push(InferredType.FLOAT); + break; + case 'D': + push(InferredType.DOUBLE); + push(InferredType.TOP); + break; + case 'J': + push(InferredType.LONG); + push(InferredType.TOP); + break; + case 'L': + case '[': + push(InferredType.create(desc.substring(index))); + break; + default: + throw new RuntimeException("Unhandled type: " + desc); + } + } + + private final InferredType pop() { + return pop(1); + } + + private final void popDescriptor(String desc) { + char c = desc.charAt(0); + switch (c) { + case '(': + int argumentSize = (Type.getArgumentsAndReturnSizes(desc) >> 2) - 1; + if (argumentSize > 0) { + pop(argumentSize); + } + break; + case 'J': + case 'D': + pop(2); + break; + default: + pop(1); + break; + } + } + + private final InferredType getLocalVariableType(int index) { + checkState( + index < localVariableSlots.size(), + "Cannot find type for var %s in method %s", + index, + methodSignature); + return localVariableSlots.get(index); + } + + private final void setLocalVariableTypes(int index, InferredType type) { + while (localVariableSlots.size() <= index) { + localVariableSlots.add(InferredType.TOP); + } + localVariableSlots.set(index, type); + } + + private final InferredType top() { + return operandStack.get(operandStack.size() - 1); + } + + /** Pop elements from the end of the operand stack, and return the last popped element. */ + private final InferredType pop(int count) { + checkArgument( + count >= 1, "The count should be at least one: %s (In %s)", count, methodSignature); + checkState( + operandStack.size() >= count, + "There are no enough elements in the stack. count=%s, stack=%s (In %s)", + count, + operandStack, + methodSignature); + int expectedLastIndex = operandStack.size() - count - 1; + InferredType lastPopped = null; + for (int i = operandStack.size() - 1; i > expectedLastIndex; --i) { + lastPopped = operandStack.remove(i); + } + return lastPopped; + } + + private static ImmutableList<InferredType> removeBackFromList( + ImmutableList<InferredType> list, int countToRemove) { + int newSize = list.size() - countToRemove; + return list.subList(0, newSize); + } + + /** + * Create the types of local variables at the very beginning of the method with the information of + * the declaring class and the method descriptor. + */ + private static ArrayList<InferredType> createInitialLocalVariableTypes( + int access, String ownerClass, String methodName, String methodDescriptor) { + ArrayList<InferredType> types = new ArrayList<>(); + + if (!BitFlags.isSet(access, Opcodes.ACC_STATIC)) { + // Instance method, and this is the receiver + types.add(InferredType.create(convertToDescriptor(ownerClass))); + } + Type[] argumentTypes = Type.getArgumentTypes(methodDescriptor); + for (Type argumentType : argumentTypes) { + switch (argumentType.getSort()) { + case Type.BOOLEAN: + case Type.BYTE: + case Type.CHAR: + case Type.SHORT: + case Type.INT: + types.add(InferredType.INT); + break; + case Type.FLOAT: + types.add(InferredType.FLOAT); + break; + case Type.LONG: + types.add(InferredType.LONG); + types.add(InferredType.TOP); + break; + case Type.DOUBLE: + types.add(InferredType.DOUBLE); + types.add(InferredType.TOP); + break; + case Type.ARRAY: + case Type.OBJECT: + types.add(InferredType.create(argumentType.getDescriptor())); + break; + default: + throw new RuntimeException( + "Unhandled argument type: " + + argumentType + + " in " + + ownerClass + + "." + + methodName + + methodDescriptor); + } + } + return types; + } + + private ImmutableList<InferredType> appendArrayToList( + ImmutableList<InferredType> list, int size, Object[] array) { + ImmutableList.Builder<InferredType> builder = ImmutableList.builder(); + builder.addAll(list); + for (int i = 0; i < size; ++i) { + InferredType type = convertTypeInStackMapFrame(array[i]); + builder.add(type); + if (type.isCategory2()) { + builder.add(InferredType.TOP); + } + } + return builder.build(); + } + + /** Convert the type in stack map frame to inference type. */ + private InferredType convertTypeInStackMapFrame(Object typeInStackMapFrame) { + if (typeInStackMapFrame == Opcodes.TOP) { + return InferredType.TOP; + } else if (typeInStackMapFrame == Opcodes.INTEGER) { + return InferredType.INT; + } else if (typeInStackMapFrame == Opcodes.FLOAT) { + return InferredType.FLOAT; + } else if (typeInStackMapFrame == Opcodes.DOUBLE) { + return InferredType.DOUBLE; + } else if (typeInStackMapFrame == Opcodes.LONG) { + return InferredType.LONG; + } else if (typeInStackMapFrame == Opcodes.NULL) { + return InferredType.NULL; + } else if (typeInStackMapFrame == Opcodes.UNINITIALIZED_THIS) { + return InferredType.UNINITIALIZED_THIS; + } else if (typeInStackMapFrame instanceof String) { + String referenceTypeName = (String) typeInStackMapFrame; + if (referenceTypeName.charAt(0) == '[') { + return InferredType.create(referenceTypeName); + } else { + return InferredType.create('L' + referenceTypeName + ';'); + } + } else if (typeInStackMapFrame instanceof Label) { + Label label = (Label) typeInStackMapFrame; + return InferredType.createUninitialized(label.getOffset()); + } else { + throw new RuntimeException( + "Cannot reach here. Unhandled element: value=" + + typeInStackMapFrame + + ", class=" + + typeInStackMapFrame.getClass() + + ". The current method being desugared is " + + methodSignature); + } + } + + private ImmutableList<InferredType> convertTypesInStackMapFrame(int size, Object[] array) { + ImmutableList.Builder<InferredType> builder = ImmutableList.builder(); + for (int i = 0; i < size; ++i) { + InferredType type = convertTypeInStackMapFrame(array[i]); + builder.add(type); + if (type.isCategory2()) { + builder.add(InferredType.TOP); + } + } + return builder.build(); + } + + /** A value class to represent a frame. */ + @AutoValue + abstract static class FrameInfo { + + static FrameInfo create(ImmutableList<InferredType> locals, ImmutableList<InferredType> stack) { + return new AutoValue_BytecodeTypeInference_FrameInfo(locals, stack); + } + + abstract ImmutableList<InferredType> locals(); + + abstract ImmutableList<InferredType> stack(); + } + + /** This is the type used for type inference. */ + @AutoValue + public abstract static class InferredType { + + public static final String UNINITIALIZED_PREFIX = "UNINIT@"; + + public static final InferredType BOOLEAN = + new AutoValue_BytecodeTypeInference_InferredType("Z"); + public static final InferredType BYTE = new AutoValue_BytecodeTypeInference_InferredType("B"); + public static final InferredType INT = new AutoValue_BytecodeTypeInference_InferredType("I"); + public static final InferredType FLOAT = new AutoValue_BytecodeTypeInference_InferredType("F"); + public static final InferredType LONG = new AutoValue_BytecodeTypeInference_InferredType("J"); + public static final InferredType DOUBLE = new AutoValue_BytecodeTypeInference_InferredType("D"); + /** Not a real value. */ + public static final InferredType TOP = new AutoValue_BytecodeTypeInference_InferredType("TOP"); + /** The value NULL */ + public static final InferredType NULL = + new AutoValue_BytecodeTypeInference_InferredType("NULL"); + + public static final InferredType UNINITIALIZED_THIS = + new AutoValue_BytecodeTypeInference_InferredType("UNINITIALIZED_THIS"); + + static InferredType create(String descriptor) { + char firstChar = descriptor.charAt(0); + if (firstChar == 'L' || firstChar == '[' || descriptor.startsWith(UNINITIALIZED_PREFIX)) { + // Reference, array, or uninitialized values. + return new AutoValue_BytecodeTypeInference_InferredType(descriptor); + } + switch (descriptor) { + case "Z": + return BOOLEAN; + case "B": + return BYTE; + case "I": + return INT; + case "F": + return FLOAT; + case "J": + return LONG; + case "D": + return DOUBLE; + case "TOP": + return TOP; + case "NULL": + return NULL; + case "UNINITIALIZED_THIS": + return UNINITIALIZED_THIS; + default: + throw new RuntimeException("Invalid descriptor: " + descriptor); + } + } + + /** Create a type for uninitialized value. The label is generated by ASM. */ + static InferredType createUninitialized(int label) { + return create(UNINITIALIZED_PREFIX + label); + } + + abstract String descriptor(); + + @Override + public String toString() { + return descriptor(); + } + + /** Is a category 2 value? */ + public boolean isCategory2() { + String descriptor = descriptor(); + return descriptor.equals("J") || descriptor.equals("D"); + } + + /** If the type is an array, return the element type. Otherwise, throw an exception. */ + public InferredType getElementTypeIfArrayOrThrow() { + String descriptor = descriptor(); + checkState(descriptor.charAt(0) == '[', "This type %s is not an array.", this); + return create(descriptor.substring(1)); + } + + /** Is an uninitialized value? */ + public boolean isUninitialized() { + return descriptor().startsWith(UNINITIALIZED_PREFIX); + } + + /** Is a null value? */ + public boolean isNull() { + return NULL.equals(this); + } + + /** + * If this type is a reference type, then return the internal name. Otherwise, throw an + * exception. + */ + public String getInternalNameOrThrow() { + String descriptor = descriptor(); + int length = descriptor.length(); + checkState( + descriptor.charAt(0) == 'L' && descriptor.charAt(length - 1) == ';', + "The type is expected to be either a class or an interface: %s", + descriptor); + return descriptor.substring(1, length - 1); + } + } +} diff --git a/java/com/google/devtools/build/android/desugar/ClassVsInterface.java b/java/com/google/devtools/build/android/desugar/ClassVsInterface.java new file mode 100644 index 0000000..cb62deb --- /dev/null +++ b/java/com/google/devtools/build/android/desugar/ClassVsInterface.java @@ -0,0 +1,63 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.android.desugar; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; + +import java.util.HashMap; +import javax.annotation.Nullable; +import org.objectweb.asm.ClassReader; + +/** + * Simple memoizer for whether types are classes or interfaces. + */ +class ClassVsInterface { + /** Map from internal names to whether they are an interface ({@code false} thus means class). */ + private final HashMap<String, Boolean> known = new HashMap<>(); + private final ClassReaderFactory classpath; + + public ClassVsInterface(ClassReaderFactory classpath) { + this.classpath = classpath; + } + + public ClassVsInterface addKnownClass(@Nullable String internalName) { + if (internalName != null) { + Boolean previous = known.put(internalName, false); + checkState(previous == null || !previous, "Already recorded as interface: %s", internalName); + } + return this; + } + + public ClassVsInterface addKnownInterfaces(String... internalNames) { + for (String internalName : internalNames) { + Boolean previous = known.put(internalName, true); + checkState(previous == null || previous, "Already recorded as class: %s", internalName); + } + return this; + } + + public boolean isOuterInterface(String outerName, String innerName) { + Boolean result = known.get(outerName); + if (result == null) { + // We could just load the outer class here, but this tolerates incomplete classpaths better. + // Note the outer class should be in the Jar we're desugaring, so it should always be there. + ClassReader outerClass = checkNotNull(classpath.readIfKnown(outerName), + "Couldn't find outer class %s of %s", outerName, innerName); + result = BitFlags.isInterface(outerClass.getAccess()); + known.put(outerName, result); + } + return result; + } +} diff --git a/java/com/google/devtools/build/android/desugar/CloseResourceMethodScanner.java b/java/com/google/devtools/build/android/desugar/CloseResourceMethodScanner.java new file mode 100644 index 0000000..7567515 --- /dev/null +++ b/java/com/google/devtools/build/android/desugar/CloseResourceMethodScanner.java @@ -0,0 +1,116 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.android.desugar; + +import com.google.common.base.Preconditions; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +/** + * A class scanner to check whether the class has the synthetic method $closeResource(Throwable, + * AutoCloseable). + */ +public class CloseResourceMethodScanner extends ClassVisitor { + + private boolean hasCloseResourceMethod; + private String internalName; + private int classFileVersion; + + public CloseResourceMethodScanner() { + super(Opcodes.ASM6); + } + + @Override + public void visit( + int version, + int access, + String name, + String signature, + String superName, + String[] interfaces) { + Preconditions.checkState(internalName == null, "This scanner has been used."); + this.internalName = name; + this.classFileVersion = version; + super.visit(version, access, name, signature, superName, interfaces); + } + + public boolean hasCloseResourceMethod() { + return hasCloseResourceMethod; + } + + @Override + public MethodVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + if (classFileVersion <= 50) { + // A Java 6 or below class file should not have $closeResource method. + return null; + } + if (!hasCloseResourceMethod) { + hasCloseResourceMethod = + TryWithResourcesRewriter.isSyntheticCloseResourceMethod(access, name, desc); + } + return new StackMapFrameCollector(name, desc); + } + + private class StackMapFrameCollector extends MethodVisitor { + + private final String methodSignature; + private boolean hasCallToCloseResourceMethod; + private boolean hasJumpInstructions; + private boolean hasStackMapFrame; + + public StackMapFrameCollector(String name, String desc) { + super(Opcodes.ASM6); + methodSignature = internalName + '.' + name + desc; + } + + @Override + public void visitEnd() { + if (!hasCallToCloseResourceMethod) { + return; + } + if (hasJumpInstructions && !hasStackMapFrame) { + throw new UnsupportedOperationException( + "The method " + + methodSignature + + " calls $closeResource(Throwable, AutoCloseable), " + + "and Desugar thus needs to perform type inference for it " + + "to rewrite $closeResourceMethod. " + + "However, this method has jump instructions, but does not have stack map frames. " + + "Please recompile this class with stack map frames."); + } + } + + @Override + public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) { + if (!hasCallToCloseResourceMethod + && TryWithResourcesRewriter.isCallToSyntheticCloseResource( + internalName, opcode, owner, name, desc)) { + hasCallToCloseResourceMethod = true; + } + } + + @Override + public void visitFrame(int type, int nLocal, Object[] local, int nStack, Object[] stack) { + hasStackMapFrame = true; + } + + @Override + public void visitJumpInsn(int opcode, Label label) { + hasJumpInstructions = true; + } + } +} diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java index 87aa0d7..5d3df4a 100644 --- a/java/com/google/devtools/build/android/desugar/Desugar.java +++ b/java/com/google/devtools/build/android/desugar/Desugar.java @@ -24,7 +24,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSet.Builder; import com.google.common.io.ByteStreams; import com.google.common.io.Closer; import com.google.devtools.build.android.Converters.ExistingPathConverter; @@ -358,7 +357,7 @@ class Desugar { } ImmutableSet.Builder<String> interfaceLambdaMethodCollector = ImmutableSet.builder(); - + ClassVsInterface interfaceCache = new ClassVsInterface(classpathReader); desugarClassesInInput( inputFiles, outputFileProvider, @@ -366,6 +365,7 @@ class Desugar { classpathReader, depsCollector, bootclasspathReader, + interfaceCache, interfaceLambdaMethodCollector); desugarAndWriteDumpedLambdaClassesToOutput( @@ -374,6 +374,7 @@ class Desugar { classpathReader, depsCollector, bootclasspathReader, + interfaceCache, interfaceLambdaMethodCollector.build(), bridgeMethodReader); @@ -407,8 +408,7 @@ class Desugar { "com.google.devtools.build.android.desugar.dependencies.MetadataCollector") .getConstructor(Boolean.TYPE) .newInstance(options.tolerateMissingDependencies); - } catch (ReflectiveOperationException - | SecurityException e) { + } catch (ReflectiveOperationException | SecurityException e) { throw new IllegalStateException("Can't emit desugaring metadata as requested"); } } else if (options.tolerateMissingDependencies) { @@ -445,7 +445,8 @@ class Desugar { @Nullable ClassReaderFactory classpathReader, DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, - Builder<String> interfaceLambdaMethodCollector) + ClassVsInterface interfaceCache, + ImmutableSet.Builder<String> interfaceLambdaMethodCollector) throws IOException { for (String filename : inputFiles) { if (OutputFileProvider.DESUGAR_DEPS_FILENAME.equals(filename)) { @@ -465,6 +466,7 @@ class Desugar { classpathReader, depsCollector, bootclasspathReader, + interfaceCache, interfaceLambdaMethodCollector, writer, reader); @@ -492,6 +494,7 @@ class Desugar { @Nullable ClassReaderFactory classpathReader, DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, + ClassVsInterface interfaceCache, ImmutableSet<String> interfaceLambdaMethods, @Nullable ClassReaderFactory bridgeMethodReader) throws IOException { @@ -522,10 +525,12 @@ class Desugar { classpathReader, depsCollector, bootclasspathReader, + interfaceCache, interfaceLambdaMethods, bridgeMethodReader, lambdaClass.getValue(), - writer); + writer, + reader); reader.accept(visitor, 0); String filename = rewriter.unprefix(lambdaClass.getValue().desiredInternalName()) + ".class"; @@ -564,15 +569,23 @@ class Desugar { @Nullable ClassReaderFactory classpathReader, DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, + ClassVsInterface interfaceCache, ImmutableSet<String> interfaceLambdaMethods, @Nullable ClassReaderFactory bridgeMethodReader, LambdaInfo lambdaClass, - UnprefixingClassWriter writer) { + UnprefixingClassWriter writer, + ClassReader input) { ClassVisitor visitor = checkNotNull(writer); if (!allowTryWithResources) { + CloseResourceMethodScanner closeResourceMethodScanner = new CloseResourceMethodScanner(); + input.accept(closeResourceMethodScanner, ClassReader.SKIP_DEBUG); visitor = new TryWithResourcesRewriter( - visitor, loader, visitedExceptionTypes, numOfTryWithResourcesInvoked); + visitor, + loader, + visitedExceptionTypes, + numOfTryWithResourcesInvoked, + closeResourceMethodScanner.hasCloseResourceMethod()); } if (!allowCallsToObjectsNonNull) { // Not sure whether there will be implicit null check emitted by javac, so we rerun @@ -591,7 +604,12 @@ class Desugar { visitor, classpathReader, depsCollector, bootclasspathReader, loader); visitor = new InterfaceDesugaring( - visitor, depsCollector, bootclasspathReader, store, options.legacyJacocoFix); + visitor, + interfaceCache, + depsCollector, + bootclasspathReader, + store, + options.legacyJacocoFix); } } visitor = @@ -621,14 +639,21 @@ class Desugar { @Nullable ClassReaderFactory classpathReader, DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, - Builder<String> interfaceLambdaMethodCollector, + ClassVsInterface interfaceCache, + ImmutableSet.Builder<String> interfaceLambdaMethodCollector, UnprefixingClassWriter writer, ClassReader input) { ClassVisitor visitor = checkNotNull(writer); if (!allowTryWithResources) { + CloseResourceMethodScanner closeResourceMethodScanner = new CloseResourceMethodScanner(); + input.accept(closeResourceMethodScanner, ClassReader.SKIP_DEBUG); visitor = new TryWithResourcesRewriter( - visitor, loader, visitedExceptionTypes, numOfTryWithResourcesInvoked); + visitor, + loader, + visitedExceptionTypes, + numOfTryWithResourcesInvoked, + closeResourceMethodScanner.hasCloseResourceMethod()); } if (!allowCallsToObjectsNonNull) { visitor = new ObjectsRequireNonNullMethodRewriter(visitor); @@ -645,7 +670,12 @@ class Desugar { visitor, classpathReader, depsCollector, bootclasspathReader, loader); visitor = new InterfaceDesugaring( - visitor, depsCollector, bootclasspathReader, store, options.legacyJacocoFix); + visitor, + interfaceCache, + depsCollector, + bootclasspathReader, + store, + options.legacyJacocoFix); } } // LambdaDesugaring is relatively expensive, so check first whether we need it. Additionally, diff --git a/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java b/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java index b93613c..b8b3ead 100644 --- a/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java +++ b/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java @@ -44,6 +44,7 @@ class InterfaceDesugaring extends ClassVisitor { static final String INTERFACE_STATIC_COMPANION_METHOD_SUFFIX = "$$STATIC$$"; + private final ClassVsInterface interfaceCache; private final DependencyCollector depsCollector; private final ClassReaderFactory bootclasspath; private final GeneratedClassStore store; @@ -58,11 +59,13 @@ class InterfaceDesugaring extends ClassVisitor { public InterfaceDesugaring( ClassVisitor dest, + ClassVsInterface interfaceCache, DependencyCollector depsCollector, ClassReaderFactory bootclasspath, GeneratedClassStore store, boolean legacyJaCoCo) { super(Opcodes.ASM6, dest); + this.interfaceCache = interfaceCache; this.depsCollector = depsCollector; this.bootclasspath = bootclasspath; this.store = store; @@ -83,10 +86,14 @@ class InterfaceDesugaring extends ClassVisitor { bytecodeVersion = version; accessFlags = access; if (isInterface()) { + interfaceCache.addKnownInterfaces(name); // Record interface hierarchy. This helps avoid parsing .class files when double-checking // desugaring results later using collected dependency information. depsCollector.recordExtendedInterfaces(name, interfaces); + } else { + interfaceCache.addKnownClass(name); } + interfaceCache.addKnownClass(superName).addKnownInterfaces(interfaces); super.visit(version, access, name, signature, superName, interfaces); } @@ -174,16 +181,14 @@ class InterfaceDesugaring extends ClassVisitor { checkArgument(BitFlags.noneSet(access, Opcodes.ACC_NATIVE), "Forbidden per JLS ch 9.4"); boolean isLambdaBody = - name.startsWith("lambda$") && BitFlags.isSet(access, Opcodes.ACC_SYNTHETIC); + name.startsWith("lambda$") && BitFlags.isSynthetic(access); if (isLambdaBody) { access &= ~Opcodes.ACC_PUBLIC; // undo visibility change from LambdaDesugaring } - name = - normalizeInterfaceMethodName( - name, isLambdaBody, BitFlags.isSet(access, Opcodes.ACC_STATIC)); + name = normalizeInterfaceMethodName(name, isLambdaBody, BitFlags.isStatic(access)); codeOwner = getCompanionClassName(internalName); - if (BitFlags.isSet(access, Opcodes.ACC_STATIC)) { + if (BitFlags.isStatic(access)) { // Completely move static interface methods, which requires rewriting call sites result = companion() @@ -233,8 +238,27 @@ class InterfaceDesugaring extends ClassVisitor { : null; } + @Override + public void visitOuterClass(String owner, String name, String desc) { + // Proguard gets grumpy if an outer method doesn't exist, which can be the result of moving + // interface methods to companion classes (b/68260836). In that case (for which we need to + // figure out if "owner" is an interface) need to adjust the outer method information. + if (name != null && interfaceCache.isOuterInterface(owner, internalName)) { + // Just drop outer method info. That's unfortunate, but the only alternative would be to + // change the outer method to point to the companion class, which would mean the + // reflection methods that use this information would return a companion ($$CC) class name + // as well as a possibly-modified method name and signature, so it seems better to return + // the correct original interface name and no method information. Doing this also saves + // us from doing even more work to figure out whether the method is static and a lambda + // method, which we'd need to known to adjust name and descriptor correctly. + name = null; + desc = null; + } // otherwise there's no enclosing method that could've been moved, or owner is a class + super.visitOuterClass(owner, name, desc); + } + private boolean isInterface() { - return BitFlags.isSet(accessFlags, Opcodes.ACC_INTERFACE); + return BitFlags.isInterface(accessFlags); } private static boolean isStaticInitializer(String methodName) { @@ -243,18 +267,16 @@ class InterfaceDesugaring extends ClassVisitor { private static String normalizeInterfaceMethodName( String name, boolean isLambda, boolean isStatic) { - String suffix; if (isLambda) { // Rename lambda method to reflect the new owner. Not doing so confuses LambdaDesugaring // if it's run over this class again. LambdaDesugaring has already renamed the method from // its original name to include the interface name at this point. - suffix = DependencyCollector.INTERFACE_COMPANION_SUFFIX; + return name + DependencyCollector.INTERFACE_COMPANION_SUFFIX; } else if (isStatic) { - suffix = INTERFACE_STATIC_COMPANION_METHOD_SUFFIX; + return name + INTERFACE_STATIC_COMPANION_METHOD_SUFFIX; } else { return name; } - return name + suffix; } static String getCompanionClassName(String interfaceName) { @@ -410,7 +432,7 @@ class InterfaceDesugaring extends ClassVisitor { private static class MoveJacocoFieldAccess extends MethodVisitor { public MoveJacocoFieldAccess(MethodVisitor mv) { - super(Opcodes.ASM5, mv); + super(Opcodes.ASM6, mv); } @Override diff --git a/java/com/google/devtools/build/android/desugar/Java7Compatibility.java b/java/com/google/devtools/build/android/desugar/Java7Compatibility.java index 30de63d..37a45dd 100644 --- a/java/com/google/devtools/build/android/desugar/Java7Compatibility.java +++ b/java/com/google/devtools/build/android/desugar/Java7Compatibility.java @@ -119,7 +119,7 @@ public class Java7Compatibility extends ClassVisitor { boolean updated = false; public UpdateBytecodeVersionIfNecessary(MethodVisitor methodVisitor) { - super(Opcodes.ASM5, methodVisitor); + super(Opcodes.ASM6, methodVisitor); } @Override diff --git a/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java b/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java index 652f025..5f41347 100644 --- a/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java +++ b/java/com/google/devtools/build/android/desugar/LambdaDesugaring.java @@ -533,12 +533,13 @@ class LambdaDesugaring extends ClassVisitor { i, internalName, insn.getOpcode()); - } else if (insn.getOpcode() != paramTypes[i].getOpcode(Opcodes.ILOAD)) { - // Otherwise expect load of a (effectively) final local variable. Not seeing that means - // we're dealing with a method reference on some arbitrary expression, <expression>::m. - // In that case we give up and keep using the factory method for now, since inserting - // the NEW/DUP so the new object ends up in the right stack slot is hard in that case. - // Note this still covers simple cases such as this::m or x::m, where x is a local. + } else if (!isPushForType(insn, paramTypes[i])) { + // Otherwise expect load of a (effectively) final local variable or a constant. Not seeing + // that means we're dealing with a method reference on some arbitrary expression, + // <expression>::m. In that case we give up and keep using the factory method for now, + // since inserting the NEW/DUP so the new object ends up in the right stack slot is hard + // in that case. Note this still covers simple cases such as this::m or x::m, where x is a + // local. checkState( paramTypes.length == 1, "Expected a load for %s to set up parameter %s for %s but got %s", @@ -562,6 +563,59 @@ class LambdaDesugaring extends ClassVisitor { return true; } + /** + * Returns whether a given instruction can be used to push argument of {@code type} on stack. + */ + private /* static */ boolean isPushForType(AbstractInsnNode insn, Type type) { + int opcode = insn.getOpcode(); + if (opcode == type.getOpcode(Opcodes.ILOAD)) { + return true; + } + // b/62060793: AsyncAwait rewrites bytecode to convert java methods into state machine with + // support of lambdas. Constant zero values are pushed on stack for all yet uninitialized + // local variables. And SIPUSH instruction is used to advance an internal state of a state + // machine. + switch (type.getSort()) { + case Type.BOOLEAN: + return opcode == Opcodes.ICONST_0 + || opcode == Opcodes.ICONST_1; + + case Type.BYTE: + case Type.CHAR: + case Type.SHORT: + case Type.INT: + return opcode == Opcodes.SIPUSH + || opcode == Opcodes.ICONST_0 + || opcode == Opcodes.ICONST_1 + || opcode == Opcodes.ICONST_2 + || opcode == Opcodes.ICONST_3 + || opcode == Opcodes.ICONST_4 + || opcode == Opcodes.ICONST_5 + || opcode == Opcodes.ICONST_M1; + + case Type.LONG: + return opcode == Opcodes.LCONST_0 + || opcode == Opcodes.LCONST_1; + + case Type.FLOAT: + return opcode == Opcodes.FCONST_0 + || opcode == Opcodes.FCONST_1 + || opcode == Opcodes.FCONST_2; + + case Type.DOUBLE: + return opcode == Opcodes.DCONST_0 + || opcode == Opcodes.DCONST_1; + + case Type.OBJECT: + case Type.ARRAY: + return opcode == Opcodes.ACONST_NULL; + + default: + // Support for BIPUSH and LDC* opcodes is not implemented as there is no known use case. + return false; + } + } + private Lookup createLookup(String lookupClass) throws ReflectiveOperationException { Class<?> clazz = loadFromInternal(lookupClass); Constructor<Lookup> constructor = Lookup.class.getDeclaredConstructor(Class.class); diff --git a/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java b/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java index 270d32b..e4d4da5 100644 --- a/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java +++ b/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java @@ -14,23 +14,32 @@ package com.google.devtools.build.android.desugar; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static org.objectweb.asm.Opcodes.ACC_STATIC; import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC; import static org.objectweb.asm.Opcodes.ASM6; +import static org.objectweb.asm.Opcodes.INVOKEINTERFACE; import static org.objectweb.asm.Opcodes.INVOKESTATIC; import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL; import com.google.common.base.Function; +import com.google.common.base.Preconditions; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.android.desugar.BytecodeTypeInference.InferredType; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import javax.annotation.Nullable; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.Label; import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.commons.ClassRemapper; +import org.objectweb.asm.commons.Remapper; +import org.objectweb.asm.tree.MethodNode; /** * Desugar try-with-resources. This class visitor intercepts calls to the following methods, and @@ -97,22 +106,34 @@ public class TryWithResourcesRewriter extends ClassVisitor { private final ClassLoader classLoader; private final Set<String> visitedExceptionTypes; private final AtomicInteger numOfTryWithResourcesInvoked; + /** Stores the internal class names of resources that need to be closed. */ + private final LinkedHashSet<String> resourceTypeInternalNames = new LinkedHashSet<>(); + + private final boolean hasCloseResourceMethod; + private String internalName; /** * Indicate whether the current class being desugared should be ignored. If the current class is * one of the runtime extension classes, then it should be ignored. */ private boolean shouldCurrentClassBeIgnored; + /** + * A method node for $closeResource(Throwable, AutoCloseable). At then end, we specialize this + * method node. + */ + @Nullable private MethodNode closeResourceMethod; public TryWithResourcesRewriter( ClassVisitor classVisitor, ClassLoader classLoader, Set<String> visitedExceptionTypes, - AtomicInteger numOfTryWithResourcesInvoked) { + AtomicInteger numOfTryWithResourcesInvoked, + boolean hasCloseResourceMethod) { super(ASM6, classVisitor); this.classLoader = classLoader; this.visitedExceptionTypes = visitedExceptionTypes; this.numOfTryWithResourcesInvoked = numOfTryWithResourcesInvoked; + this.hasCloseResourceMethod = hasCloseResourceMethod; } @Override @@ -126,6 +147,33 @@ public class TryWithResourcesRewriter extends ClassVisitor { super.visit(version, access, name, signature, superName, interfaces); internalName = name; shouldCurrentClassBeIgnored = THROWABLE_EXT_CLASS_INTERNAL_NAMES.contains(name); + Preconditions.checkState( + !shouldCurrentClassBeIgnored || !hasCloseResourceMethod, + "The current class which will be ignored " + + "contains $closeResource(Throwable, AutoCloseable)."); + } + + @Override + public void visitEnd() { + if (!resourceTypeInternalNames.isEmpty()) { + checkNotNull(closeResourceMethod); + for (String resourceInternalName : resourceTypeInternalNames) { + boolean isInterface = isInterface(resourceInternalName.replace('/', '.')); + // We use "this" to desugar the body of the close resource method. + closeResourceMethod.accept( + new CloseResourceMethodSpecializer(cv, resourceInternalName, isInterface)); + } + } else { + checkState( + closeResourceMethod == null, + "The field resourceTypeInternalNames is empty. " + + "But the class has the $closeResource method."); + checkState( + !hasCloseResourceMethod, + "The class %s has close resource method, but resourceTypeInternalNames is empty.", + internalName); + } + super.visitEnd(); } @Override @@ -136,21 +184,70 @@ public class TryWithResourcesRewriter extends ClassVisitor { Collections.addAll(visitedExceptionTypes, exceptions); } if (isSyntheticCloseResourceMethod(access, name, desc)) { - return null; // Discard this method. + checkState(closeResourceMethod == null, "The TWR rewriter has been used."); + closeResourceMethod = new MethodNode(ASM6, access, name, desc, signature, exceptions); + // Run the TWR desugar pass over the $closeResource(Throwable, AutoCloseable) first, for + // example, to rewrite calls to AutoCloseable.close().. + TryWithResourceVisitor twrVisitor = + new TryWithResourceVisitor( + internalName, name + desc, closeResourceMethod, classLoader, null); + return twrVisitor; } MethodVisitor visitor = super.cv.visitMethod(access, name, desc, signature, exceptions); - return visitor == null || shouldCurrentClassBeIgnored - ? visitor - : new TryWithResourceVisitor(internalName, name + desc, visitor, classLoader); + if (visitor == null || shouldCurrentClassBeIgnored) { + return visitor; + } + + BytecodeTypeInference inference = null; + if (hasCloseResourceMethod) { + /* + * BytecodeTypeInference will run after the TryWithResourceVisitor, because when we are + * processing a bytecode instruction, we need to know the types in the operand stack, which + * are inferred after the previous instruction. + */ + inference = new BytecodeTypeInference(access, internalName, name, desc); + inference.setDelegateMethodVisitor(visitor); + visitor = inference; + } + + TryWithResourceVisitor twrVisitor = + new TryWithResourceVisitor(internalName, name + desc, visitor, classLoader, inference); + return twrVisitor; } - private boolean isSyntheticCloseResourceMethod(int access, String name, String desc) { + public static boolean isSyntheticCloseResourceMethod(int access, String name, String desc) { return BitFlags.isSet(access, ACC_SYNTHETIC | ACC_STATIC) && CLOSE_RESOURCE_METHOD_NAME.equals(name) && CLOSE_RESOURCE_METHOD_DESC.equals(desc); } + private boolean isInterface(String className) { + try { + Class<?> klass = classLoader.loadClass(className); + return klass.isInterface(); + } catch (ClassNotFoundException e) { + throw new AssertionError("Failed to load class when desugaring class " + internalName); + } + } + + public static boolean isCallToSyntheticCloseResource( + String currentClassInternalName, int opcode, String owner, String name, String desc) { + if (opcode != INVOKESTATIC) { + return false; + } + if (!currentClassInternalName.equals(owner)) { + return false; + } + if (!CLOSE_RESOURCE_METHOD_NAME.equals(name)) { + return false; + } + if (!CLOSE_RESOURCE_METHOD_DESC.equals(desc)) { + return false; + } + return true; + } + private class TryWithResourceVisitor extends MethodVisitor { private final ClassLoader classLoader; @@ -158,16 +255,19 @@ public class TryWithResourcesRewriter extends ClassVisitor { private final String internalName; private final String methodSignature; + @Nullable private final BytecodeTypeInference typeInference; public TryWithResourceVisitor( String internalName, String methodSignature, MethodVisitor methodVisitor, - ClassLoader classLoader) { + ClassLoader classLoader, + @Nullable BytecodeTypeInference typeInference) { super(ASM6, methodVisitor); this.classLoader = classLoader; this.internalName = internalName; this.methodSignature = methodSignature; + this.typeInference = typeInference; } @Override @@ -180,13 +280,42 @@ public class TryWithResourcesRewriter extends ClassVisitor { @Override public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) { - if (isCallToSyntheticCloseResource(opcode, owner, name, desc)) { - // Rewrite the call to the runtime library. + if (isCallToSyntheticCloseResource(internalName, opcode, owner, name, desc)) { + checkNotNull( + typeInference, + "This method %s.%s has a call to $closeResource(Throwable, AutoCloseable) method, " + + "but the type inference is null.", + internalName, + methodSignature); + { + // Check the exception type. + InferredType exceptionClass = typeInference.getTypeOfOperandFromTop(1); + if (!exceptionClass.isNull()) { + String exceptionClassInternalName = exceptionClass.getInternalNameOrThrow(); + checkState( + isAssignableFrom( + "java.lang.Throwable", exceptionClassInternalName.replace('/', '.')), + "The exception type %s in %s.%s should be a subclass of java.lang.Throwable.", + exceptionClassInternalName, + internalName, + methodSignature); + } + } + + String resourceClassInternalName = + typeInference.getTypeOfOperandFromTop(0).getInternalNameOrThrow(); + checkState( + isAssignableFrom( + "java.lang.AutoCloseable", resourceClassInternalName.replace('/', '.')), + "The resource type should be a subclass of java.lang.AutoCloseable: %s", + resourceClassInternalName); + + resourceTypeInternalNames.add(resourceClassInternalName); super.visitMethodInsn( opcode, - THROWABLE_EXTENSION_INTERNAL_NAME, - "closeResource", - "(Ljava/lang/Throwable;Ljava/lang/Object;)V", + owner, + "$closeResource", + "(Ljava/lang/Throwable;L" + resourceClassInternalName + ";)V", itf); return; } @@ -201,23 +330,6 @@ public class TryWithResourcesRewriter extends ClassVisitor { INVOKESTATIC, THROWABLE_EXTENSION_INTERNAL_NAME, name, METHOD_DESC_MAP.get(desc), false); } - private boolean isCallToSyntheticCloseResource( - int opcode, String owner, String name, String desc) { - if (opcode != INVOKESTATIC) { - return false; - } - if (!internalName.equals(owner)) { - return false; - } - if (!CLOSE_RESOURCE_METHOD_NAME.equals(name)) { - return false; - } - if (!CLOSE_RESOURCE_METHOD_DESC.equals(desc)) { - return false; - } - return true; - } - private boolean isMethodCallTargeted(int opcode, String owner, String name, String desc) { if (opcode != INVOKEVIRTUAL) { return false; @@ -228,15 +340,80 @@ public class TryWithResourcesRewriter extends ClassVisitor { if (visitedExceptionTypes.contains(owner)) { return true; // The owner is an exception that has been visited before. } + return isAssignableFrom("java.lang.Throwable", owner.replace('/', '.')); + } + + private boolean isAssignableFrom(String baseClassName, String subClassName) { try { - Class<?> throwableClass = classLoader.loadClass("java.lang.Throwable"); - Class<?> klass = classLoader.loadClass(owner.replace('/', '.')); - return throwableClass.isAssignableFrom(klass); + Class<?> baseClass = classLoader.loadClass(baseClassName); + Class<?> subClass = classLoader.loadClass(subClassName); + return baseClass.isAssignableFrom(subClass); } catch (ClassNotFoundException e) { throw new AssertionError( - "Failed to load class when desugaring method " + internalName + "." + methodSignature, + "Failed to load class when desugaring method " + + internalName + + "." + + methodSignature + + " when checking the assignable relation for class " + + baseClassName + + " and " + + subClassName, e); } } } + + /** + * A class to specialize the method $closeResource(Throwable, AutoCloseable), which does + * + * <ul> + * <li>Rename AutoCloseable to the given concrete resource type. + * <li>Adjust the invoke instruction that calls AutoCloseable.close() + * </ul> + */ + private static class CloseResourceMethodSpecializer extends ClassRemapper { + + private final boolean isResourceAnInterface; + private final String targetResourceInternalName; + + public CloseResourceMethodSpecializer( + ClassVisitor cv, String targetResourceInternalName, boolean isResourceAnInterface) { + super( + cv, + new Remapper() { + @Override + public String map(String typeName) { + if (typeName.equals("java/lang/AutoCloseable")) { + return targetResourceInternalName; + } else { + return typeName; + } + } + }); + this.targetResourceInternalName = targetResourceInternalName; + this.isResourceAnInterface = isResourceAnInterface; + } + + @Override + public MethodVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, desc, signature, exceptions); + return new MethodVisitor(ASM6, mv) { + @Override + public void visitMethodInsn( + int opcode, String owner, String name, String desc, boolean itf) { + if (opcode == INVOKEINTERFACE + && owner.endsWith("java/lang/AutoCloseable") + && name.equals("close") + && desc.equals("()V") + && itf) { + opcode = isResourceAnInterface ? INVOKEINTERFACE : INVOKEVIRTUAL; + owner = targetResourceInternalName; + itf = isResourceAnInterface; + } + super.visitMethodInsn(opcode, owner, name, desc, itf); + } + }; + } + } } diff --git a/java/com/google/devtools/common/options/LegacyParamsFilePreProcessor.java b/java/com/google/devtools/common/options/LegacyParamsFilePreProcessor.java deleted file mode 100644 index 56a7d2c..0000000 --- a/java/com/google/devtools/common/options/LegacyParamsFilePreProcessor.java +++ /dev/null @@ -1,151 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.common.options; - -import java.io.IOException; -import java.io.Reader; -import java.nio.charset.StandardCharsets; -import java.nio.file.FileSystem; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.List; -import java.util.NoSuchElementException; - -/** - * A {@link ParamsFilePreProcessor} that processes a parameter file using a custom format. This - * format assumes each parameter is separated by whitespace and allows arguments to use single and - * double quotes and quote and whitespace escaping. - * - * <p><em>NOTE:</em> This class is deprecated; use either {@link ShellQuotedParamsFilePreProcessor} - * or {@link UnquotedParamsFilePreProcessor} depending on the format of the provided params file. - */ -@Deprecated -public class LegacyParamsFilePreProcessor extends ParamsFilePreProcessor { - - public LegacyParamsFilePreProcessor(FileSystem fs) { - super(fs); - } - - @Override - protected List<String> parse(Path paramsFile) throws IOException, OptionsParsingException { - try (Reader params = Files.newBufferedReader(paramsFile, StandardCharsets.UTF_8)) { - List<String> newArgs = new ArrayList<>(); - StringBuilder arg = new StringBuilder(); - CharIterator iterator = CharIterator.wrap(params); - while (iterator.hasNext()) { - char next = iterator.next(); - if (Character.isWhitespace(next) && !iterator.isInQuote() && !iterator.isEscaped()) { - newArgs.add(unescape(arg.toString())); - arg = new StringBuilder(); - } else { - arg.append(next); - } - } - // If there is an arg in the buffer, add it. - if (arg.length() > 0) { - newArgs.add(arg.toString()); - } - // If we're still in a quote by the end of the file, throw an error. - if (iterator.isInQuote()) { - throw new OptionsParsingException( - String.format(ERROR_MESSAGE_FORMAT, paramsFile, iterator.getUnmatchedQuoteMessage())); - } - return newArgs; - } - } - - private String unescape(String arg) { - if (arg.startsWith("'") && arg.endsWith("'")) { - String unescaped = arg.replace("'\\''", "'"); - return unescaped.substring(1, unescaped.length() - 1); - } - return arg; - } - - // Doesn't implement iterator to avoid autoboxing and to throw exceptions. - private static class CharIterator { - - private final Reader reader; - private int readerPosition = 0; - private int singleQuoteStart = -1; - private int doubleQuoteStart = -1; - private boolean escaped = false; - private char lastChar = (char) -1; - - public static CharIterator wrap(Reader reader) { - return new CharIterator(reader); - } - - public CharIterator(Reader reader) { - this.reader = reader; - } - - public boolean hasNext() throws IOException { - return peek() != -1; - } - - private int peek() throws IOException { - reader.mark(1); - int next = reader.read(); - reader.reset(); - return next; - } - - public boolean isInQuote() { - return singleQuoteStart != -1 || doubleQuoteStart != -1; - } - - public boolean isEscaped() { - return escaped; - } - - public String getUnmatchedQuoteMessage() { - StringBuilder message = new StringBuilder(); - if (singleQuoteStart != -1) { - message.append(String.format(UNFINISHED_QUOTE_MESSAGE_FORMAT, "'", singleQuoteStart)); - } - if (doubleQuoteStart != -1) { - message.append(String.format(UNFINISHED_QUOTE_MESSAGE_FORMAT, "\"", doubleQuoteStart)); - } - return message.toString(); - } - - public char next() throws IOException { - if (!hasNext()) { - throw new NoSuchElementException(); - } - char current = (char) reader.read(); - - // check for \r\n line endings. If found, drop the \r for normalized parsing. - if (current == '\r' && peek() == '\n') { - current = (char) reader.read(); - } - - // check to see if the current position is escaped - escaped = (lastChar == '\\'); - - if (!escaped && current == '\'') { - singleQuoteStart = singleQuoteStart == -1 ? readerPosition : -1; - } - if (!escaped && current == '"') { - doubleQuoteStart = doubleQuoteStart == -1 ? readerPosition : -1; - } - - readerPosition++; - lastChar = current; - return current; - } - } -} diff --git a/java/com/google/devtools/common/options/OptionDefinition.java b/java/com/google/devtools/common/options/OptionDefinition.java index 1c01932..84a9d2d 100644 --- a/java/com/google/devtools/common/options/OptionDefinition.java +++ b/java/com/google/devtools/common/options/OptionDefinition.java @@ -29,7 +29,7 @@ import java.util.Comparator; * the {@link Field} that is annotated, and should contain all logic about default settings and * behavior. */ -public class OptionDefinition { +public class OptionDefinition implements Comparable<OptionDefinition> { // TODO(b/65049598) make ConstructionException checked, which will make this checked as well. static class NotAnOptionException extends ConstructionException { @@ -304,6 +304,11 @@ public class OptionDefinition { } @Override + public int compareTo(OptionDefinition o) { + return getOptionName().compareTo(o.getOptionName()); + } + + @Override public String toString() { return String.format("option '--%s'", getOptionName()); } diff --git a/java/com/google/devtools/common/options/OptionPriority.java b/java/com/google/devtools/common/options/OptionPriority.java index 96c471e..ec5d0d8 100644 --- a/java/com/google/devtools/common/options/OptionPriority.java +++ b/java/com/google/devtools/common/options/OptionPriority.java @@ -26,18 +26,42 @@ import java.util.Objects; public class OptionPriority implements Comparable<OptionPriority> { private final PriorityCategory priorityCategory; private final int index; + private final boolean locked; - private OptionPriority(PriorityCategory priorityCategory, int index) { + private OptionPriority(PriorityCategory priorityCategory, int index, boolean locked) { this.priorityCategory = priorityCategory; this.index = index; + this.locked = locked; } - public static OptionPriority lowestOptionPriorityAtCategory(PriorityCategory category) { - return new OptionPriority(category, 0); + /** Get the first OptionPriority for that category. */ + static OptionPriority lowestOptionPriorityAtCategory(PriorityCategory category) { + return new OptionPriority(category, 0, false); } - public static OptionPriority nextOptionPriority(OptionPriority priority) { - return new OptionPriority(priority.priorityCategory, priority.index + 1); + /** + * Get the priority for the option following this one. In normal, incremental option parsing, the + * returned priority would compareTo as after the current one. Does not increment locked + * priorities. + */ + static OptionPriority nextOptionPriority(OptionPriority priority) { + if (priority.locked) { + return priority; + } + return new OptionPriority(priority.priorityCategory, priority.index + 1, false); + } + + /** + * Return a priority for this option that will avoid priority increases by calls to + * nextOptionPriority. + * + * <p>Some options are expanded in-place, and need to be all parsed at the priority of the + * original option. In this case, parsing one of these after another should not cause the option + * to be considered as higher priority than the ones before it (this would cause overlap between + * the expansion of --expansion_flag and a option following it in the same list of options). + */ + public static OptionPriority getLockedPriority(OptionPriority priority) { + return new OptionPriority(priority.priorityCategory, priority.index, true); } public PriorityCategory getPriorityCategory() { @@ -66,6 +90,11 @@ public class OptionPriority implements Comparable<OptionPriority> { return Objects.hash(priorityCategory, index); } + @Override + public String toString() { + return String.format("OptionPriority(%s,%s)", priorityCategory, index); + } + /** * The priority of option values, in order of increasing priority. * diff --git a/java/com/google/devtools/common/options/OptionValueDescription.java b/java/com/google/devtools/common/options/OptionValueDescription.java index 0d31137..616b3b5 100644 --- a/java/com/google/devtools/common/options/OptionValueDescription.java +++ b/java/com/google/devtools/common/options/OptionValueDescription.java @@ -14,7 +14,6 @@ package com.google.devtools.common.options; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; @@ -25,6 +24,7 @@ import java.util.Comparator; import java.util.List; import java.util.Map.Entry; import java.util.stream.Collectors; +import javax.annotation.Nullable; /** * The value of an option. @@ -74,6 +74,16 @@ public abstract class OptionValueDescription { } /** + * Returns the canonical instances of this option - the instances that affect the current value. + * + * <p>For options that do not have values in their own right, this should be the empty list. In + * contrast, the DefaultOptionValue does not have a canonical form at all, since it was never set, + * and is null. + */ + @Nullable + public abstract List<ParsedOptionDescription> getCanonicalInstances(); + + /** * For the given option, returns the correct type of OptionValueDescription, to which unparsed * values can be added. * @@ -103,7 +113,7 @@ public abstract class OptionValueDescription { return new DefaultOptionValueDescription(option); } - static class DefaultOptionValueDescription extends OptionValueDescription { + private static class DefaultOptionValueDescription extends OptionValueDescription { private DefaultOptionValueDescription(OptionDefinition optionDefinition) { super(optionDefinition); @@ -125,13 +135,18 @@ public abstract class OptionValueDescription { "Cannot add values to the default option value. Create a modifiable " + "OptionValueDescription using createOptionValueDescription() instead."); } + + @Override + public ImmutableList<ParsedOptionDescription> getCanonicalInstances() { + return null; + } } /** * The form of a value for a default type of flag, one that does not accumulate multiple values * and has no expansion. */ - static class SingleOptionValueDescription extends OptionValueDescription { + private static class SingleOptionValueDescription extends OptionValueDescription { private ParsedOptionDescription effectiveOptionInstance; private Object effectiveValue; @@ -183,6 +198,11 @@ public abstract class OptionValueDescription { // Output warnings if there is conflicting options set different values in a way that might // not have been obvious to the user, such as through expansions and implicit requirements. if (!effectiveValue.equals(newValue)) { + boolean samePriorityCategory = + parsedOption + .getPriority() + .getPriorityCategory() + .equals(effectiveOptionInstance.getPriority().getPriorityCategory()); if ((implicitDependent != null) && (optionThatDependsOnEffectiveValue != null)) { if (!implicitDependent.equals(optionThatDependsOnEffectiveValue)) { warnings.add( @@ -190,8 +210,7 @@ public abstract class OptionValueDescription { "%s is implicitly defined by both %s and %s", optionDefinition, optionThatDependsOnEffectiveValue, implicitDependent)); } - } else if ((implicitDependent != null) - && parsedOption.getPriority().equals(effectiveOptionInstance.getPriority())) { + } else if ((implicitDependent != null) && samePriorityCategory) { warnings.add( String.format( "%s is implicitly defined by %s; the implicitly set value " @@ -203,7 +222,7 @@ public abstract class OptionValueDescription { "A new value for %s overrides a previous implicit setting of that " + "option by %s", optionDefinition, optionThatDependsOnEffectiveValue)); - } else if ((parsedOption.getPriority().equals(effectiveOptionInstance.getPriority())) + } else if (samePriorityCategory && ((optionThatExpandedToEffectiveValue == null) && (expandedFrom != null))) { // Create a warning if an expansion option overrides an explicit option: warnings.add( @@ -227,14 +246,19 @@ public abstract class OptionValueDescription { return null; } - @VisibleForTesting - ParsedOptionDescription getEffectiveOptionInstance() { - return effectiveOptionInstance; + @Override + public ImmutableList<ParsedOptionDescription> getCanonicalInstances() { + // If the current option is an implicit requirement, we don't need to list this value since + // the parent implies it. In this case, it is sufficient to not list this value at all. + if (effectiveOptionInstance.getImplicitDependent() == null) { + return ImmutableList.of(effectiveOptionInstance); + } + return ImmutableList.of(); } } /** The form of a value for an option that accumulates multiple values on the command line. */ - static class RepeatableOptionValueDescription extends OptionValueDescription { + private static class RepeatableOptionValueDescription extends OptionValueDescription { ListMultimap<OptionPriority, ParsedOptionDescription> parsedOptions; ListMultimap<OptionPriority, Object> optionValues; @@ -252,8 +276,10 @@ public abstract class OptionValueDescription { public String getSourceString() { return parsedOptions .asMap() - .values() + .entrySet() .stream() + .sorted(Comparator.comparing(Entry::getKey)) + .map(Entry::getValue) .flatMap(Collection::stream) .map(ParsedOptionDescription::getSource) .distinct() @@ -289,6 +315,20 @@ public abstract class OptionValueDescription { } return null; } + + @Override + public ImmutableList<ParsedOptionDescription> getCanonicalInstances() { + return parsedOptions + .asMap() + .entrySet() + .stream() + .sorted(Comparator.comparing(Entry::getKey)) + .map(Entry::getValue) + .flatMap(Collection::stream) + // Only provide the options that aren't implied elsewhere. + .filter(optionDesc -> optionDesc.getImplicitDependent() == null) + .collect(ImmutableList.toImmutableList()); + } } /** @@ -296,7 +336,7 @@ public abstract class OptionValueDescription { * in place to other options. This should be used for both flags with a static expansion defined * in {@link Option#expansion()} and flags with an {@link Option#expansionFunction()}. */ - static class ExpansionOptionValueDescription extends OptionValueDescription { + private static class ExpansionOptionValueDescription extends OptionValueDescription { private final List<String> expansion; private ExpansionOptionValueDescription( @@ -337,10 +377,18 @@ public abstract class OptionValueDescription { : String.format( "expanded from %s (source %s)", optionDefinition, parsedOption.getSource())); } + + @Override + public ImmutableList<ParsedOptionDescription> getCanonicalInstances() { + // The options this expands to are incorporated in their own right - this option does + // not have a canonical form. + return ImmutableList.of(); + } } /** The form of a value for a flag with implicit requirements. */ - static class OptionWithImplicitRequirementsValueDescription extends SingleOptionValueDescription { + private static class OptionWithImplicitRequirementsValueDescription + extends SingleOptionValueDescription { private OptionWithImplicitRequirementsValueDescription(OptionDefinition optionDefinition) { super(optionDefinition); @@ -384,7 +432,7 @@ public abstract class OptionValueDescription { } /** Form for options that contain other options in the value text to which they expand. */ - static final class WrapperOptionValueDescription extends OptionValueDescription { + private static final class WrapperOptionValueDescription extends OptionValueDescription { WrapperOptionValueDescription(OptionDefinition optionDefinition) { super(optionDefinition); @@ -418,6 +466,13 @@ public abstract class OptionValueDescription { : String.format( "unwrapped from %s (source %s)", optionDefinition, parsedOption.getSource())); } + + @Override + public ImmutableList<ParsedOptionDescription> getCanonicalInstances() { + // No wrapper options get listed in the canonical form - the options they are wrapping will + // be in the right place. + return ImmutableList.of(); + } } } diff --git a/java/com/google/devtools/common/options/OptionsParser.java b/java/com/google/devtools/common/options/OptionsParser.java index f84ee47..fb7161c 100644 --- a/java/com/google/devtools/common/options/OptionsParser.java +++ b/java/com/google/devtools/common/options/OptionsParser.java @@ -549,7 +549,7 @@ public class OptionsParser implements OptionsProvider { * or null if the value has not been set. * @throws IllegalArgumentException if there is no option by the given name. */ - OptionValueDescription getOptionValueDescription(String name) { + public OptionValueDescription getOptionValueDescription(String name) { return impl.getOptionValueDescription(name); } @@ -619,6 +619,19 @@ public class OptionsParser implements OptionsProvider { } } + public void parseOptionsFixedAtSpecificPriority( + OptionPriority priority, String source, List<String> args) throws OptionsParsingException { + Preconditions.checkNotNull(priority, "Priority not specified for arglist " + args); + Preconditions.checkArgument( + priority.getPriorityCategory() != OptionPriority.PriorityCategory.DEFAULT, + "Priority cannot be default, which was specified for arglist " + args); + residue.addAll(impl.parseOptionsFixedAtSpecificPriority(priority, o -> source, args)); + if (!allowResidue && !residue.isEmpty()) { + String errorMsg = "Unrecognized arguments: " + Joiner.on(' ').join(residue); + throw new OptionsParsingException(errorMsg); + } + } + /** * @param origin the origin of this option instance, it includes the priority of the value. If * other values have already been or will be parsed at a higher priority, they might override @@ -652,9 +665,7 @@ public class OptionsParser implements OptionsProvider { return ImmutableList.copyOf(residue); } - /** - * Returns a list of warnings about problems encountered by previous parse calls. - */ + /** Returns a list of warnings about problems encountered by previous parse calls. */ public List<String> getWarnings() { return impl.getWarnings(); } @@ -680,7 +691,12 @@ public class OptionsParser implements OptionsProvider { } @Override - public List<OptionValueDescription> asListOfEffectiveOptions() { + public List<ParsedOptionDescription> asListOfCanonicalOptions() { + return impl.asCanonicalizedListOfParsedOptions(); + } + + @Override + public List<OptionValueDescription> asListOfOptionValues() { return impl.asListOfEffectiveOptions(); } @@ -787,8 +803,7 @@ public class OptionsParser implements OptionsProvider { * Option} annotation. */ private static void validateFieldsSets( - Class<? extends OptionsBase> optionsClass, - LinkedHashSet<Field> fieldsFromMap) { + Class<? extends OptionsBase> optionsClass, LinkedHashSet<Field> fieldsFromMap) { ImmutableList<OptionDefinition> optionDefsFromClasses = OptionsData.getAllOptionDefinitionsForClass(optionsClass); Set<Field> fieldsFromClass = diff --git a/java/com/google/devtools/common/options/OptionsParserImpl.java b/java/com/google/devtools/common/options/OptionsParserImpl.java index b543328..496927b 100644 --- a/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -21,18 +21,19 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; -import com.google.common.collect.LinkedHashMultimap; -import com.google.common.collect.Multimap; import com.google.devtools.common.options.OptionPriority.PriorityCategory; import com.google.devtools.common.options.OptionValueDescription.ExpansionBundle; import com.google.devtools.common.options.OptionsParser.OptionDescription; import java.lang.reflect.Constructor; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.annotation.Nullable; /** @@ -68,28 +69,21 @@ class OptionsParserImpl { */ private final List<ParsedOptionDescription> parsedOptions = new ArrayList<>(); + private final List<String> warnings = new ArrayList<>(); + /** - * The options for use with the canonicalize command are stored separately from parsedOptions so - * that invocation policy can modify the values for canonicalization (e.g. override user-specified - * values with default values) without corrupting the data used to represent the user's original - * invocation for {@link #asListOfExplicitOptions()} and {@link #asCompleteListOfParsedOptions()}. - * A LinkedHashMultimap is used so that canonicalization happens in the correct order and multiple - * values can be stored for flags that allow multiple values. + * Since parse() expects multiple calls to it with the same {@link PriorityCategory} to be treated + * as though the args in the later call have higher priority over the earlier calls, we need to + * track the high water mark of option priority at each category. Each call to parse will start at + * this level. */ - private final Multimap<OptionDefinition, ParsedOptionDescription> canonicalizeValues = - LinkedHashMultimap.create(); - - private final List<String> warnings = new ArrayList<>(); + private final Map<PriorityCategory, OptionPriority> nextPriorityPerPriorityCategory = + Stream.of(PriorityCategory.values()) + .collect(Collectors.toMap(p -> p, OptionPriority::lowestOptionPriorityAtCategory)); private boolean allowSingleDashLongOptions = false; - private ArgsPreProcessor argsPreProcessor = - new ArgsPreProcessor() { - @Override - public List<String> preProcess(List<String> args) throws OptionsParsingException { - return args; - } - }; + private ArgsPreProcessor argsPreProcessor = args -> args; /** Create a new parser object. Do not accept a null OptionsData object. */ OptionsParserImpl(OptionsData optionsData) { @@ -135,36 +129,26 @@ class OptionsParserImpl { .collect(toCollection(ArrayList::new)); } - /** - * Implements {@link OptionsParser#canonicalize}. - */ + /** Implements {@link OptionsParser#canonicalize}. */ List<String> asCanonicalizedList() { - return canonicalizeValues - .values() + return asCanonicalizedListOfParsedOptions() .stream() - // Sort implicit requirement options to the end, keeping their existing order, and sort - // the other options alphabetically. - .sorted( - (v1, v2) -> { - if (v1.getOptionDefinition().hasImplicitRequirements()) { - return v2.getOptionDefinition().hasImplicitRequirements() ? 0 : 1; - } - if (v2.getOptionDefinition().hasImplicitRequirements()) { - return -1; - } - return v1.getOptionDefinition() - .getOptionName() - .compareTo(v2.getOptionDefinition().getOptionName()); - }) - // Ignore expansion options. - .filter(value -> !value.getOptionDefinition().isExpansionOption()) .map(ParsedOptionDescription::getDeprecatedCanonicalForm) - .collect(toCollection(ArrayList::new)); + .collect(ImmutableList.toImmutableList()); } - /** - * Implements {@link OptionsParser#asListOfEffectiveOptions()}. - */ + /** Implements {@link OptionsParser#canonicalize}. */ + List<ParsedOptionDescription> asCanonicalizedListOfParsedOptions() { + return optionValues + .keySet() + .stream() + .sorted() + .map(optionDefinition -> optionValues.get(optionDefinition).getCanonicalInstances()) + .flatMap(Collection::stream) + .collect(ImmutableList.toImmutableList()); + } + + /** Implements {@link OptionsParser#asListOfOptionValues()}. */ List<OptionValueDescription> asListOfEffectiveOptions() { List<OptionValueDescription> result = new ArrayList<>(); for (Map.Entry<String, OptionDefinition> mapEntry : optionsData.getAllOptionDefinitions()) { @@ -196,8 +180,6 @@ class OptionsParserImpl { OptionValueDescription clearValue(OptionDefinition optionDefinition) throws OptionsParsingException { - // Actually remove the value from various lists tracking effective options. - canonicalizeValues.removeAll(optionDefinition); return optionValues.remove(optionDefinition); } @@ -282,12 +264,33 @@ class OptionsParserImpl { * order. */ List<String> parse( - OptionPriority.PriorityCategory priority, + PriorityCategory priorityCat, Function<OptionDefinition, String> sourceFunction, List<String> args) throws OptionsParsingException { - return parse( - OptionPriority.lowestOptionPriorityAtCategory(priority), sourceFunction, null, null, args); + ResidueAndPriority residueAndPriority = + parse(nextPriorityPerPriorityCategory.get(priorityCat), sourceFunction, null, null, args); + nextPriorityPerPriorityCategory.put(priorityCat, residueAndPriority.nextPriority); + return residueAndPriority.residue; + } + + private static final class ResidueAndPriority { + List<String> residue; + OptionPriority nextPriority; + + public ResidueAndPriority(List<String> residue, OptionPriority nextPriority) { + this.residue = residue; + this.nextPriority = nextPriority; + } + } + + /** Parses the args at the fixed priority. */ + List<String> parseOptionsFixedAtSpecificPriority( + OptionPriority priority, Function<OptionDefinition, String> sourceFunction, List<String> args) + throws OptionsParsingException { + ResidueAndPriority residueAndPriority = + parse(OptionPriority.getLockedPriority(priority), sourceFunction, null, null, args); + return residueAndPriority.residue; } /** @@ -298,7 +301,7 @@ class OptionsParserImpl { * <p>The method treats options that have neither an implicitDependent nor an expandedFrom value * as explicitly set. */ - private List<String> parse( + private ResidueAndPriority parse( OptionPriority priority, Function<OptionDefinition, String> sourceFunction, OptionDefinition implicitDependent, @@ -325,6 +328,7 @@ class OptionsParserImpl { identifyOptionAndPossibleArgument( arg, argsIterator, priority, sourceFunction, implicitDependent, expandedFrom); handleNewParsedOption(parsedOption); + priority = OptionPriority.nextOptionPriority(priority); } // Go through the final values and make sure they are valid values for their option. Unlike any @@ -334,7 +338,7 @@ class OptionsParserImpl { valueDescription.getValue(); } - return unparsedArgs; + return new ResidueAndPriority(unparsedArgs, priority); } /** @@ -401,28 +405,23 @@ class OptionsParserImpl { // Log explicit options and expanded options in the order they are parsed (can be sorted // later). This information is needed to correctly canonicalize flags. parsedOptions.add(parsedOption); - if (optionDefinition.allowsMultiple()) { - canonicalizeValues.put(optionDefinition, parsedOption); - } else { - canonicalizeValues.replaceValues(optionDefinition, ImmutableList.of(parsedOption)); - } } if (expansionBundle != null) { - List<String> unparsed = + ResidueAndPriority residueAndPriority = parse( - parsedOption.getPriority(), + OptionPriority.getLockedPriority(parsedOption.getPriority()), o -> expansionBundle.sourceOfExpansionArgs, optionDefinition.hasImplicitRequirements() ? optionDefinition : null, optionDefinition.isExpansionOption() ? optionDefinition : null, expansionBundle.expansionArgs); - if (!unparsed.isEmpty()) { + if (!residueAndPriority.residue.isEmpty()) { if (optionDefinition.isWrapperOption()) { throw new OptionsParsingException( "Unparsed options remain after unwrapping " + unconvertedValue + ": " - + Joiner.on(' ').join(unparsed)); + + Joiner.on(' ').join(residueAndPriority.residue)); } else { // Throw an assertion here, because this indicates an error in the definition of this // option's expansion or requirements, not with the input as provided by the user. @@ -430,7 +429,7 @@ class OptionsParserImpl { "Unparsed options remain after processing " + unconvertedValue + ": " - + Joiner.on(' ').join(unparsed)); + + Joiner.on(' ').join(residueAndPriority.residue)); } } } diff --git a/java/com/google/devtools/common/options/OptionsProvider.java b/java/com/google/devtools/common/options/OptionsProvider.java index 5fd8ac0..d467fe5 100644 --- a/java/com/google/devtools/common/options/OptionsProvider.java +++ b/java/com/google/devtools/common/options/OptionsProvider.java @@ -57,10 +57,20 @@ public interface OptionsProvider extends OptionsClassProvider { List<ParsedOptionDescription> asListOfExplicitOptions(); /** - * Returns a list of all options, including undocumented ones, and their - * effective values. There is no guaranteed ordering for the result. + * Returns a list of the parsed options whose values are in the final value of the option, i.e. + * the options that were added explicitly, expanded if necessary to the valued options they + * affect. This will not include values that were set and then overridden by a later value of the + * same option. + * + * <p>The list includes undocumented options. + */ + List<ParsedOptionDescription> asListOfCanonicalOptions(); + + /** + * Returns a list of all options, including undocumented ones, and their effective values. There + * is no guaranteed ordering for the result. */ - List<OptionValueDescription> asListOfEffectiveOptions(); + List<OptionValueDescription> asListOfOptionValues(); /** * Canonicalizes the list of options that this OptionsParser has parsed. The |