Add non-default implementation of peekNext() / peekPrevious() where applicable
And make IteratorUtils.peekPrevious() return null (instead of throw)
when !iterator.hasPrevious().
These methods sound like they should not have side-effects, but they
can change which value is removed when remove() is called.
E.g.: For A->B->C->D:
it.next() --> current = A
it.next() --> current = B
it.peekNext()
it.next() --> current = C
it.previous() --> current = C
This change tries to guard against remove() being called after peek
methods by making it trigger InvalidStateException() in most cases.
Bug: None
Change-Id: I224e79043f96ea3028b3d95969bdc0f9e1d2f534
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
index 54531af..640d3bd 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionListIterator.java
@@ -84,6 +84,16 @@
}
@Override
+ public Instruction peekNext() {
+ // Reset current since listIterator.remove() changes based on whether next() or previous() was
+ // last called.
+ // E.g.: next() -> current=C
+ // peekNext(): next() -> current=D, previous() -> current=D
+ current = null;
+ return IteratorUtils.peekNext(listIterator);
+ }
+
+ @Override
public boolean hasPrevious() {
return listIterator.hasPrevious();
}
@@ -100,6 +110,16 @@
}
@Override
+ public Instruction peekPrevious() {
+ // Reset current since listIterator.remove() changes based on whether next() or previous() was
+ // last called.
+ // E.g.: previous() -> current=B
+ // peekPrevious(): previous() -> current=A, next() -> current=A
+ current = null;
+ return IteratorUtils.peekPrevious(listIterator);
+ }
+
+ @Override
public boolean hasInsertionPosition() {
return position != null;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
index 4874229..325cea8 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCodeInstructionListIterator.java
@@ -23,7 +23,7 @@
public class IRCodeInstructionListIterator implements InstructionListIterator {
- private final ListIterator<BasicBlock> blockIterator;
+ private final BasicBlockIterator blockIterator;
private InstructionListIterator instructionIterator;
private final IRCode code;
@@ -174,6 +174,16 @@
}
@Override
+ public Instruction peekNext() {
+ // Default impl calls next() / previous(), which affects what remove() does.
+ Instruction next = instructionIterator.peekNext();
+ if (next == null && blockIterator.hasNext()) {
+ next = blockIterator.peekNext().entry();
+ }
+ return next;
+ }
+
+ @Override
public boolean hasPrevious() {
return instructionIterator.hasPrevious() || blockIterator.hasPrevious();
}
@@ -193,6 +203,16 @@
}
@Override
+ public Instruction peekPrevious() {
+ // Default impl calls next() / previous(), which affects what remove() does.
+ Instruction previous = instructionIterator.peekPrevious();
+ if (previous == null && blockIterator.hasPrevious()) {
+ previous = blockIterator.peekPrevious().exit();
+ }
+ return previous;
+ }
+
+ @Override
public int nextIndex() {
throw new UnsupportedOperationException();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
index 24e7793..21ab96c 100644
--- a/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/LinearFlowInstructionListIterator.java
@@ -249,6 +249,27 @@
return currentBlockIterator.next();
}
+ @Override
+ public Instruction peekNext() {
+ // Default impl calls next() / previous(), which will alter this.seen.
+ Instruction current = currentBlockIterator.peekNext();
+ if (!current.isGoto()) {
+ return current;
+ }
+ BasicBlock target = current.asGoto().getTarget();
+ if (!isLinearEdge(currentBlock, target)) {
+ return current;
+ }
+ while (target.isTrivialGoto()) {
+ BasicBlock candidate = target.exit().asGoto().getTarget();
+ if (!isLinearEdge(target, candidate)) {
+ break;
+ }
+ target = candidate;
+ }
+ return target.entry();
+ }
+
private BasicBlock getBeginningOfTrivialLinearGotoChain(BasicBlock block) {
if (block.getPredecessors().size() != 1
|| !isLinearEdge(block.getPredecessors().get(0), block)) {
@@ -290,6 +311,20 @@
}
@Override
+ public Instruction peekPrevious() {
+ // Default impl calls next() / previous(), which will alter this.seen.
+ Instruction ret = currentBlockIterator.peekPrevious();
+ if (ret != null) {
+ return ret;
+ }
+ BasicBlock target = getBeginningOfTrivialLinearGotoChain(currentBlock);
+ if (target == null || target.size() < 2) {
+ return null;
+ }
+ return target.getInstructions().get(target.size() - 2);
+ }
+
+ @Override
public int nextIndex() {
throw new UnsupportedOperationException();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/DexConstantOptimizer.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/DexConstantOptimizer.java
index 740c180..e4845ca 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/DexConstantOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/DexConstantOptimizer.java
@@ -489,7 +489,8 @@
addConstantInBlock
.computeIfAbsent(dominator, k -> new LinkedHashMap<>())
.put(copy.outValue(), copy);
- assert iterator.peekPrevious() == instruction;
+ // Using peekPrevious() would disable remove().
+ assert iterator.previous() == instruction && iterator.next() == instruction;
iterator.removeOrReplaceByDebugLocalRead();
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/IteratorUtils.java b/src/main/java/com/android/tools/r8/utils/IteratorUtils.java
index 506d1e3b..6730a23 100644
--- a/src/main/java/com/android/tools/r8/utils/IteratorUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/IteratorUtils.java
@@ -88,13 +88,24 @@
return null;
}
+ /**
+ * Returns the previous element or null if !hasPrevious(). A subsequent call to iterator.remove()
+ * will remove the peeked element.
+ */
public static <T> T peekPrevious(ListIterator<T> iterator) {
- T previous = iterator.previous();
- T next = iterator.next();
- assert previous == next;
- return previous;
+ if (iterator.hasPrevious()) {
+ T previous = iterator.previous();
+ T next = iterator.next();
+ assert previous == next;
+ return previous;
+ }
+ return null;
}
+ /**
+ * Returns the next element or null if !hasNext(). A subsequent call to iterator.remove() will
+ * remove the peeked element.
+ */
public static <T> T peekNext(ListIterator<T> iterator) {
if (iterator.hasNext()) {
T next = iterator.next();
