Skip to content

Commit a94c383

Browse files
rbucktonrbuckton
authored
Merge pull request microsoft#27292 from kpreisser/fix26090
Align the ShimMap iterator behavior with native Maps
2 parents b564e15 + 5043bf7 commit a94c383

File tree

3 files changed

+241
-22
lines changed

3 files changed

+241
-22
lines changed

‎src/compiler/core.ts

Lines changed: 134 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -118,42 +118,105 @@ namespace ts {
118118
export const MapCtr = typeof Map !== "undefined" && "entries" in Map.prototype ? Map : shimMap();
119119

120120
// Keep the class inside a function so it doesn't get compiled if it's not used.
121-
function shimMap(): new <T>() => Map<T> {
121+
export function shimMap(): new <T>() => Map<T> {
122+
123+
interface MapEntry<T> {
124+
readonly key?: string;
125+
value?: T;
126+
127+
// Linked list references for iterators.
128+
nextEntry?: MapEntry<T>;
129+
previousEntry?: MapEntry<T>;
130+
131+
/**
132+
* Specifies if iterators should skip the next entry.
133+
* This will be set when an entry is deleted.
134+
* See https://github.com/Microsoft/TypeScript/pull/27292 for more information.
135+
*/
136+
skipNext?: boolean;
137+
}
122138

123139
class MapIterator<T, U extends (string | T | [string, T])> {
124-
private data: MapLike<T>;
125-
private keys: ReadonlyArray<string>;
126-
private index = 0;
127-
private selector: (data: MapLike<T>, key: string) => U;
128-
constructor(data: MapLike<T>, selector: (data: MapLike<T>, key: string) => U) {
129-
this.data = data;
140+
private currentEntry?: MapEntry<T>;
141+
private selector: (key: string, value: T) => U;
142+
143+
constructor(currentEntry: MapEntry<T>, selector: (key: string, value: T) => U) {
144+
this.currentEntry = currentEntry;
130145
this.selector = selector;
131-
this.keys = Object.keys(data);
132146
}
133147

134148
public next(): { value: U, done: false } | { value: never, done: true } {
135-
const index = this.index;
136-
if (index < this.keys.length) {
137-
this.index++;
138-
return { value: this.selector(this.data, this.keys[index]), done: false };
149+
// Navigate to the next entry.
150+
while (this.currentEntry) {
151+
const skipNext = !!this.currentEntry.skipNext;
152+
this.currentEntry = this.currentEntry.nextEntry;
153+
154+
if (!skipNext) {
155+
break;
156+
}
157+
}
158+
159+
if (this.currentEntry) {
160+
return { value: this.selector(this.currentEntry.key!, this.currentEntry.value!), done: false };
161+
}
162+
else {
163+
return { value: undefined as never, done: true };
139164
}
140-
return { value: undefined as never, done: true };
141165
}
142166
}
143167

144168
return class <T> implements Map<T> {
145-
private data = createDictionaryObject<T>();
169+
private data = createDictionaryObject<MapEntry<T>>();
146170
public size = 0;
147171

172+
// Linked list references for iterators.
173+
// See https://github.com/Microsoft/TypeScript/pull/27292
174+
// for more information.
175+
176+
/**
177+
* The first entry in the linked list.
178+
* Note that this is only a stub that serves as starting point
179+
* for iterators and doesn't contain a key and a value.
180+
*/
181+
private readonly firstEntry: MapEntry<T>;
182+
private lastEntry: MapEntry<T>;
183+
184+
constructor() {
185+
// Create a first (stub) map entry that will not contain a key
186+
// and value but serves as starting point for iterators.
187+
this.firstEntry = {};
188+
// When the map is empty, the last entry is the same as the
189+
// first one.
190+
this.lastEntry = this.firstEntry;
191+
}
192+
148193
get(key: string): T | undefined {
149-
return this.data[key];
194+
const entry = this.data[key] as MapEntry<T> | undefined;
195+
return entry && entry.value!;
150196
}
151197

152198
set(key: string, value: T): this {
153199
if (!this.has(key)) {
154200
this.size++;
201+
202+
// Create a new entry that will be appended at the
203+
// end of the linked list.
204+
const newEntry: MapEntry<T> = {
205+
key,
206+
value
207+
};
208+
this.data[key] = newEntry;
209+
210+
// Adjust the references.
211+
const previousLastEntry = this.lastEntry;
212+
previousLastEntry.nextEntry = newEntry;
213+
newEntry.previousEntry = previousLastEntry;
214+
this.lastEntry = newEntry;
155215
}
156-
this.data[key] = value;
216+
else {
217+
this.data[key].value = value;
218+
}
219+
157220
return this;
158221
}
159222

@@ -165,32 +228,81 @@ namespace ts {
165228
delete(key: string): boolean {
166229
if (this.has(key)) {
167230
this.size--;
231+
const entry = this.data[key];
168232
delete this.data[key];
233+
234+
// Adjust the linked list references of the neighbor entries.
235+
const previousEntry = entry.previousEntry!;
236+
previousEntry.nextEntry = entry.nextEntry;
237+
if (entry.nextEntry) {
238+
entry.nextEntry.previousEntry = previousEntry;
239+
}
240+
241+
// When the deleted entry was the last one, we need to
242+
// adust the lastEntry reference.
243+
if (this.lastEntry === entry) {
244+
this.lastEntry = previousEntry;
245+
}
246+
247+
// Adjust the forward reference of the deleted entry
248+
// in case an iterator still references it. This allows us
249+
// to throw away the entry, but when an active iterator
250+
// (which points to the current entry) continues, it will
251+
// navigate to the entry that originally came before the
252+
// current one and skip it.
253+
entry.previousEntry = undefined;
254+
entry.nextEntry = previousEntry;
255+
entry.skipNext = true;
256+
169257
return true;
170258
}
171259
return false;
172260
}
173261

174262
clear(): void {
175-
this.data = createDictionaryObject<T>();
263+
this.data = createDictionaryObject<MapEntry<T>>();
176264
this.size = 0;
265+
266+
// Reset the linked list. Note that we must adjust the forward
267+
// references of the deleted entries to ensure iterators stuck
268+
// in the middle of the list don't continue with deleted entries,
269+
// but can continue with new entries added after the clear()
270+
// operation.
271+
const firstEntry = this.firstEntry;
272+
let currentEntry = firstEntry.nextEntry;
273+
while (currentEntry) {
274+
const nextEntry = currentEntry.nextEntry;
275+
currentEntry.previousEntry = undefined;
276+
currentEntry.nextEntry = firstEntry;
277+
currentEntry.skipNext = true;
278+
279+
currentEntry = nextEntry;
280+
}
281+
firstEntry.nextEntry = undefined;
282+
this.lastEntry = firstEntry;
177283
}
178284

179285
keys(): Iterator<string> {
180-
return new MapIterator(this.data, (_data, key) => key);
286+
return new MapIterator(this.firstEntry, key => key);
181287
}
182288

183289
values(): Iterator<T> {
184-
return new MapIterator(this.data, (data, key) => data[key]);
290+
return new MapIterator(this.firstEntry, (_key, value) => value);
185291
}
186292

187293
entries(): Iterator<[string, T]> {
188-
return new MapIterator(this.data, (data, key) => [key, data[key]] as [string, T]);
294+
return new MapIterator(this.firstEntry, (key, value) => [key, value] as [string, T]);
189295
}
190296

191297
forEach(action: (value: T, key: string) => void): void {
192-
for (const key in this.data) {
193-
action(this.data[key], key);
298+
const iterator = this.entries();
299+
while (true) {
300+
const { value: entry, done } = iterator.next();
301+
if (done) {
302+
break;
303+
}
304+
305+
action(entry[1], entry[0]);
194306
}
195307
}
196308
};

‎src/testRunner/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
"unittests/publicApi.ts",
5959
"unittests/reuseProgramStructure.ts",
6060
"unittests/semver.ts",
61+
"unittests/shimMap.ts",
6162
"unittests/transform.ts",
6263
"unittests/tsbuild.ts",
6364
"unittests/tsbuildWatchMode.ts",

‎src/testRunner/unittests/shimMap.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
namespace ts {
2+
describe("unittests:: shimMap", () => {
3+
4+
function testMapIterationAddedValues(map: Map<string>, useForEach: boolean): string {
5+
let resultString = "";
6+
7+
map.set("1", "1");
8+
map.set("3", "3");
9+
map.set("2", "2");
10+
map.set("4", "4");
11+
12+
let addedThree = false;
13+
const doForEach = (value: string, key: string) => {
14+
resultString += `${key}:${value};`;
15+
16+
// Add a new key ("0") - the map should provide this
17+
// one in the next iteration.
18+
if (key === "1") {
19+
map.set("1", "X1");
20+
map.set("0", "X0");
21+
map.set("4", "X4");
22+
}
23+
else if (key === "3") {
24+
if (!addedThree) {
25+
addedThree = true;
26+
27+
// Remove and re-add key "3"; the map should
28+
// visit it after "0".
29+
map.delete("3");
30+
map.set("3", "Y3");
31+
32+
// Change the value of "2"; the map should provide
33+
// it when visiting the key.
34+
map.set("2", "Y2");
35+
}
36+
else {
37+
// Check that an entry added when we visit the
38+
// currently last entry will still be visited.
39+
map.set("999", "999");
40+
}
41+
}
42+
else if (key === "999") {
43+
// Ensure that clear() behaves correctly same as removing all keys.
44+
map.set("A", "A");
45+
map.set("B", "B");
46+
map.set("C", "C");
47+
}
48+
else if (key === "A") {
49+
map.clear();
50+
map.set("Z", "Z");
51+
}
52+
else if (key === "Z") {
53+
// Check that the map behaves correctly when two items are
54+
// added and removed immediately.
55+
map.set("X", "X");
56+
map.set("X1", "X1");
57+
map.set("X2", "X2");
58+
map.delete("X1");
59+
map.delete("X2");
60+
map.set("Y", "Y");
61+
}
62+
};
63+
64+
if (useForEach) {
65+
map.forEach(doForEach);
66+
}
67+
else {
68+
// Use an iterator.
69+
const iterator = map.entries();
70+
while (true) {
71+
const { value: tuple, done } = iterator.next();
72+
if (done) {
73+
break;
74+
}
75+
76+
doForEach(tuple[1], tuple[0]);
77+
}
78+
}
79+
80+
return resultString;
81+
}
82+
83+
it("iterates values in insertion order and handles changes", () => {
84+
const expectedResult = "1:1;3:3;2:Y2;4:X4;0:X0;3:Y3;999:999;A:A;Z:Z;X:X;Y:Y;";
85+
86+
// First, ensure the test actually has the same behavior as a native Map.
87+
let nativeMap = createMap<string>();
88+
const nativeMapForEachResult = testMapIterationAddedValues(nativeMap, /* useForEach */ true);
89+
assert.equal(nativeMapForEachResult, expectedResult, "nativeMap-forEach");
90+
91+
nativeMap = createMap<string>();
92+
const nativeMapIteratorResult = testMapIterationAddedValues(nativeMap, /* useForEach */ false);
93+
assert.equal(nativeMapIteratorResult, expectedResult, "nativeMap-iterator");
94+
95+
// Then, test the shimMap.
96+
let localShimMap = new (shimMap())<string>();
97+
const shimMapForEachResult = testMapIterationAddedValues(localShimMap, /* useForEach */ true);
98+
assert.equal(shimMapForEachResult, expectedResult, "shimMap-forEach");
99+
100+
localShimMap = new (shimMap())<string>();
101+
const shimMapIteratorResult = testMapIterationAddedValues(localShimMap, /* useForEach */ false);
102+
assert.equal(shimMapIteratorResult, expectedResult, "shimMap-iterator");
103+
104+
});
105+
});
106+
}

0 commit comments

Comments
 (0)