From b40221e4f50007ca4c1b59e6300ce6c7c1f5a1fb Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 8 Mar 2017 21:58:38 +0000 Subject: Desugar calls to Objects.requireNonNull(Object o) to o.getClass() Only requireNonNull(Object o) is desugared. The following two overloaded versions are NOT desugared. requireNonNUll(Object o, String msg) requireNonNull(Object o, Supplier msg) RELNOTES: desugar calls to Objects.requireNonNull(Object o) with o.getClass() for android -- PiperOrigin-RevId: 149579668 MOS_MIGRATED_REVID=149579668 GitOrigin-RevId: b35a0c086152ac6e5e0e695b21cacfd61de68b51 Change-Id: I1ef312fb5740c2171b4931745adc13125e2a10bf --- .../devtools/build/android/desugar/Desugar.java | 136 ++++++++++++--------- .../ObjectsRequireNonNullMethodInliner.java | 68 +++++++++++ 2 files changed, 148 insertions(+), 56 deletions(-) create mode 100644 java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java index 9dc07d7..36d9f55 100644 --- a/java/com/google/devtools/build/android/desugar/Desugar.java +++ b/java/com/google/devtools/build/android/desugar/Desugar.java @@ -50,55 +50,66 @@ import org.objectweb.asm.ClassWriter; */ class Desugar { - /** - * Commandline options for {@link Desugar}. - */ + /** Commandline options for {@link Desugar}. */ public static class Options extends OptionsBase { - @Option(name = "input", - defaultValue = "null", - category = "input", - converter = ExistingPathConverter.class, - abbrev = 'i', - help = "Input Jar with classes to desugar.") + @Option( + name = "input", + defaultValue = "null", + category = "input", + converter = ExistingPathConverter.class, + abbrev = 'i', + help = "Input Jar with classes to desugar." + ) public Path inputJar; - @Option(name = "classpath_entry", - allowMultiple = true, - defaultValue = "", - category = "input", - converter = ExistingPathConverter.class, - help = "Ordered classpath to resolve symbols in the --input Jar, like javac's -cp flag.") + @Option( + name = "classpath_entry", + allowMultiple = true, + defaultValue = "", + category = "input", + converter = ExistingPathConverter.class, + help = "Ordered classpath to resolve symbols in the --input Jar, like javac's -cp flag." + ) public List classpath; - @Option(name = "bootclasspath_entry", - allowMultiple = true, - defaultValue = "", - category = "input", - converter = ExistingPathConverter.class, - help = "Bootclasspath that was used to compile the --input Jar with, like javac's " - + "-bootclasspath flag. If no bootclasspath is explicitly given then the tool's own " - + "bootclasspath is used.") + @Option( + name = "bootclasspath_entry", + allowMultiple = true, + defaultValue = "", + category = "input", + converter = ExistingPathConverter.class, + help = + "Bootclasspath that was used to compile the --input Jar with, like javac's " + + "-bootclasspath flag. If no bootclasspath is explicitly given then the tool's own " + + "bootclasspath is used." + ) public List bootclasspath; - @Option(name = "allow_empty_bootclasspath", - defaultValue = "false", - category = "misc", - help = "Don't use the tool's bootclasspath if no bootclasspath is given.") + @Option( + name = "allow_empty_bootclasspath", + defaultValue = "false", + category = "misc", + help = "Don't use the tool's bootclasspath if no bootclasspath is given." + ) public boolean allowEmptyBootclasspath; - @Option(name = "output", - defaultValue = "null", - category = "output", - converter = PathConverter.class, - abbrev = 'o', - help = "Output Jar to write desugared classes into.") + @Option( + name = "output", + defaultValue = "null", + category = "output", + converter = PathConverter.class, + abbrev = 'o', + help = "Output Jar to write desugared classes into." + ) public Path outputJar; - @Option(name = "verbose", - defaultValue = "false", - category = "misc", - abbrev = 'v', - help = "Enables verbose debugging output.") + @Option( + name = "verbose", + defaultValue = "false", + category = "misc", + abbrev = 'v', + help = "Enables verbose debugging output." + ) public boolean verbose; @Option( @@ -109,10 +120,12 @@ class Desugar { ) public int minSdkVersion; - @Option(name = "core_library", - defaultValue = "false", - category = "undocumented", - help = "Enables rewriting to desugar java.* classes.") + @Option( + name = "core_library", + defaultValue = "false", + category = "undocumented", + help = "Enables rewriting to desugar java.* classes." + ) public boolean coreLibrary; } @@ -133,8 +146,7 @@ class Desugar { args = Files.readAllLines(Paths.get(args[0].substring(1)), ISO_8859_1).toArray(new String[0]); } - OptionsParser optionsParser = - OptionsParser.newOptionsParser(Options.class); + OptionsParser optionsParser = OptionsParser.newOptionsParser(Options.class); optionsParser.parseAndExitUponError(args); Options options = optionsParser.getOptions(Options.class); @@ -159,10 +171,11 @@ class Desugar { ClassLoader loader = createClassLoader( rewriter, options.bootclasspath, options.inputJar, options.classpath, parent); - + boolean allowCallsToObjectsNonNull = options.minSdkVersion >= 19; try (ZipFile in = new ZipFile(options.inputJar.toFile()); - ZipOutputStream out = new ZipOutputStream(new BufferedOutputStream( - Files.newOutputStream(options.outputJar)))) { + ZipOutputStream out = + new ZipOutputStream( + new BufferedOutputStream(Files.newOutputStream(options.outputJar)))) { LambdaClassMaker lambdas = new LambdaClassMaker(dumpDirectory); ClassReaderFactory readerFactory = new ClassReaderFactory(in, rewriter); @@ -188,6 +201,9 @@ class Desugar { new LambdaDesugaring( visitor, loader, lambdas, interfaceLambdaMethodCollector, allowDefaultMethods); + if (!allowCallsToObjectsNonNull) { + visitor = new ObjectsRequireNonNullMethodInliner(visitor); + } reader.accept(visitor, 0); writeStoredEntry(out, entry.getName(), writer.toByteArray()); @@ -204,7 +220,8 @@ class Desugar { ImmutableSet interfaceLambdaMethods = interfaceLambdaMethodCollector.build(); if (allowDefaultMethods) { - checkState(interfaceLambdaMethods.isEmpty(), + checkState( + interfaceLambdaMethods.isEmpty(), "Desugaring with default methods enabled moved interface lambdas"); } @@ -231,9 +248,13 @@ class Desugar { allowDefaultMethods); // Send lambda classes through desugaring to make sure there's no invokedynamic // instructions in generated lambda classes (checkState below will fail) - reader.accept( - new LambdaDesugaring(visitor, loader, lambdas, null, allowDefaultMethods), - 0); + visitor = new LambdaDesugaring(visitor, loader, lambdas, null, allowDefaultMethods); + if (!allowCallsToObjectsNonNull) { + // Not sure whether there will be implicit null check emitted by javac, so we rerun the + // inliner again + visitor = new ObjectsRequireNonNullMethodInliner(visitor); + } + reader.accept(visitor, 0); String filename = rewriter.unprefix(lambdaClass.getValue().desiredInternalName()) + ".class"; writeStoredEntry(out, filename, writer.toByteArray()); @@ -264,9 +285,13 @@ class Desugar { out.closeEntry(); } - private static ClassLoader createClassLoader(CoreLibraryRewriter rewriter, - List bootclasspath, Path inputJar, List classpath, - ClassLoader parent) throws IOException { + private static ClassLoader createClassLoader( + CoreLibraryRewriter rewriter, + List bootclasspath, + Path inputJar, + List classpath, + ClassLoader parent) + throws IOException { // Prepend classpath with input jar itself so LambdaDesugaring can load classes with lambdas. // Note that inputJar and classpath need to be in the same classloader because we typically get // the header Jar for inputJar on the classpath and having the header Jar in a parent loader @@ -282,8 +307,7 @@ class Desugar { private static class ThrowingClassLoader extends ClassLoader { @Override - protected Class loadClass(String name, boolean resolve) - throws ClassNotFoundException { + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { if (name.startsWith("java.")) { // Use system class loader for java. classes, since ClassLoader.defineClass gets // grumpy when those don't come from the standard place. diff --git a/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java b/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java new file mode 100644 index 0000000..e56cfe1 --- /dev/null +++ b/java/com/google/devtools/build/android/desugar/ObjectsRequireNonNullMethodInliner.java @@ -0,0 +1,68 @@ +// Copyright 2016 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 org.objectweb.asm.Opcodes.ASM5; +import static org.objectweb.asm.Opcodes.DUP; +import static org.objectweb.asm.Opcodes.INVOKESTATIC; +import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL; +import static org.objectweb.asm.Opcodes.POP; + +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.MethodVisitor; + +/** + * This class desugars any call to Objects.requireNonNull(Object o), Objects.requireNonNull(Object + * o, String msg), and Objects.requireNonNull(Object o, Supplier msg), by replacing the call with + * o.getClass(). Note that currently we discard the message, as inlining the message involves + * changes to the control flow graph, which further requires re-computation of the stack map frames. + */ +public class ObjectsRequireNonNullMethodInliner extends ClassVisitor { + + public ObjectsRequireNonNullMethodInliner(ClassVisitor cv) { + super(ASM5, cv); + } + + @Override + public MethodVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + MethodVisitor visitor = super.cv.visitMethod(access, name, desc, signature, exceptions); + return visitor == null ? visitor : new ObjectsMethodInlinerMethodVisitor(visitor); + } + + private static class ObjectsMethodInlinerMethodVisitor extends MethodVisitor { + + public ObjectsMethodInlinerMethodVisitor(MethodVisitor mv) { + super(ASM5, mv); + } + + @Override + public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) { + if (opcode != INVOKESTATIC + || !owner.equals("java/util/Objects") + || !name.equals("requireNonNull") + || !desc.equals("(Ljava/lang/Object;)Ljava/lang/Object;")) { + super.visitMethodInsn(opcode, owner, name, desc, itf); + return; + } + + // a call to Objects.requireNonNull(Object o) + // duplicate the first argument 'o', as this method returns 'o'. + super.visitInsn(DUP); + super.visitMethodInsn( + INVOKEVIRTUAL, "java/lang/Object", "getClass", "()Ljava/lang/Class;", false); + super.visitInsn(POP); + } + } +} -- cgit v1.2.3