Merge remote-tracking branch 'aosp/upstream-master' into master
* aosp/upstream-master:
Migrate the options parser to java8 functions.
In UseBridge.class, check whether the owner of the method call instruction and the owner of the method reference have assignable relation. If yes, use the bridge method.
Move the DurationConverter to the common.options package
Bug: 62218600
Test: m -j checkbuild
Change-Id: If718ef705cddd4dd6ce0b3390633f0d3921a076a
diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java
index cbc7834..3feda5a 100644
--- a/java/com/google/devtools/build/android/desugar/Desugar.java
+++ b/java/com/google/devtools/build/android/desugar/Desugar.java
@@ -505,6 +505,7 @@
visitor,
lambdaClass,
bridgeMethodReader,
+ loader,
interfaceLambdaMethods,
allowDefaultMethods,
outputJava7);
diff --git a/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java b/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
index 5e50fc8..630559e 100644
--- a/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
+++ b/java/com/google/devtools/build/android/desugar/LambdaClassFixer.java
@@ -35,7 +35,7 @@
* Visitor intended to fix up lambda classes to match assumptions made in {@link LambdaDesugaring}.
* Specifically this includes fixing visibilities and generating any missing factory methods.
*
- * <p>Each instance can only visit one class. This is because the signature of the needed factory
+ * <p>Each instance can only visit one class. This is because the signature of the needed factory
* method is passed into the constructor.
*/
class LambdaClassFixer extends ClassVisitor {
@@ -50,6 +50,7 @@
private final ImmutableSet<String> interfaceLambdaMethods;
private final boolean allowDefaultMethods;
private final boolean copyBridgeMethods;
+ private final ClassLoader classLoader;
private final HashSet<String> implementedMethods = new HashSet<>();
private final LinkedHashSet<String> methodsToMoveIn = new LinkedHashSet<>();
@@ -62,14 +63,20 @@
private String desc;
private String signature;
- public LambdaClassFixer(ClassVisitor dest, LambdaInfo lambdaInfo, ClassReaderFactory factory,
- ImmutableSet<String> interfaceLambdaMethods, boolean allowDefaultMethods,
+ public LambdaClassFixer(
+ ClassVisitor dest,
+ LambdaInfo lambdaInfo,
+ ClassReaderFactory factory,
+ ClassLoader classLoader,
+ ImmutableSet<String> interfaceLambdaMethods,
+ boolean allowDefaultMethods,
boolean copyBridgeMethods) {
super(Opcodes.ASM5, dest);
checkArgument(!allowDefaultMethods || interfaceLambdaMethods.isEmpty());
checkArgument(allowDefaultMethods || copyBridgeMethods);
this.lambdaInfo = lambdaInfo;
this.factory = factory;
+ this.classLoader = classLoader;
this.interfaceLambdaMethods = interfaceLambdaMethods;
this.allowDefaultMethods = allowDefaultMethods;
this.copyBridgeMethods = copyBridgeMethods;
@@ -105,7 +112,8 @@
@Override
public MethodVisitor visitMethod(
int access, String name, String desc, String signature, String[] exceptions) {
- if (name.equals("writeReplace") && BitFlags.noneSet(access, Opcodes.ACC_STATIC)
+ if (name.equals("writeReplace")
+ && BitFlags.noneSet(access, Opcodes.ACC_STATIC)
&& desc.equals("()Ljava/lang/Object;")) {
// Lambda serialization hooks use java/lang/invoke/SerializedLambda, which isn't available on
// Android. Since Jack doesn't do anything special for serializable lambdas we just drop these
@@ -137,7 +145,8 @@
if (!lambdaInfo.bridgeMethod().equals(lambdaInfo.methodReference())) {
// Skip UseBridgeMethod unless we actually need it
methodVisitor =
- new UseBridgeMethod(methodVisitor, lambdaInfo, access, name, desc, signature, exceptions);
+ new UseBridgeMethod(
+ methodVisitor, lambdaInfo, classLoader, access, name, desc, signature, exceptions);
}
if (!FACTORY_METHOD_NAME.equals(name) && !"<init>".equals(name)) {
methodVisitor = new LambdaClassInvokeSpecialRewriter(methodVisitor);
@@ -147,36 +156,42 @@
@Override
public void visitEnd() {
- checkState(!hasState || hasFactory,
- "Expected factory method for capturing lambda %s", getInternalName());
+ checkState(
+ !hasState || hasFactory,
+ "Expected factory method for capturing lambda %s",
+ getInternalName());
if (!hasState) {
- checkState(signature == null,
- "Didn't expect generic constructor signature %s %s", getInternalName(), signature);
- checkState(lambdaInfo.factoryMethodDesc().startsWith("()"),
- "Expected 0-arg factory method for %s but found %s", getInternalName(),
+ checkState(
+ signature == null,
+ "Didn't expect generic constructor signature %s %s",
+ getInternalName(),
+ signature);
+ checkState(
+ lambdaInfo.factoryMethodDesc().startsWith("()"),
+ "Expected 0-arg factory method for %s but found %s",
+ getInternalName(),
lambdaInfo.factoryMethodDesc());
// Since this is a stateless class we populate and use a static singleton field "$instance".
// Field is package-private so we can read it from the class that had the invokedynamic.
String singletonFieldDesc = lambdaInfo.factoryMethodDesc().substring("()".length());
super.visitField(
- Opcodes.ACC_STATIC | Opcodes.ACC_FINAL,
- SINGLETON_FIELD_NAME,
- singletonFieldDesc,
- (String) null,
- (Object) null)
+ Opcodes.ACC_STATIC | Opcodes.ACC_FINAL,
+ SINGLETON_FIELD_NAME,
+ singletonFieldDesc,
+ (String) null,
+ (Object) null)
.visitEnd();
MethodVisitor codeBuilder =
- super.visitMethod(
- Opcodes.ACC_STATIC,
- "<clinit>",
- "()V",
- (String) null,
- new String[0]);
+ super.visitMethod(Opcodes.ACC_STATIC, "<clinit>", "()V", (String) null, new String[0]);
codeBuilder.visitTypeInsn(Opcodes.NEW, getInternalName());
codeBuilder.visitInsn(Opcodes.DUP);
- codeBuilder.visitMethodInsn(Opcodes.INVOKESPECIAL, getInternalName(), "<init>",
- checkNotNull(desc, "didn't see a constructor for %s", getInternalName()), /*itf*/ false);
+ codeBuilder.visitMethodInsn(
+ Opcodes.INVOKESPECIAL,
+ getInternalName(),
+ "<init>",
+ checkNotNull(desc, "didn't see a constructor for %s", getInternalName()),
+ /*itf=*/ false);
codeBuilder.visitFieldInsn(
Opcodes.PUTSTATIC, getInternalName(), SINGLETON_FIELD_NAME, singletonFieldDesc);
codeBuilder.visitInsn(Opcodes.RETURN);
@@ -199,8 +214,11 @@
for (String rewritten : methodsToMoveIn) {
String interfaceInternalName = rewritten.substring(0, rewritten.indexOf('#'));
String methodName = rewritten.substring(interfaceInternalName.length() + 1);
- ClassReader bytecode = checkNotNull(factory.readIfKnown(interfaceInternalName),
- "Couldn't load interface with lambda method %s", rewritten);
+ ClassReader bytecode =
+ checkNotNull(
+ factory.readIfKnown(interfaceInternalName),
+ "Couldn't load interface with lambda method %s",
+ rewritten);
CopyOneMethod copier = new CopyOneMethod(methodName);
// TODO(kmb): Set source file attribute for lambda classes so lambda debug info makes sense
bytecode.accept(copier, ClassReader.SKIP_DEBUG);
@@ -219,9 +237,7 @@
}
}
- /**
- * Rewriter for methods in generated lambda classes.
- */
+ /** Rewriter for methods in generated lambda classes. */
private class LambdaClassMethodRewriter extends MethodVisitor {
public LambdaClassMethodRewriter(MethodVisitor dest) {
super(Opcodes.ASM5, dest);
@@ -304,7 +320,8 @@
*/
private class CopyBridgeMethods extends ClassVisitor {
- @SuppressWarnings("hiding") private ImmutableList<String> interfaces;
+ @SuppressWarnings("hiding")
+ private ImmutableList<String> interfaces;
public CopyBridgeMethods() {
// No delegate visitor; instead we'll add methods to the outer class's delegate where needed
@@ -393,8 +410,8 @@
*
* <p>This class should only be used to visit interface methods and assumes that the code in
* {@code $jacocoInit()} is always executed in the interface's static initializer, which is the
- * case in the absence of hand-written static or default interface methods (which
- * {@link Java7Compatibility} makes sure of).
+ * case in the absence of hand-written static or default interface methods (which {@link
+ * Java7Compatibility} makes sure of).
*/
private static class AvoidJacocoInit extends MethodVisitor {
public AvoidJacocoInit(MethodVisitor dest) {
@@ -416,32 +433,61 @@
private final MethodVisitor dest;
private final LambdaInfo lambdaInfo;
+ private final ClassLoader classLoader;
- public UseBridgeMethod(MethodVisitor dest, LambdaInfo lambdaInfo,
- int access, String name, String desc, String signature, String[] exceptions) {
+ public UseBridgeMethod(
+ MethodVisitor dest,
+ LambdaInfo lambdaInfo,
+ ClassLoader classLoader,
+ int access,
+ String name,
+ String desc,
+ String signature,
+ String[] exceptions) {
super(Opcodes.ASM5, access, name, desc, signature, exceptions);
this.dest = dest;
this.lambdaInfo = lambdaInfo;
+ this.classLoader = classLoader;
+ checkArgument(
+ !lambdaInfo.methodReference().equals(lambdaInfo.bridgeMethod()),
+ "This class only works for a lambda that has a bridge method. lambdaInfo=%s, bridge=%s",
+ lambdaInfo.methodReference(),
+ lambdaInfo.bridgeMethod());
}
@Override
public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
- if (owner.equals(lambdaInfo.methodReference().getOwner())
- && name.equals(lambdaInfo.methodReference().getName())
- && desc.equals(lambdaInfo.methodReference().getDesc())) {
+ if (!name.equals(lambdaInfo.methodReference().getName())
+ || !desc.equals(lambdaInfo.methodReference().getDesc())) {
+ super.visitMethodInsn(opcode, owner, name, desc, itf);
+ return;
+ }
+
+ boolean useBridgeMethod = false;
+ if (owner.equals(lambdaInfo.methodReference().getOwner())) {
if (lambdaInfo.methodReference().getTag() == Opcodes.H_NEWINVOKESPECIAL
&& lambdaInfo.bridgeMethod().getTag() != Opcodes.H_NEWINVOKESPECIAL) {
// We're changing a constructor call to a factory method call, so we unfortunately need
// to go find the NEW/DUP pair preceding the constructor call and remove it
removeLastAllocation();
}
+ useBridgeMethod = true;
+ } else if ((lambdaInfo.methodReference().getTag() == Opcodes.H_INVOKEVIRTUAL
+ || lambdaInfo.methodReference().getTag() == Opcodes.H_INVOKESPECIAL)
+ && hasAssignableRelation(owner, lambdaInfo.methodReference().getOwner())) {
+ // For rewriting instance methods calls, we consider the class hierarchy.
+ // This is for JDK 9: (b/62218600).
+ // TODO(cnsun): revisit this to make sure Desugar is fully compatible with this change
+ // in JDK: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/a3b3c7b6464d
+ useBridgeMethod = true;
+ }
+ if (useBridgeMethod) {
super.visitMethodInsn(
LambdaDesugaring.invokeOpcode(lambdaInfo.bridgeMethod()),
lambdaInfo.bridgeMethod().getOwner(),
lambdaInfo.bridgeMethod().getName(),
lambdaInfo.bridgeMethod().getDesc(),
lambdaInfo.bridgeMethod().isInterface());
-
} else {
super.visitMethodInsn(opcode, owner, name, desc, itf);
}
@@ -451,7 +497,8 @@
AbstractInsnNode insn = instructions.getLast();
while (insn != null && insn.getPrevious() != null) {
AbstractInsnNode prev = insn.getPrevious();
- if (prev.getOpcode() == Opcodes.NEW && insn.getOpcode() == Opcodes.DUP
+ if (prev.getOpcode() == Opcodes.NEW
+ && insn.getOpcode() == Opcodes.DUP
&& ((TypeInsnNode) prev).desc.equals(lambdaInfo.methodReference().getOwner())) {
instructions.remove(prev);
instructions.remove(insn);
@@ -463,6 +510,19 @@
"Couldn't find allocation to rewrite ::new reference " + lambdaInfo.methodReference());
}
+ private boolean hasAssignableRelation(String ownerOfMethodInsn, String ownerOfMethodReference) {
+ try {
+ Class<?> methodInsnOwnerClass = classLoader.loadClass(ownerOfMethodInsn.replace('/', '.'));
+ Class<?> methodReferenceOwnerClass =
+ classLoader.loadClass(ownerOfMethodReference.replace('/', '.'));
+ return methodInsnOwnerClass.isAssignableFrom(methodReferenceOwnerClass)
+ || methodReferenceOwnerClass.isAssignableFrom(methodInsnOwnerClass);
+ } catch (ClassNotFoundException e) {
+ throw new IllegalStateException(
+ "Failed to load method owners for inserting bridge method: " + lambdaInfo, e);
+ }
+ }
+
@Override
public void visitEnd() {
accept(dest);
diff --git a/java/com/google/devtools/common/options/Converters.java b/java/com/google/devtools/common/options/Converters.java
index 4584f80..aacc64b 100644
--- a/java/com/google/devtools/common/options/Converters.java
+++ b/java/com/google/devtools/common/options/Converters.java
@@ -16,10 +16,12 @@
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps;
+import java.time.Duration;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.logging.Level;
+import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
@@ -178,6 +180,47 @@
}
/**
+ * Standard converter for the {@link java.time.Duration} type.
+ */
+ public static class DurationConverter implements Converter<Duration> {
+ private final Pattern durationRegex = Pattern.compile("^([0-9]+)(d|h|m|s|ms)$");
+
+ @Override
+ public Duration convert(String input) throws OptionsParsingException {
+ // To be compatible with the previous parser, '0' doesn't need a unit.
+ if ("0".equals(input)) {
+ return Duration.ZERO;
+ }
+ Matcher m = durationRegex.matcher(input);
+ if (!m.matches()) {
+ throw new OptionsParsingException("Illegal duration '" + input + "'.");
+ }
+ long duration = Long.parseLong(m.group(1));
+ String unit = m.group(2);
+ switch(unit) {
+ case "d":
+ return Duration.ofDays(duration);
+ case "h":
+ return Duration.ofHours(duration);
+ case "m":
+ return Duration.ofMinutes(duration);
+ case "s":
+ return Duration.ofSeconds(duration);
+ case "ms":
+ return Duration.ofMillis(duration);
+ default:
+ throw new IllegalStateException("This must not happen. Did you update the regex without "
+ + "the switch case?");
+ }
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "An immutable length of time.";
+ }
+ }
+
+ /**
* The converters that are available to the options parser by default. These are used if the
* {@code @Option} annotation does not specify its own {@code converter}, and its type is one of
* the following.
@@ -185,12 +228,14 @@
static final Map<Class<?>, Converter<?>> DEFAULT_CONVERTERS = Maps.newHashMap();
static {
+ // 1:1 correspondence with UsesOnlyCoreTypes.CORE_TYPES.
DEFAULT_CONVERTERS.put(String.class, new Converters.StringConverter());
DEFAULT_CONVERTERS.put(int.class, new Converters.IntegerConverter());
DEFAULT_CONVERTERS.put(long.class, new Converters.LongConverter());
DEFAULT_CONVERTERS.put(double.class, new Converters.DoubleConverter());
DEFAULT_CONVERTERS.put(boolean.class, new Converters.BooleanConverter());
DEFAULT_CONVERTERS.put(TriState.class, new Converters.TriStateConverter());
+ DEFAULT_CONVERTERS.put(Duration.class, new Converters.DurationConverter());
DEFAULT_CONVERTERS.put(Void.class, new Converters.VoidConverter());
}
diff --git a/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java b/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
index 97be190..562221d 100644
--- a/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -13,8 +13,6 @@
// limitations under the License.
package com.google.devtools.common.options;
-import com.google.common.base.Function;
-import com.google.common.base.Functions;
import com.google.common.base.Joiner;
import com.google.common.base.Verify;
import com.google.common.collect.ArrayListMultimap;
@@ -38,6 +36,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
@@ -53,8 +52,7 @@
private static final Logger log = Logger.getLogger(InvocationPolicyEnforcer.class.getName());
- private static final Function<Object, String> INVOCATION_POLICY_SOURCE =
- Functions.constant("Invocation policy");
+ private static final Function<Object, String> INVOCATION_POLICY_SOURCE = o -> "Invocation policy";
@Nullable private final InvocationPolicy invocationPolicy;
diff --git a/java/com/google/devtools/common/options/OptionsParser.java b/java/com/google/devtools/common/options/OptionsParser.java
index fdfa256..b507785 100644
--- a/java/com/google/devtools/common/options/OptionsParser.java
+++ b/java/com/google/devtools/common/options/OptionsParser.java
@@ -14,8 +14,6 @@
package com.google.devtools.common.options;
-import com.google.common.base.Function;
-import com.google.common.base.Functions;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -34,6 +32,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.function.Function;
import javax.annotation.Nullable;
/**
@@ -710,7 +709,7 @@
*/
public void parse(OptionPriority priority, String source,
List<String> args) throws OptionsParsingException {
- parseWithSourceFunction(priority, Functions.constant(source), args);
+ parseWithSourceFunction(priority, o -> source, args);
}
/**
diff --git a/java/com/google/devtools/common/options/OptionsParserImpl.java b/java/com/google/devtools/common/options/OptionsParserImpl.java
index b4ae458..d49d5c4 100644
--- a/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -14,8 +14,6 @@
package com.google.devtools.common.options;
-import com.google.common.base.Function;
-import com.google.common.base.Functions;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
@@ -40,6 +38,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.function.Function;
import javax.annotation.Nullable;
/**
@@ -475,13 +474,14 @@
if (option.wrapperOption()) {
if (value.startsWith("-")) {
-
- List<String> unparsed = parse(
- priority,
- Functions.constant("Unwrapped from wrapper option --" + originalName),
- null, // implicitDependent
- null, // expandedFrom
- ImmutableList.of(value));
+ String sourceMessage = "Unwrapped from wrapper option --" + originalName;
+ List<String> unparsed =
+ parse(
+ priority,
+ o -> sourceMessage,
+ null, // implicitDependent
+ null, // expandedFrom
+ ImmutableList.of(value));
if (!unparsed.isEmpty()) {
throw new OptionsParsingException(
@@ -526,12 +526,11 @@
if (OptionsData.isExpansionOption(field.getAnnotation(Option.class))) {
ImmutableList<String> expansion = optionsData.getEvaluatedExpansion(field, value);
- Function<Object, String> expansionSourceFunction =
- Functions.constant(
- "expanded from option --"
- + originalName
- + " from "
- + sourceFunction.apply(originalName));
+ String sourceMessage = "expanded from option --"
+ + originalName
+ + " from "
+ + sourceFunction.apply(originalName);
+ Function<Object, String> expansionSourceFunction = o -> sourceMessage;
maybeAddDeprecationWarning(field);
List<String> unparsed =
parse(priority, expansionSourceFunction, null, originalName, expansion);
@@ -582,12 +581,13 @@
// TODO(bazel-team): this should happen when the option is encountered.
if (!implicitRequirements.isEmpty()) {
for (Map.Entry<String, List<String>> entry : implicitRequirements.entrySet()) {
+ String sourceMessage =
+ "implicit requirement of option --"
+ + entry.getKey()
+ + " from "
+ + sourceFunction.apply(entry.getKey());
Function<Object, String> requirementSourceFunction =
- Functions.constant(
- "implicit requirement of option --"
- + entry.getKey()
- + " from "
- + sourceFunction.apply(entry.getKey()));
+ o -> sourceMessage;
List<String> unparsed = parse(priority, requirementSourceFunction, entry.getKey(), null,
entry.getValue());
diff --git a/java/com/google/devtools/common/options/UsesOnlyCoreTypes.java b/java/com/google/devtools/common/options/UsesOnlyCoreTypes.java
index 6f2714f..c40495d 100644
--- a/java/com/google/devtools/common/options/UsesOnlyCoreTypes.java
+++ b/java/com/google/devtools/common/options/UsesOnlyCoreTypes.java
@@ -20,6 +20,7 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
+import java.time.Duration;
import java.util.List;
/**
@@ -52,6 +53,7 @@
double.class,
boolean.class,
TriState.class,
- Void.class
+ Void.class,
+ Duration.class
);
}
