8144733: Iterating over elements of a Scope can return spurious inner class elements
When a Symbol is removed from a Scope while iterating over it, update the iterator as well to reflect the change. Reviewed-by: mcimadamore
This commit is contained in:
parent
335b44a953
commit
d898c2f21c
langtools
src/jdk.compiler/share/classes/com/sun/tools/javac/code
test/tools/javac/scope
@ -296,6 +296,8 @@ public abstract class Scope {
|
||||
*/
|
||||
int nelems = 0;
|
||||
|
||||
int removeCount = 0;
|
||||
|
||||
/** Use as a "not-found" result for lookup.
|
||||
* Also used to mark deleted entries in the table.
|
||||
*/
|
||||
@ -474,6 +476,8 @@ public abstract class Scope {
|
||||
te = te.sibling;
|
||||
}
|
||||
|
||||
removeCount++;
|
||||
|
||||
//notify listeners
|
||||
listeners.symbolRemoved(sym, this);
|
||||
}
|
||||
@ -569,15 +573,29 @@ public abstract class Scope {
|
||||
return new Iterator<Symbol>() {
|
||||
private ScopeImpl currScope = ScopeImpl.this;
|
||||
private Scope.Entry currEntry = elems;
|
||||
private int seenRemoveCount = currScope.removeCount;
|
||||
{
|
||||
update();
|
||||
}
|
||||
|
||||
public boolean hasNext() {
|
||||
if (seenRemoveCount != currScope.removeCount &&
|
||||
currEntry != null &&
|
||||
!currEntry.scope.includes(currEntry.sym)) {
|
||||
doNext(); //skip entry that is no longer in the Scope
|
||||
seenRemoveCount = currScope.removeCount;
|
||||
}
|
||||
return currEntry != null;
|
||||
}
|
||||
|
||||
public Symbol next() {
|
||||
if (!hasNext()) {
|
||||
throw new NoSuchElementException();
|
||||
}
|
||||
|
||||
return doNext();
|
||||
}
|
||||
private Symbol doNext() {
|
||||
Symbol sym = (currEntry == null ? null : currEntry.sym);
|
||||
if (currEntry != null) {
|
||||
currEntry = currEntry.sibling;
|
||||
@ -596,6 +614,7 @@ public abstract class Scope {
|
||||
while (currEntry == null && currScope.next != null) {
|
||||
currScope = currScope.next;
|
||||
currEntry = currScope.elems;
|
||||
seenRemoveCount = currScope.removeCount;
|
||||
skipToNextMatchingEntry();
|
||||
}
|
||||
}
|
||||
@ -618,13 +637,26 @@ public abstract class Scope {
|
||||
public Iterator<Symbol> iterator() {
|
||||
return new Iterator<Symbol>() {
|
||||
Scope.Entry currentEntry = lookup(name, sf);
|
||||
int seenRemoveCount = currentEntry.scope != null ?
|
||||
currentEntry.scope.removeCount : -1;
|
||||
|
||||
public boolean hasNext() {
|
||||
if (currentEntry.scope != null &&
|
||||
seenRemoveCount != currentEntry.scope.removeCount &&
|
||||
!currentEntry.scope.includes(currentEntry.sym)) {
|
||||
doNext(); //skip entry that is no longer in the Scope
|
||||
}
|
||||
return currentEntry.scope != null &&
|
||||
(lookupKind == RECURSIVE ||
|
||||
currentEntry.scope == ScopeImpl.this);
|
||||
}
|
||||
public Symbol next() {
|
||||
if (!hasNext()) {
|
||||
throw new NoSuchElementException();
|
||||
}
|
||||
return doNext();
|
||||
}
|
||||
private Symbol doNext() {
|
||||
Scope.Entry prevEntry = currentEntry;
|
||||
currentEntry = currentEntry.next(sf);
|
||||
return prevEntry.sym;
|
||||
@ -686,9 +718,9 @@ public abstract class Scope {
|
||||
/** The entry's scope.
|
||||
* scope == null iff this == sentinel
|
||||
*/
|
||||
public Scope scope;
|
||||
public ScopeImpl scope;
|
||||
|
||||
public Entry(Symbol sym, Entry shadowed, Entry sibling, Scope scope) {
|
||||
public Entry(Symbol sym, Entry shadowed, Entry sibling, ScopeImpl scope) {
|
||||
this.sym = sym;
|
||||
this.shadowed = shadowed;
|
||||
this.sibling = sibling;
|
||||
|
106
langtools/test/tools/javac/scope/IterateAndRemove.java
Normal file
106
langtools/test/tools/javac/scope/IterateAndRemove.java
Normal file
@ -0,0 +1,106 @@
|
||||
/*
|
||||
* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
|
||||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
|
||||
*
|
||||
* This code is free software; you can redistribute it and/or modify it
|
||||
* under the terms of the GNU General Public License version 2 only, as
|
||||
* published by the Free Software Foundation.
|
||||
*
|
||||
* This code is distributed in the hope that it will be useful, but WITHOUT
|
||||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
|
||||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
|
||||
* version 2 for more details (a copy is included in the LICENSE file that
|
||||
* accompanied this code).
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License version
|
||||
* 2 along with this work; if not, write to the Free Software Foundation,
|
||||
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
|
||||
*
|
||||
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
|
||||
* or visit www.oracle.com if you need additional information or have any
|
||||
* questions.
|
||||
*/
|
||||
|
||||
/**
|
||||
* @test
|
||||
* @bug 8144733
|
||||
* @summary Verify that Scope.remove removes the Symbol also from already running iterations.
|
||||
* @modules jdk.compiler/com.sun.tools.javac.code
|
||||
* jdk.compiler/com.sun.tools.javac.util
|
||||
*/
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.function.Function;
|
||||
|
||||
import com.sun.tools.javac.code.Scope;
|
||||
import com.sun.tools.javac.code.Scope.WriteableScope;
|
||||
import com.sun.tools.javac.code.Symbol;
|
||||
import com.sun.tools.javac.code.Symbol.PackageSymbol;
|
||||
import com.sun.tools.javac.util.Context;
|
||||
import com.sun.tools.javac.util.Name;
|
||||
import com.sun.tools.javac.util.Names;
|
||||
|
||||
public class IterateAndRemove {
|
||||
public static void main(String... args) {
|
||||
new IterateAndRemove().run();
|
||||
}
|
||||
|
||||
void run() {
|
||||
Context ctx = new Context();
|
||||
Names names = Names.instance(ctx);
|
||||
Symbol root = new PackageSymbol(names.empty, null);
|
||||
Name one = names.fromString("1");
|
||||
PackageSymbol sym1 = new PackageSymbol(one, new PackageSymbol(names.fromString("a"), root));
|
||||
PackageSymbol sym2 = new PackageSymbol(one, new PackageSymbol(names.fromString("b"), root));
|
||||
PackageSymbol sym3 = new PackageSymbol(one, new PackageSymbol(names.fromString("c"), root));
|
||||
List<Symbol> symbols = Arrays.asList(sym1, sym2, sym3);
|
||||
|
||||
List<Function<Scope, Iterable<Symbol>>> getters = Arrays.asList(
|
||||
scope -> scope.getSymbols(),
|
||||
scope -> scope.getSymbolsByName(one)
|
||||
);
|
||||
for (Function<Scope, Iterable<Symbol>> scope2Content : getters) {
|
||||
for (int removeAt : new int[] {0, 1, 2, 3}) {
|
||||
for (Symbol removeWhat : new Symbol[] {sym1, sym2, sym3}) {
|
||||
WriteableScope s = WriteableScope.create(root);
|
||||
|
||||
symbols.forEach(s :: enter);
|
||||
|
||||
Iterator<Symbol> it = scope2Content.apply(s).iterator();
|
||||
List<PackageSymbol> actual = new ArrayList<>();
|
||||
int count = 0;
|
||||
|
||||
while (true) {
|
||||
if (count++ == removeAt)
|
||||
s.remove(removeWhat);
|
||||
if (!it.hasNext())
|
||||
break;
|
||||
actual.add((PackageSymbol) it.next());
|
||||
}
|
||||
|
||||
List<Symbol> copy = new ArrayList<>(symbols);
|
||||
|
||||
Collections.reverse(copy);
|
||||
|
||||
count = 0;
|
||||
|
||||
while (true) {
|
||||
if (count == removeAt && copy.indexOf(removeWhat) >= count)
|
||||
copy.remove(removeWhat);
|
||||
count++;
|
||||
if (count >= copy.size())
|
||||
break;
|
||||
}
|
||||
|
||||
if (!copy.equals(actual)) {
|
||||
throw new AssertionError("differs: actual: " + actual + "; expected: " + copy);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
Loading…
x
Reference in New Issue
Block a user