Rewrite parser to remember last added entry
This will ensure not to report additional entries for the same
signature for same renamings but different obfuscation ranges, per the
following example:
int foo(Z) -> a
1:1:int qux(A):3:3 -> a
1:1:int bar(A):3 -> a
2:2:int qux(A):3:3 -> a
2:2:int bar(A):4 -> a
3:3:int bar(A):5:5 -> a
Here, bar(A) should only be added once.
Bug: 131349062
Change-Id: I7b683a92868b699a79c782fd9a54c8ea0e6cb7c5
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java b/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
index 81b745c..ef9aedd 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapReader.java
@@ -196,114 +196,76 @@
}
private void parseMemberMappings(ClassNaming.Builder classNamingBuilder) throws IOException {
+ MemberNaming lastAddedNaming = null;
MemberNaming activeMemberNaming = null;
- Range previousObfuscatedRange = null;
- boolean previousWasPotentiallySynthesized = false;
- Signature previousSignature = null;
- String previousRenamedName = null;
- boolean lastRound = false;
- for (; ; ) {
- Signature signature = null;
+ Range previousMappedRange = null;
+ do {
Object originalRange = null;
- String renamedName = null;
- Range obfuscatedRange = null;
+ Range mappedRange = null;
- // In the last round we're only here to flush the last line read (which may trigger adding a
- // new MemberNaming) and flush activeMemberNaming, so skip parsing.
- if (!lastRound) {
- if (!Character.isWhitespace(peekCodePoint())) {
- lastRound = true;
- continue;
- }
- skipWhitespace();
- Object maybeRangeOrInt = maybeParseRangeOrInt();
- if (maybeRangeOrInt != null) {
- if (!(maybeRangeOrInt instanceof Range)) {
- throw new ParseException(
- String.format("Invalid obfuscated line number range (%s).", maybeRangeOrInt));
- }
- obfuscatedRange = (Range) maybeRangeOrInt;
- expect(':');
- }
- signature = parseSignature();
- if (peekChar(0) == ':') {
- // This is a mapping or inlining definition
- nextChar();
- originalRange = maybeParseRangeOrInt();
- if (originalRange == null) {
- throw new ParseException("No number follows the colon after the method signature.");
- }
- }
- skipWhitespace();
- skipArrow();
- skipWhitespace();
- renamedName = parseMethodName();
- }
-
- // If this line refers to a member that should be added to classNamingBuilder (as opposed to
- // an inner inlined callee) and it's different from the activeMemberNaming, then flush (add)
- // the current activeMemberNaming and create a new one.
- // We're also entering this in the last round when there's no current line.
- if (previousRenamedName != null
- && (!Objects.equals(previousObfuscatedRange, obfuscatedRange)
- || !Objects.equals(previousRenamedName, renamedName)
- || (originalRange != null && originalRange instanceof Range))) {
- // Flush activeMemberNaming if it's for a different member.
- if (activeMemberNaming != null) {
- if (!activeMemberNaming.getOriginalSignature().equals(previousSignature)) {
- classNamingBuilder.addMemberEntry(activeMemberNaming);
- activeMemberNaming = null;
- } else {
- if (activeMemberNaming.getRenamedName().equals(previousRenamedName)) {
- // The method was potentially synthesized.
- previousWasPotentiallySynthesized = previousObfuscatedRange == null;
- } else {
- assert previousWasPotentiallySynthesized;
- }
- }
- }
- if (activeMemberNaming == null) {
- activeMemberNaming =
- new MemberNaming(previousSignature, previousRenamedName, getPosition());
- }
- }
-
- if (lastRound) {
- if (activeMemberNaming != null) {
- classNamingBuilder.addMemberEntry(activeMemberNaming);
- }
+ // Parse the member line ' x:y:name:z:q -> renamedName'.
+ if (!Character.isWhitespace(peekCodePoint())) {
break;
}
-
- // Interpret what we've just parsed.
- if (obfuscatedRange == null) {
- if (originalRange != null) {
- throw new ParseException("No mapping for original range " + originalRange + ".");
+ skipWhitespace();
+ Object maybeRangeOrInt = maybeParseRangeOrInt();
+ if (maybeRangeOrInt != null) {
+ if (!(maybeRangeOrInt instanceof Range)) {
+ throw new ParseException(
+ String.format("Invalid obfuscated line number range (%s).", maybeRangeOrInt));
}
- // Here we have a line like 'a() -> b' or a field like 'a -> b'
- if (activeMemberNaming != null) {
- classNamingBuilder.addMemberEntry(activeMemberNaming);
- }
- activeMemberNaming = new MemberNaming(signature, renamedName, getPosition());
- } else {
-
- // Note that at this point originalRange may be null which either means, it's the same as
- // the obfuscatedRange (identity mapping) or that it's unknown (source line number
- // information was not available).
- assert signature instanceof MethodSignature;
+ mappedRange = (Range) maybeRangeOrInt;
+ expect(':');
}
+ Signature signature = parseSignature();
+ if (peekChar(0) == ':') {
+ // This is a mapping or inlining definition
+ nextChar();
+ originalRange = maybeParseRangeOrInt();
+ if (originalRange == null) {
+ throw new ParseException("No number follows the colon after the method signature.");
+ }
+ }
+ if (mappedRange == null && originalRange != null) {
+ throw new ParseException("No mapping for original range " + originalRange + ".");
+ }
+
+ skipWhitespace();
+ skipArrow();
+ skipWhitespace();
+ String renamedName = parseMethodName();
if (signature instanceof MethodSignature) {
classNamingBuilder.addMappedRange(
- obfuscatedRange, (MethodSignature) signature, originalRange, renamedName);
+ mappedRange, (MethodSignature) signature, originalRange, renamedName);
}
- previousRenamedName = renamedName;
- previousObfuscatedRange = obfuscatedRange;
- previousSignature = signature;
+ assert mappedRange == null || signature instanceof MethodSignature;
- if (!nextLine()) {
- lastRound = true;
+ // If this line refers to a member that should be added to classNamingBuilder (as opposed to
+ // an inner inlined callee) and it's different from the the previous activeMemberNaming, then
+ // flush (add) the current activeMemberNaming.
+ if (activeMemberNaming != null) {
+ boolean changedName = !activeMemberNaming.getRenamedName().equals(renamedName);
+ boolean changedMappedRange = !Objects.equals(previousMappedRange, mappedRange);
+ boolean notAdded =
+ lastAddedNaming == null
+ || !lastAddedNaming.getOriginalSignature().equals(activeMemberNaming.signature);
+ if (changedName || previousMappedRange == null || (changedMappedRange && notAdded)) {
+ classNamingBuilder.addMemberEntry(activeMemberNaming);
+ lastAddedNaming = activeMemberNaming;
+ }
+ }
+ activeMemberNaming = new MemberNaming(signature, renamedName, getPosition());
+ previousMappedRange = mappedRange;
+ } while (nextLine());
+
+ if (activeMemberNaming != null) {
+ boolean notAdded =
+ lastAddedNaming == null
+ || !lastAddedNaming.getOriginalSignature().equals(activeMemberNaming.signature);
+ if (previousMappedRange == null || notAdded) {
+ classNamingBuilder.addMemberEntry(activeMemberNaming);
}
}
}
diff --git a/src/test/java/com/android/tools/r8/naming/SeedMapperTests.java b/src/test/java/com/android/tools/r8/naming/SeedMapperTests.java
index 6205bd7..4c265ef 100644
--- a/src/test/java/com/android/tools/r8/naming/SeedMapperTests.java
+++ b/src/test/java/com/android/tools/r8/naming/SeedMapperTests.java
@@ -16,7 +16,6 @@
import com.android.tools.r8.utils.Reporter;
import java.io.IOException;
import java.nio.file.Path;
-import org.junit.Ignore;
import org.junit.Test;
public class SeedMapperTests extends TestBase {
@@ -164,29 +163,31 @@
}
}
- @Ignore("b/131349062")
@Test
public void testInliningFrames() throws IOException {
Path applyMappingFile =
getApplyMappingFile(
"A.B.C -> a:",
" int foo(A) -> a",
- " 1:2:int bar(A):3 -> a");
+ " 1:2:int bar(A):3:4 -> a",
+ " 1:2:int baz(B):3 -> a");
TestDiagnosticMessagesImpl testDiagnosticMessages = new TestDiagnosticMessagesImpl();
Reporter reporter = new Reporter(testDiagnosticMessages);
SeedMapper.seedMapperFromFile(reporter, applyMappingFile);
}
- @Ignore("b/131349062")
@Test
public void testDuplicateInliningFrames() throws IOException {
Path applyMappingFile =
getApplyMappingFile(
"A.B.C -> a:",
" int foo(Z) -> a",
+ " 1:1:int qux(A):3:3 -> a",
" 1:1:int bar(A):3 -> a",
- " 2:2:int baz(B):4 -> a",
- " 3:3:int bar(A):5 -> a");
+ " 2:2:int qux(A):3:3 -> a",
+ " 2:2:int bar(A):4 -> a",
+ " 3:3:int bar(A):5:5 -> a",
+ " int qux(C) -> a");
TestDiagnosticMessagesImpl testDiagnosticMessages = new TestDiagnosticMessagesImpl();
Reporter reporter = new Reporter(testDiagnosticMessages);
SeedMapper.seedMapperFromFile(reporter, applyMappingFile);
