Merge "Emit warning for duplicate fields and methods"
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index 64ba508..e998206 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -27,15 +27,21 @@
import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.graph.DexValue.DexValueType;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.FieldSignatureEquivalence;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.MethodSignatureEquivalence;
+import com.android.tools.r8.utils.StringDiagnostic;
+import com.google.common.base.Equivalence.Wrapper;
import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
import java.util.Objects;
+import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import org.objectweb.asm.AnnotationVisitor;
@@ -148,8 +154,10 @@
private List<DexAnnotationElement> defaultAnnotations = null;
private final List<DexEncodedField> staticFields = new ArrayList<>();
private final List<DexEncodedField> instanceFields = new ArrayList<>();
+ private final Set<Wrapper<DexField>> fieldSignatures = new HashSet<>();
private final List<DexEncodedMethod> directMethods = new ArrayList<>();
private final List<DexEncodedMethod> virtualMethods = new ArrayList<>();
+ private final Set<Wrapper<DexMethod>> methodSignatures = new HashSet<>();
public CreateDexClassVisitor(
Origin origin,
@@ -380,13 +388,20 @@
public void visitEnd() {
FieldAccessFlags flags = FieldAccessFlags.fromCfAccessFlags(cleanAccessFlags(access));
DexField dexField = parent.application.getField(parent.type, name, desc);
- DexAnnotationSet annotationSet = createAnnotationSet(annotations);
- DexValue staticValue = flags.isStatic() ? getStaticValue(value, dexField.type) : null;
- DexEncodedField field = new DexEncodedField(dexField, flags, annotationSet, staticValue);
- if (flags.isStatic()) {
- parent.staticFields.add(field);
+ Wrapper<DexField> signature = FieldSignatureEquivalence.get().wrap(dexField);
+ if (parent.fieldSignatures.add(signature)) {
+ DexAnnotationSet annotationSet = createAnnotationSet(annotations);
+ DexValue staticValue = flags.isStatic() ? getStaticValue(value, dexField.type) : null;
+ DexEncodedField field = new DexEncodedField(dexField, flags, annotationSet, staticValue);
+ if (flags.isStatic()) {
+ parent.staticFields.add(field);
+ } else {
+ parent.instanceFields.add(field);
+ }
} else {
- parent.instanceFields.add(field);
+ parent.application.options.reporter.warning(
+ new StringDiagnostic(
+ String.format("Field `%s` has multiple definitions", dexField.toSourceString())));
}
}
@@ -603,10 +618,20 @@
annotationsList,
code,
parent.version);
- if (flags.isStatic() || flags.isConstructor() || flags.isPrivate()) {
- parent.directMethods.add(dexMethod);
+ Wrapper<DexMethod> signature = MethodSignatureEquivalence.get().wrap(method);
+ if (parent.methodSignatures.add(signature)) {
+ if (flags.isStatic() || flags.isConstructor() || flags.isPrivate()) {
+ parent.directMethods.add(dexMethod);
+ } else {
+ parent.virtualMethods.add(dexMethod);
+ }
} else {
- parent.virtualMethods.add(dexMethod);
+ internalOptions.reporter.warning(
+ new StringDiagnostic(
+ String.format(
+ "Ignoring an implementation of the method `%s` because it has multiple "
+ + "definitions",
+ method.toSourceString())));
}
if (defaultAnnotation != null) {
parent.addDefaultAnnotation(name, defaultAnnotation);
diff --git a/src/test/java/com/android/tools/r8/invalid/DuplicateDefinitionsTest.java b/src/test/java/com/android/tools/r8/invalid/DuplicateDefinitionsTest.java
new file mode 100644
index 0000000..93f9084
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/invalid/DuplicateDefinitionsTest.java
@@ -0,0 +1,100 @@
+// Copyright (c) 2018, 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.invalid;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
+import com.android.tools.r8.jasmin.JasminTestBase;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.google.common.collect.ImmutableList;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.nio.charset.Charset;
+import org.junit.Test;
+
+public class DuplicateDefinitionsTest extends JasminTestBase {
+
+ @Test
+ public void testDuplicateMethods() throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ ClassBuilder classBuilder = jasminBuilder.addClass("C");
+ classBuilder.addMainMethod(".limit locals 1", ".limit stack 0", "return");
+ classBuilder.addMainMethod(".limit locals 1", ".limit stack 0", "return");
+ classBuilder.addVirtualMethod("method", "V", ".limit locals 1", ".limit stack 0", "return");
+ classBuilder.addVirtualMethod("method", "V", ".limit locals 1", ".limit stack 0", "return");
+
+ // Run D8 and intercept warnings.
+ PrintStream stderr = System.err;
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ System.setErr(new PrintStream(baos));
+
+ AndroidApp app = compileWithD8(jasminBuilder.build());
+
+ String output = new String(baos.toByteArray(), Charset.defaultCharset());
+ System.setOut(stderr);
+
+ // Check that warnings were emitted.
+ assertThat(
+ output,
+ containsString(
+ "Ignoring an implementation of the method `void C.main(java.lang.String[])` because "
+ + "it has multiple definitions"));
+ assertThat(
+ output,
+ containsString(
+ "Ignoring an implementation of the method `void C.method()` because "
+ + "it has multiple definitions"));
+
+ DexInspector inspector = new DexInspector(app);
+ ClassSubject clazz = inspector.clazz("C");
+ assertThat(clazz, isPresent());
+
+ // There are two direct methods, but only because one is <init>.
+ assertEquals(2, clazz.getDexClass().directMethods().length);
+ assertThat(clazz.method("void", "<init>", ImmutableList.of()), isPresent());
+
+ // There is only one virtual method.
+ assertEquals(1, clazz.getDexClass().virtualMethods().length);
+ }
+
+ @Test
+ public void testDuplicateFields() throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ ClassBuilder classBuilder = jasminBuilder.addClass("C");
+ classBuilder.addField("public", "fld", "LC;", null);
+ classBuilder.addField("public", "fld", "LC;", null);
+ classBuilder.addStaticField("staticFld", "LC;", null);
+ classBuilder.addStaticField("staticFld", "LC;", null);
+
+ // Run D8 and intercept warnings.
+ PrintStream stderr = System.err;
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ System.setErr(new PrintStream(baos));
+
+ AndroidApp app = compileWithD8(jasminBuilder.build());
+
+ String output = new String(baos.toByteArray(), Charset.defaultCharset());
+ System.setOut(stderr);
+
+ // Check that warnings were emitted.
+ assertThat(output, containsString("Field `C C.fld` has multiple definitions"));
+ assertThat(output, containsString("Field `C C.staticFld` has multiple definitions"));
+
+ DexInspector inspector = new DexInspector(app);
+ ClassSubject clazz = inspector.clazz("C");
+ assertThat(clazz, isPresent());
+
+ // Redundant fields have been removed.
+ assertEquals(1, clazz.getDexClass().instanceFields().length);
+ assertEquals(1, clazz.getDexClass().staticFields().length);
+ }
+}
