Version 1.4.24 Cherry pick: Avoid running visibility bridge remover if nothing publicized. https://r8-review.googlesource.com/33005 Cherry pick: Do not add identity mappings to nested lense. https://r8-review.googlesource.com/33020 Cherry pick: Avoid staticizer lense creation if no method/field mappings found. https://r8-review.googlesource.com/32900 Bug: 122846041 Change-Id: Ib3f036275d532dffada5f36370c806c0fee56ef2
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java index ee45cf4..8ef9af6 100644 --- a/src/main/java/com/android/tools/r8/R8.java +++ b/src/main/java/com/android/tools/r8/R8.java
@@ -363,12 +363,15 @@ } if (options.getProguardConfiguration().isAccessModificationAllowed()) { - appView.setGraphLense( - ClassAndMemberPublicizer.run(executorService, timing, application, appView, rootSet)); - // We can now remove visibility bridges. Note that we do not need to update the - // invoke-targets here, as the existing invokes will simply dispatch to the now - // visible super-method. MemberRebinding, if run, will then dispatch it correctly. - application = new VisibilityBridgeRemover(appView.appInfo(), application).run(); + GraphLense publicizedLense = + ClassAndMemberPublicizer.run(executorService, timing, application, appView, rootSet); + if (publicizedLense != appView.graphLense()) { + appView.setGraphLense(publicizedLense); + // We can now remove visibility bridges. Note that we do not need to update the + // invoke-targets here, as the existing invokes will simply dispatch to the now + // visible super-method. MemberRebinding, if run, will then dispatch it correctly. + application = new VisibilityBridgeRemover(appView.appInfo(), application).run(); + } } // Build conservative main dex content before first round of tree shaking. This is used
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java index e0b2994..4336ca5 100644 --- a/src/main/java/com/android/tools/r8/Version.java +++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@ // This field is accessed from release scripts using simple pattern matching. // Therefore, changing this field could break our release scripts. - public static final String LABEL = "1.4.23"; + public static final String LABEL = "1.4.24"; private Version() { }
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java index 247a5fe..9a373fa 100644 --- a/src/main/java/com/android/tools/r8/graph/GraphLense.java +++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -297,23 +297,38 @@ private final BiMap<DexMethod, DexMethod> originalMethodSignatures = HashBiMap.create(); public void map(DexType from, DexType to) { + if (from == to) { + return; + } typeMap.put(from, to); } public void map(DexMethod from, DexMethod to) { + if (from == to) { + return; + } methodMap.put(from, to); } public void map(DexField from, DexField to) { + if (from == to) { + return; + } fieldMap.put(from, to); } public void move(DexMethod from, DexMethod to) { + if (from == to) { + return; + } map(from, to); originalMethodSignatures.put(to, from); } public void move(DexField from, DexField to) { + if (from == to) { + return; + } fieldMap.put(from, to); originalFieldSignatures.put(to, from); } @@ -761,9 +776,11 @@ @Override public String toString() { StringBuilder builder = new StringBuilder(); - for (Map.Entry<DexType, DexType> entry : typeMap.entrySet()) { - builder.append(entry.getKey().toSourceString()).append(" -> "); - builder.append(entry.getValue().toSourceString()).append(System.lineSeparator()); + if (typeMap != null) { + for (Map.Entry<DexType, DexType> entry : typeMap.entrySet()) { + builder.append(entry.getKey().toSourceString()).append(" -> "); + builder.append(entry.getValue().toSourceString()).append(System.lineSeparator()); + } } for (Map.Entry<DexMethod, DexMethod> entry : methodMap.entrySet()) { builder.append(entry.getKey().toSourceString()).append(" -> ");
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java index fa0b28a..351ec8c 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
@@ -289,7 +289,7 @@ // invoke-virtual { s1, ... } mtd1 // goto Exit // b2: - // s2 <- static-get singleoton + // s2 <- static-get singleton // ... // invoke-virtual { s2, ... } mtd1 // goto Exit @@ -500,7 +500,7 @@ } } - if (!methodMapping.isEmpty() || fieldMapping.isEmpty()) { + if (!methodMapping.isEmpty() || !fieldMapping.isEmpty()) { classStaticizer.converter.appView.setGraphLense( new ClassStaticizerGraphLense( classStaticizer.converter.graphLense(),
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java index df797ad..7b3ed4a 100644 --- a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java +++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java
@@ -29,11 +29,14 @@ import com.android.tools.r8.ir.optimize.staticizer.movetohost.CandidateConflictField; import com.android.tools.r8.ir.optimize.staticizer.movetohost.CandidateConflictMethod; import com.android.tools.r8.ir.optimize.staticizer.movetohost.CandidateOk; +import com.android.tools.r8.ir.optimize.staticizer.movetohost.CandidateOkFieldOnly; import com.android.tools.r8.ir.optimize.staticizer.movetohost.CandidateOkSideEffects; import com.android.tools.r8.ir.optimize.staticizer.movetohost.HostConflictField; import com.android.tools.r8.ir.optimize.staticizer.movetohost.HostConflictMethod; import com.android.tools.r8.ir.optimize.staticizer.movetohost.HostOk; +import com.android.tools.r8.ir.optimize.staticizer.movetohost.HostOkFieldOnly; import com.android.tools.r8.ir.optimize.staticizer.movetohost.HostOkSideEffects; +import com.android.tools.r8.ir.optimize.staticizer.movetohost.MoveToHostFieldOnlyTestClass; import com.android.tools.r8.ir.optimize.staticizer.movetohost.MoveToHostTestClass; import com.android.tools.r8.ir.optimize.staticizer.trivial.Simple; import com.android.tools.r8.ir.optimize.staticizer.trivial.SimpleWithGetter; @@ -150,6 +153,36 @@ } @Test + public void testMoveToHost_fieldOnly() throws Exception { + assumeTrue("b/112831361", backend == Backend.DEX); + Class<?> main = MoveToHostFieldOnlyTestClass.class; + Class<?>[] classes = { + NeverInline.class, + MoveToHostFieldOnlyTestClass.class, + HostOkFieldOnly.class, + CandidateOkFieldOnly.class + }; + TestRunResult result = testForR8(backend) + .addProgramClasses(classes) + .enableProguardTestOptions() + .enableInliningAnnotations() + .addKeepMainRule(main) + .noMinification() + .addKeepRules("-allowaccessmodification") + .addOptionsModification(this::configure) + .run(main); + + CodeInspector inspector = result.inspector(); + ClassSubject clazz = inspector.clazz(main); + + assertEquals( + Lists.newArrayList(), + references(clazz, "testOk_fieldOnly", "void")); + + assertFalse(inspector.clazz(CandidateOkFieldOnly.class).isPresent()); + } + + @Test public void testMoveToHost() throws Exception { assumeTrue("b/112831361", backend == Backend.DEX); Class<?> main = MoveToHostTestClass.class;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/CandidateOkFieldOnly.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/CandidateOkFieldOnly.java new file mode 100644 index 0000000..48963f4 --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/CandidateOkFieldOnly.java
@@ -0,0 +1,8 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.optimize.staticizer.movetohost; + +public class CandidateOkFieldOnly { + // No instance methods. +}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/HostOkFieldOnly.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/HostOkFieldOnly.java new file mode 100644 index 0000000..e132627 --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/HostOkFieldOnly.java
@@ -0,0 +1,8 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.optimize.staticizer.movetohost; + +public class HostOkFieldOnly { + static CandidateOkFieldOnly INSTANCE = new CandidateOkFieldOnly(); +}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/MoveToHostFieldOnlyTestClass.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/MoveToHostFieldOnlyTestClass.java new file mode 100644 index 0000000..f725d21 --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/movetohost/MoveToHostFieldOnlyTestClass.java
@@ -0,0 +1,29 @@ +// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.ir.optimize.staticizer.movetohost; + +import com.android.tools.r8.NeverInline; + +public class MoveToHostFieldOnlyTestClass { + + public static void main(String[] args) { + MoveToHostFieldOnlyTestClass test = new MoveToHostFieldOnlyTestClass(); + test.testOk_fieldOnly(); + } + + @NeverInline + private void testOk_fieldOnly() { + // Any instance method call whose target holder is not the candidate will invalidate candidacy, + // for example, toString() without overriding, getClass(), etc. + // Note that having instance methods in the candidate class guarantees that method mappings will + // exist when field mappings do so. + // Any other uses other than invoke-virtual or invoke-direct (to either <init> or private) are + // not allowed, e.g., System.out.println(INSTANCE), null check, or static-put to somewhere else. + // Therefore, it's merely dead code, and thus it has not been harmful to forget to create a + // staticizer lense when there is no method mapping (for instance methods to staticized ones) + // while there are field mappings as shown in this example. + Object x = HostOkFieldOnly.INSTANCE; + } +} +