Create a specific diagnostic for checkdiscard failure
Bug: 166258805
Change-Id: I997c9b1fc8e3a068c1ea53407206b6f65143aa61
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 9bed6c3..ab78b10 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.dex.ApplicationWriter;
import com.android.tools.r8.dex.Marker;
import com.android.tools.r8.dex.Marker.Tool;
+import com.android.tools.r8.errors.CheckDiscardDiagnostic;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.experimental.graphinfo.GraphConsumer;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
@@ -1082,16 +1083,10 @@
timing);
}
}
- for (DexDefinition definition : failed) {
- ByteArrayOutputStream baos = new ByteArrayOutputStream();
- whyAreYouKeepingConsumer.printWhyAreYouKeeping(
- enqueuer.getGraphReporter().getGraphNode(definition.toReference()),
- new PrintStream(baos));
- options.reporter.info(
- new StringDiagnostic(
- "Item " + definition.toSourceString() + " was not discarded.\n" + baos.toString()));
- }
- options.reporter.error(new StringDiagnostic("Discard checks failed."));
+ options.reporter.error(
+ new CheckDiscardDiagnostic.Builder()
+ .addFailedItems(failed, enqueuer.getGraphReporter(), whyAreYouKeepingConsumer)
+ .build());
options.reporter.failIfPendingErrors();
}
diff --git a/src/main/java/com/android/tools/r8/errors/CheckDiscardDiagnostic.java b/src/main/java/com/android/tools/r8/errors/CheckDiscardDiagnostic.java
new file mode 100644
index 0000000..8073696
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/errors/CheckDiscardDiagnostic.java
@@ -0,0 +1,75 @@
+// Copyright (c) 2020, 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.errors;
+
+import com.android.tools.r8.Diagnostic;
+import com.android.tools.r8.Keep;
+import com.android.tools.r8.graph.DexDefinition;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+import com.android.tools.r8.shaking.GraphReporter;
+import com.android.tools.r8.shaking.WhyAreYouKeepingConsumer;
+import com.google.common.collect.ImmutableList;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.util.List;
+
+@Keep
+public class CheckDiscardDiagnostic implements Diagnostic {
+
+ private final List<String> messages;
+
+ public static class Builder {
+ private ImmutableList.Builder<String> messagesBuilder = ImmutableList.builder();
+
+ public Builder addFailedItems(
+ List<DexDefinition> failed,
+ GraphReporter graphReporter,
+ WhyAreYouKeepingConsumer whyAreYouKeepingConsumer) {
+ for (DexDefinition definition : failed) {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ whyAreYouKeepingConsumer.printWhyAreYouKeeping(
+ graphReporter.getGraphNode(definition.toReference()), new PrintStream(baos));
+ messagesBuilder.add(
+ "Item " + definition.toSourceString() + " was not discarded.\n" + baos.toString());
+ }
+ return this;
+ }
+
+ public CheckDiscardDiagnostic build() {
+ return new CheckDiscardDiagnostic(messagesBuilder.build());
+ }
+ }
+
+ private CheckDiscardDiagnostic(List<String> messages) {
+ this.messages = messages;
+ }
+
+ /** The origin of a -checkdiscarded failure is not unique. (The whole app is to blame.) */
+ @Override
+ public Origin getOrigin() {
+ return Origin.unknown();
+ }
+
+ /** The position of a -checkdiscarded failure is always unknown. */
+ @Override
+ public Position getPosition() {
+ return Position.UNKNOWN;
+ }
+
+ @Override
+ public String getDiagnosticMessage() {
+ StringBuilder builder = new StringBuilder("Discard checks failed.");
+ if (messages.size() > 0) {
+ builder.append(System.lineSeparator());
+ builder.append("The following items where not discarded");
+ messages.forEach(
+ message -> {
+ builder.append(System.lineSeparator());
+ builder.append(message);
+ });
+ }
+ return builder.toString();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/checkdiscarded/B162969014.java b/src/test/java/com/android/tools/r8/checkdiscarded/B162969014.java
index 6eb081f..f19d1c1 100644
--- a/src/test/java/com/android/tools/r8/checkdiscarded/B162969014.java
+++ b/src/test/java/com/android/tools/r8/checkdiscarded/B162969014.java
@@ -4,10 +4,11 @@
package com.android.tools.r8.checkdiscarded;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
+import static org.hamcrest.core.StringContains.containsString;
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.AssumeNoSideEffects;
@@ -54,10 +55,17 @@
.enableAssumeNoSideEffectsAnnotations()
.enableInliningAnnotations()
.setMinApi(parameters.getApiLevel())
- .compile();
+ .compileWithExpectedDiagnostics(
+ diagnostics -> {
+ if (checkLogIsDiscarded) {
+ diagnostics.assertErrorsMatch(
+ diagnosticMessage(containsString("Discard checks failed.")));
+ } else {
+ diagnostics.assertNoErrors();
+ }
+ });
} catch (CompilationFailedException e) {
assertTrue(checkLogIsDiscarded);
- assertEquals(e.getCause().getMessage(), "Discard checks failed.");
return;
}
diff --git a/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardModifyDiagnosticsLevelTest.java b/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardModifyDiagnosticsLevelTest.java
index dd7757c..bac9468 100644
--- a/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardModifyDiagnosticsLevelTest.java
+++ b/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardModifyDiagnosticsLevelTest.java
@@ -18,10 +18,9 @@
import com.android.tools.r8.checkdiscarded.testclasses.Main;
import com.android.tools.r8.checkdiscarded.testclasses.UnusedClass;
import com.android.tools.r8.checkdiscarded.testclasses.UsedClass;
+import com.android.tools.r8.errors.CheckDiscardDiagnostic;
import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.StringDiagnostic;
import com.google.common.collect.ImmutableList;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import org.hamcrest.Matcher;
@@ -51,7 +50,11 @@
}
private Matcher<Diagnostic> discardCheckFailedMatcher() {
- return diagnosticMessage(startsWith("Discard checks failed"));
+ return diagnosticMessage(
+ allOf(
+ startsWith("Discard checks failed"),
+ containsString("UsedClass was not discarded"),
+ containsString("is instantiated in")));
}
private Collection<Matcher<Diagnostic>> errorMatchers() {
@@ -67,15 +70,9 @@
}
private Collection<Matcher<Diagnostic>> infoMatchers() {
- List<Matcher<Diagnostic>> matchers = new ArrayList();
- matchers.add(
- allOf(
- diagnosticMessage(containsString("UsedClass was not discarded")),
- diagnosticMessage(containsString("is instantiated in"))));
- if (mappedLevel == DiagnosticsLevel.INFO) {
- matchers.add(discardCheckFailedMatcher());
- }
- return matchers;
+ return mappedLevel == DiagnosticsLevel.INFO
+ ? ImmutableList.of(discardCheckFailedMatcher())
+ : ImmutableList.of();
}
@Test
@@ -88,8 +85,7 @@
.addOptionsModification(this::noInlining)
.setDiagnosticsLevelModifier(
(level, diagnostic) -> {
- if (diagnostic instanceof StringDiagnostic
- && diagnostic.getDiagnosticMessage().equals("Discard checks failed.")) {
+ if (diagnostic instanceof CheckDiscardDiagnostic) {
return mappedLevel;
} else {
return level;
diff --git a/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java b/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java
index df38cb2..61f8917 100644
--- a/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java
+++ b/src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java
@@ -23,7 +23,6 @@
import com.android.tools.r8.checkdiscarded.testclasses.WillStay;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.InternalOptions;
-import com.google.common.collect.ImmutableList;
import java.util.List;
import java.util.function.Consumer;
import org.junit.Test;
@@ -90,16 +89,16 @@
Consumer<TestDiagnosticMessages> check =
diagnostics ->
diagnostics
+ .assertNoInfos()
.assertNoWarnings()
- .assertErrorsMatch(diagnosticMessage(containsString("Discard checks failed")))
- .assertInfosMatch(
- ImmutableList.of(
+ .assertErrorsMatch(
+ diagnosticMessage(
allOf(
- diagnosticMessage(containsString("UsedClass was not discarded")),
- diagnosticMessage(containsString("is instantiated in"))),
- allOf(
- diagnosticMessage(containsString("Main was not discarded")),
- diagnosticMessage(containsString("is referenced in keep rule")))));
+ containsString("Discard checks failed"),
+ containsString("UsedClass was not discarded"),
+ containsString("is instantiated in"),
+ containsString("Main was not discarded"),
+ containsString("is referenced in keep rule"))));
compile(WillStay.class, false, check);
}
@@ -113,12 +112,14 @@
Consumer<TestDiagnosticMessages> check =
diagnostics ->
diagnostics
+ .assertNoInfos()
.assertNoWarnings()
- .assertErrorsMatch(diagnosticMessage(containsString("Discard checks failed")))
- .assertInfosMatch(
- allOf(
- diagnosticMessage(containsString("was not discarded")),
- diagnosticMessage(containsString("is invoked from"))));
+ .assertErrorsMatch(
+ diagnosticMessage(
+ allOf(
+ containsString("Discard checks failed"),
+ containsString("was not discarded"),
+ containsString("is invoked from"))));
compile(WillStay.class, true, check);
}
