From a672cfbe2bcd1d768a6123a02f65f57ca6ec42f5 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 27 Apr 2017 19:40:10 +0200 Subject: Use system property for lambda dir if set If system property that triggers writing lambda classes is already set, use that value for the lambda dump directory path. Otherwise, generate a temporary dir path, and set it. RELNOTES: n/a PiperOrigin-RevId: 154440327 GitOrigin-RevId: a50a56cf9cd6c0f7c459b265213669b3a2f7ee5e Change-Id: Ia273792f2f7e89758d962a3dfe051074691c0e59 --- .../google/devtools/build/android/desugar/Desugar.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java index c4528ae..cd6b6ab 100644 --- a/java/com/google/devtools/build/android/desugar/Desugar.java +++ b/java/com/google/devtools/build/android/desugar/Desugar.java @@ -522,14 +522,20 @@ class Desugar { * LambdaClassMaker generates lambda classes for us, but it does so by essentially simulating the * call to LambdaMetafactory that the JVM would make when encountering an invokedynamic. * LambdaMetafactory is in the JDK and its implementation has a property to write out ("dump") - * generated classes, which we take advantage of here. Set property before doing anything else - * since the property is read in the static initializer; if this breaks we can investigate setting - * the property when calling the tool. + * generated classes, which we take advantage of here. This property can be set externally, and in + * that case the specified directory is used as a temporary dir. Otherwise, it will be set here, + * before doing anything else since the property is read in the static initializer. */ private static Path createAndRegisterLambdaDumpDirectory() throws IOException { - Path dumpDirectory = Files.createTempDirectory("lambdas"); - System.setProperty( - LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY, dumpDirectory.toString()); + String propertyValue = System.getProperty(LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY); + Path dumpDirectory; + if (propertyValue != null) { + dumpDirectory = Paths.get(propertyValue); + } else { + dumpDirectory = Files.createTempDirectory("lambdas"); + System.setProperty( + LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY, dumpDirectory.toString()); + } deleteTreeOnExit(dumpDirectory); return dumpDirectory; -- cgit v1.2.3 From 2968287a8d2acadee6d7161a76bdafd361c64537 Mon Sep 17 00:00:00 2001 From: kmb Date: Thu, 27 Apr 2017 21:02:32 +0200 Subject: Don't copy lambda bodies from interfaces into implementing classes. RELNOTES: none PiperOrigin-RevId: 154452158 GitOrigin-RevId: 11e976972c4f35829e630cf69c7563ca40b7b9bf Change-Id: I4de975f90ece754d6a6f4adea7a421a5db3990eb --- .../android/desugar/DefaultMethodClassFixer.java | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java b/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java index 8ad5dc2..732f064 100644 --- a/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java +++ b/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java @@ -132,6 +132,19 @@ public class DefaultMethodClassFixer extends ClassVisitor { return false; } + /** + * Returns {@code true} for non-bridge default methods not in {@link #instanceMethods}. + */ + private boolean shouldStub(int access, String name, String desc) { + // Ignore private methods, which technically aren't default methods and can only be called from + // other methods defined in the interface. This also ignores lambda body methods, which is fine + // as we don't want or need to stub those. Also ignore bridge methods as javac adds them to + // concrete classes as needed anyway and we handle them separately for generated lambda classes. + return BitFlags.noneSet(access, + Opcodes.ACC_ABSTRACT | Opcodes.ACC_STATIC | Opcodes.ACC_BRIDGE | Opcodes.ACC_PRIVATE) + && !instanceMethods.contains(name + ":" + desc); + } + private void stubMissingDefaultMethods(ImmutableList interfaces) { for (String implemented : interfaces) { if (!seenInterfaces.add(implemented)) { @@ -183,12 +196,10 @@ public class DefaultMethodClassFixer extends ClassVisitor { @Override public MethodVisitor visitMethod( int access, String name, String desc, String signature, String[] exceptions) { - if (BitFlags.noneSet(access, Opcodes.ACC_ABSTRACT | Opcodes.ACC_STATIC | Opcodes.ACC_BRIDGE) - && !instanceMethods.contains(name + ":" + desc)) { + if (shouldStub(access, name, desc)) { // Add this method to the class we're desugaring and stub in a body to call the default // implementation in the interface's companion class. ijar omits these methods when setting - // ACC_SYNTHETIC modifier, so don't. Don't do this for bridge methods, which we handle - // separately. + // ACC_SYNTHETIC modifier, so don't. // Signatures can be wrong, e.g., when type variables are introduced, instantiated, or // refined in the class we're processing, so drop them. MethodVisitor stubMethod = @@ -255,8 +266,7 @@ public class DefaultMethodClassFixer extends ClassVisitor { @Override public MethodVisitor visitMethod( int access, String name, String desc, String signature, String[] exceptions) { - if (BitFlags.noneSet(access, Opcodes.ACC_ABSTRACT | Opcodes.ACC_STATIC | Opcodes.ACC_BRIDGE) - && !instanceMethods.contains(name + ":" + desc)) { + if (!found && shouldStub(access, name, desc)) { // Found a default method we're not ignoring (instanceMethods at this point contains methods // the top-level visited class implements itself). found = true; -- cgit v1.2.3 From acd4846b9693b50d0c42c22b4b16196ddf8f73b1 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 28 Apr 2017 19:30:36 +0200 Subject: Don't clean-up externally set dump dir If the lambda dump dir has been set externally, its content will not be removed after Desugar ends. RELNOTES: n/a PiperOrigin-RevId: 154554712 GitOrigin-RevId: 8488c7f7545a054ef78daa9ca664b6580adda729 Change-Id: I6b2d4a40b8f484505965167eb13633bf04236a09 --- java/com/google/devtools/build/android/desugar/Desugar.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java index cd6b6ab..ca7032e 100644 --- a/java/com/google/devtools/build/android/desugar/Desugar.java +++ b/java/com/google/devtools/build/android/desugar/Desugar.java @@ -528,15 +528,13 @@ class Desugar { */ private static Path createAndRegisterLambdaDumpDirectory() throws IOException { String propertyValue = System.getProperty(LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY); - Path dumpDirectory; if (propertyValue != null) { - dumpDirectory = Paths.get(propertyValue); - } else { - dumpDirectory = Files.createTempDirectory("lambdas"); - System.setProperty( - LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY, dumpDirectory.toString()); + return Paths.get(propertyValue); } + Path dumpDirectory = Files.createTempDirectory("lambdas"); + System.setProperty( + LambdaClassMaker.LAMBDA_METAFACTORY_DUMPER_PROPERTY, dumpDirectory.toString()); deleteTreeOnExit(dumpDirectory); return dumpDirectory; } -- cgit v1.2.3 From 885bce7f8d296b091d15c0007a7716f01c3fee33 Mon Sep 17 00:00:00 2001 From: cnsun Date: Sat, 29 Apr 2017 01:04:00 +0200 Subject: Bug fix. Enable Desugar to desugar try-with-resources multiple times. RELNOTES: n/a PiperOrigin-RevId: 154594200 GitOrigin-RevId: 198f00a930e9d5f52f7ee6be52bdaaf5b7999ee2 Change-Id: I3e1b0a53ae79c292ae3a7296b8b57538bc4e2d47 --- .../android/desugar/TryWithResourcesRewriter.java | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java b/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java index 2429d2f..cde223e 100644 --- a/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java +++ b/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java @@ -85,6 +85,11 @@ public class TryWithResourcesRewriter extends ClassVisitor { private final ClassLoader classLoader; private final AtomicInteger numOfTryWithResourcesInvoked; + /** + * 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; public TryWithResourcesRewriter( ClassVisitor classVisitor, @@ -95,11 +100,24 @@ public class TryWithResourcesRewriter extends ClassVisitor { this.numOfTryWithResourcesInvoked = numOfTryWithResourcesInvoked; } + @Override + public void visit( + int version, + int access, + String name, + String signature, + String superName, + String[] interfaces) { + super.visit(version, access, name, signature, superName, interfaces); + shouldCurrentClassBeIgnored = THROWABLE_EXT_CLASS_INTERNAL_NAMES.contains(name); + } + + @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 || THROWABLE_EXT_CLASS_INTERNAL_NAMES.contains(name) + return visitor == null || shouldCurrentClassBeIgnored ? visitor : new TryWithResourceVisitor(visitor, classLoader); } -- cgit v1.2.3