Fixes IteratorClose to throw correctly (#25)

mhassan1 · web-flow · commit c3b2ba1527a0 · 2024-03-21T15:17:57.000-04:00
This PR fixes `IteratorClose` to `throw` (instead of `return`) when a
`ThrowCompletion` is passed to it. It also includes new tests in a few
polyfills; those tests would fail, currently.
diff --git a/polyfills/Array/from/config.toml b/polyfills/Array/from/config.toml
@@ -26,6 +26,7 @@ dependencies = [
 	"_ESAbstract.ToObject",
 	"_ESAbstract.ToLength",
 	"_ESAbstract.Get",
+	"_ESAbstract.ThrowCompletion",
 	"Map",
 	"Set"
 ]
diff --git a/polyfills/Array/from/polyfill.js b/polyfills/Array/from/polyfill.js
@@ -1,5 +1,5 @@
 /* globals
-	IsCallable, GetMethod, Symbol, IsConstructor, Construct, ArrayCreate, GetIterator, IteratorClose,
+	IsCallable, GetMethod, Symbol, IsConstructor, Construct, ArrayCreate, GetIterator, IteratorClose, ThrowCompletion,
 	ToString, IteratorStep, IteratorValue, Call, CreateDataPropertyOrThrow, ToObject, ToLength, Get, CreateMethodProperty
 */
 (function () {
@@ -60,7 +60,7 @@
 				// i. If k ≥ 2^53-1, then
 				if (k >= (Math.pow(2, 53) - 1)) {
 					// 1. Let error be Completion{[[Type]]: throw, [[Value]]: a newly created TypeError object, [[Target]]: empty}.
-					var error = new TypeError('Iteration count can not be greater than or equal 9007199254740991.');
+					var error = ThrowCompletion(new TypeError('Iteration count can not be greater than or equal 9007199254740991.'));
 					// 2. Return ? IteratorClose(iteratorRecord, error).
 					return IteratorClose(iteratorRecord, error);
 				}
@@ -86,7 +86,7 @@
 						// 2. If mappedValue is an abrupt completion, return ? IteratorClose(iteratorRecord, mappedValue).
 						// 3. Let mappedValue be mappedValue.[[Value]].
 					} catch (e) {
-						return IteratorClose(iteratorRecord, e);
+						return IteratorClose(iteratorRecord, ThrowCompletion(e));
 					}
 
 					// vii. Else, let mappedValue be nextValue.
@@ -99,7 +99,7 @@
 					CreateDataPropertyOrThrow(A, Pk, mappedValue);
 					// ix. If defineStatus is an abrupt completion, return ? IteratorClose(iteratorRecord, defineStatus).
 				} catch (e) {
-					return IteratorClose(iteratorRecord, e);
+					return IteratorClose(iteratorRecord, ThrowCompletion(e));
 				}
 				// x. Increase k by 1.
 				k = k + 1;
diff --git a/polyfills/Array/from/tests.js b/polyfills/Array/from/tests.js
@@ -234,4 +234,12 @@ describe('throws', function () {
 			Array.from([1, 2, 3], 3);
 		});
 	});
+
+	it("specified, mapping function throws", function () {
+		proclaim.throws(function () {
+			Array.from([1, 2, 3], function () {
+				throw new Error("uh oh");
+			});
+		}, /uh oh/);
+	});
 });
diff --git a/polyfills/Map/config.toml b/polyfills/Map/config.toml
@@ -12,6 +12,7 @@ dependencies = [
 	"_ESAbstract.IteratorValue",
 	"_ESAbstract.OrdinaryCreateFromConstructor",
 	"_ESAbstract.SameValueZero",
+	"_ESAbstract.ThrowCompletion",
 	"_ESAbstract.Type",
 	"Symbol",
 	"Symbol.iterator",
diff --git a/polyfills/Map/groupBy/tests.js b/polyfills/Map/groupBy/tests.js
@@ -56,3 +56,11 @@ it("throws when callbackfn is not callable", function () {
 		Map.groupBy(iterable, null);
 	}, TypeError);
 });
+
+it("throws when callbackfn throws", function () {
+	proclaim.throws(function () {
+		Map.groupBy(iterable, function () {
+			throw new Error("uh oh");
+		});
+	}, /uh oh/);
+});
diff --git a/polyfills/Map/polyfill.js b/polyfills/Map/polyfill.js
@@ -1,4 +1,4 @@
-/* global CreateIterResultObject, CreateMethodProperty, GetIterator, IsCallable, IteratorClose, IteratorStep, IteratorValue, OrdinaryCreateFromConstructor, SameValueZero, Type, Symbol */
+/* global CreateIterResultObject, CreateMethodProperty, GetIterator, IsCallable, IteratorClose, IteratorStep, IteratorValue, OrdinaryCreateFromConstructor, SameValueZero, ThrowCompletion, Type, Symbol */
 (function (global) {
 	// Need an internal counter to assign unique IDs to a key map
 	var _uniqueHashId = 0;
@@ -149,12 +149,9 @@
 				// d. If Type(nextItem) is not Object, then
 				if (Type(nextItem) !== 'object') {
 					// i. Let error be Completion{[[Type]]: throw, [[Value]]: a newly created TypeError object, [[Target]]: empty}.
-					try {
-						throw new TypeError('Iterator value ' + nextItem + ' is not an entry object');
-					} catch (error) {
-						// ii. Return ? IteratorClose(iteratorRecord, error).
-						return IteratorClose(iteratorRecord, error);
-					}
+					var error = ThrowCompletion(new TypeError('Iterator value ' + nextItem + ' is not an entry object'));
+					// ii. Return ? IteratorClose(iteratorRecord, error).
+					return IteratorClose(iteratorRecord, error);
 				}
 				try {
 					// The try catch accounts for steps: f, h, and j.
@@ -169,7 +166,7 @@
 					adder.call(map, k, v);
 				} catch (e) {
 					// j. If status is an abrupt completion, return ? IteratorClose(iteratorRecord, status).
-					return IteratorClose(iteratorRecord, e);
+					return IteratorClose(iteratorRecord, ThrowCompletion(e));
 				}
 			}
 		} catch (e) {
diff --git a/polyfills/Object/groupBy/tests.js b/polyfills/Object/groupBy/tests.js
@@ -54,3 +54,11 @@ it("throws when callbackfn is not callable", function () {
 		Object.groupBy(iterable, null);
 	}, TypeError);
 });
+
+it("throws when callbackfn throws", function () {
+	proclaim.throws(function () {
+		Object.groupBy(iterable, function () {
+			throw new Error("uh oh");
+		});
+	}, /uh oh/);
+});
diff --git a/polyfills/Set/config.toml b/polyfills/Set/config.toml
@@ -12,6 +12,7 @@ dependencies = [
 	"_ESAbstract.IteratorValue",
 	"_ESAbstract.OrdinaryCreateFromConstructor",
 	"_ESAbstract.SameValueZero",
+	"_ESAbstract.ThrowCompletion",
 	"Symbol",
 	"Symbol.iterator",
 	"Symbol.species",
diff --git a/polyfills/Set/polyfill.js b/polyfills/Set/polyfill.js
@@ -1,4 +1,4 @@
-/* global CreateIterResultObject, CreateMethodProperty, GetIterator, IsCallable, IteratorClose, IteratorStep, IteratorValue, OrdinaryCreateFromConstructor, SameValueZero, Symbol */
+/* global CreateIterResultObject, CreateMethodProperty, GetIterator, IsCallable, IteratorClose, IteratorStep, IteratorValue, OrdinaryCreateFromConstructor, SameValueZero, Symbol, ThrowCompletion */
 (function (global) {
 	// Deleted set items mess with iterator pointers, so rather than removing them mark them as deleted. Can't use undefined or null since those both valid keys so use a private symbol.
 	var undefMarker = Symbol('undef');
@@ -52,7 +52,7 @@
 					adder.call(set, nextValue);
 				} catch (e) {
 					// e. If status is an abrupt completion, return ? IteratorClose(iteratorRecord, status).
-					return IteratorClose(iteratorRecord, e);
+					return IteratorClose(iteratorRecord, ThrowCompletion(e));
 				}
 			}
 		} catch (e) {
diff --git a/polyfills/WeakMap/config.toml b/polyfills/WeakMap/config.toml
@@ -12,6 +12,7 @@ dependencies = [
 	"_ESAbstract.IsArray",
 	"_ESAbstract.Type",
 	"_ESAbstract.SameValue",
+	"_ESAbstract.ThrowCompletion",
 	"Symbol",
 	"Symbol.toStringTag",
 ]
diff --git a/polyfills/WeakMap/polyfill.js b/polyfills/WeakMap/polyfill.js
@@ -1,4 +1,4 @@
-/* globals Symbol, OrdinaryCreateFromConstructor, IsCallable, GetIterator, IteratorStep, IteratorValue, IteratorClose, Get, Call, CreateMethodProperty, Type, SameValue */
+/* globals Symbol, OrdinaryCreateFromConstructor, IsCallable, GetIterator, IteratorStep, IteratorValue, IteratorClose, Get, Call, CreateMethodProperty, ThrowCompletion, Type, SameValue */
 (function (global) {
 	// Deleted map items mess with iterator pointers, so rather than removing them mark them as deleted. Can't use undefined or null since those both valid keys so use a private symbol.
 	var undefMarker = Symbol('undef');
@@ -51,12 +51,9 @@
 				// d. If Type(nextItem) is not Object, then
 				if (Type(nextItem) !== 'object') {
 					// i. Let error be Completion{[[Type]]: throw, [[Value]]: a newly created TypeError object, [[Target]]: empty}.
-					try {
-						throw new TypeError('Iterator value ' + nextItem + ' is not an entry object');
-					} catch (error) {
-						// ii. Return ? IteratorClose(iteratorRecord, error).
-						return IteratorClose(iteratorRecord, error);
-					}
+					var error = ThrowCompletion(new TypeError('Iterator value ' + nextItem + ' is not an entry object'));
+					// ii. Return ? IteratorClose(iteratorRecord, error).
+					return IteratorClose(iteratorRecord, error);
 				}
 				try {
 					// The try catch accounts for steps: f, h, and j.
@@ -71,7 +68,7 @@
 					Call(adder, map, [k, v]);
 				} catch (e) {
 					// j. If status is an abrupt completion, return ? IteratorClose(iteratorRecord, status).
-					return IteratorClose(iteratorRecord, e);
+					return IteratorClose(iteratorRecord, ThrowCompletion(e));
 				}
 			}
 		} catch (e) {
diff --git a/polyfills/WeakSet/config.toml b/polyfills/WeakSet/config.toml
@@ -12,6 +12,7 @@ dependencies = [
 	"_ESAbstract.IsArray",
 	"_ESAbstract.Type",
 	"_ESAbstract.SameValueZero",
+	"_ESAbstract.ThrowCompletion",
 	"Symbol",
 	"Symbol.toStringTag",
 ]
diff --git a/polyfills/WeakSet/polyfill.js b/polyfills/WeakSet/polyfill.js
@@ -1,4 +1,4 @@
-/* global Call, CreateMethodProperty, Get, GetIterator, IsArray, IsCallable, IteratorClose, IteratorStep, IteratorValue, OrdinaryCreateFromConstructor, SameValueZero, Type, Symbol */
+/* global Call, CreateMethodProperty, Get, GetIterator, IsArray, IsCallable, IteratorClose, IteratorStep, IteratorValue, OrdinaryCreateFromConstructor, SameValueZero, ThrowCompletion, Type, Symbol */
 (function (global) {
 	// Deleted set items mess with iterator pointers, so rather than removing them mark them as deleted. Can't use undefined or null since those both valid keys so use a private symbol.
 	var undefMarker = Symbol('undef');
@@ -49,7 +49,7 @@
 					Call(adder, set, [nextValue]);
 				} catch (e) {
 					// e. If status is an abrupt completion, return ? IteratorClose(iteratorRecord, status).
-					return IteratorClose(iteratorRecord, e);
+					return IteratorClose(iteratorRecord, ThrowCompletion(e));
 				}
 			}
 		} catch (e) {
diff --git a/polyfills/_ESAbstract/AddEntriesFromIterable/config.toml b/polyfills/_ESAbstract/AddEntriesFromIterable/config.toml
@@ -6,6 +6,7 @@ dependencies = [
 	"_ESAbstract.IteratorClose",
 	"_ESAbstract.Get",
 	"_ESAbstract.Call",
+	"_ESAbstract.ThrowCompletion",
 	"_ESAbstract.Type"
 ]
 spec = "https://tc39.github.io/ecma262/#sec-add-entries-from-iterable"
diff --git a/polyfills/_ESAbstract/AddEntriesFromIterable/polyfill.js b/polyfills/_ESAbstract/AddEntriesFromIterable/polyfill.js
@@ -1,4 +1,4 @@
-/* global IsCallable, GetIterator, IteratorStep, IteratorValue, IteratorClose, Get, Call, Type */
+/* global IsCallable, GetIterator, IteratorStep, IteratorValue, IteratorClose, Get, Call, ThrowCompletion, Type */
 // eslint-disable-next-line no-unused-vars
 var AddEntriesFromIterable = (function() {
 	var toString = {}.toString;
@@ -26,10 +26,9 @@ var AddEntriesFromIterable = (function() {
 			// d. If Type(nextItem) is not Object, then
 			if (Type(nextItem) !== "object") {
 				// i. Let error be ThrowCompletion(a newly created TypeError object).
-				var error = new TypeError("nextItem is not an object");
+				var error = ThrowCompletion(new TypeError("nextItem is not an object"));
 				// ii. Return ? IteratorClose(iteratorRecord, error).
-				IteratorClose(iteratorRecord, error);
-				throw error;
+				return IteratorClose(iteratorRecord, error);
 			}
 			// fallback for non-array-like strings which exist in some ES3 user-agents
 			nextItem =
@@ -42,26 +41,26 @@ var AddEntriesFromIterable = (function() {
 				// e. Let k be Get(nextItem, "0").
 				k = Get(nextItem, "0");
 				// eslint-disable-next-line no-catch-shadow
-			} catch (k) {
+			} catch (e) {
 				// f. If k is an abrupt completion, return ? IteratorClose(iteratorRecord, k).
-				return IteratorClose(iteratorRecord, k);
+				return IteratorClose(iteratorRecord, ThrowCompletion(e));
 			}
 			var v;
 			try {
 				// g. Let v be Get(nextItem, "1").
 				v = Get(nextItem, "1");
 				// eslint-disable-next-line no-catch-shadow
-			} catch (v) {
+			} catch (e) {
 				// h. If v is an abrupt completion, return ? IteratorClose(iteratorRecord, v).
-				return IteratorClose(iteratorRecord, v);
+				return IteratorClose(iteratorRecord, ThrowCompletion(e));
 			}
 			try {
 				// i. Let status be Call(adder, target, « k.[[Value]], v.[[Value]] »).
 				Call(adder, target, [k, v]);
 				// eslint-disable-next-line no-catch-shadow
-			} catch (status) {
+			} catch (e) {
 				// j. If status is an abrupt completion, return ? IteratorClose(iteratorRecord, status).
-				return IteratorClose(iteratorRecord, status);
+				return IteratorClose(iteratorRecord, ThrowCompletion(e));
 			}
 		}
 	};
diff --git a/polyfills/_ESAbstract/GroupBy/config.toml b/polyfills/_ESAbstract/GroupBy/config.toml
@@ -7,6 +7,7 @@ dependencies = [
 	"_ESAbstract.IteratorStep",
 	"_ESAbstract.IteratorValue",
 	"_ESAbstract.RequireObjectCoercible",
+	"_ESAbstract.ThrowCompletion",
 	"_ESAbstract.ToPropertyKey",
 	"Number.MAX_SAFE_INTEGER",
 ]
diff --git a/polyfills/_ESAbstract/GroupBy/polyfill.js b/polyfills/_ESAbstract/GroupBy/polyfill.js
@@ -1,4 +1,4 @@
-/* global AddValueToKeyedGroup, Call, GetIterator, IsCallable, IteratorClose, IteratorStep, IteratorValue, RequireObjectCoercible, ToPropertyKey */
+/* global AddValueToKeyedGroup, Call, GetIterator, IsCallable, IteratorClose, IteratorStep, IteratorValue, RequireObjectCoercible, ThrowCompletion, ToPropertyKey */
 
 // 7.3.36 GroupBy ( items, callbackfn, keyCoercion )
 // eslint-disable-next-line no-unused-vars
@@ -21,7 +21,7 @@ function GroupBy(items, callbackfn, keyCoercion) {
 		// a. If k ≥ 253 - 1, then
 		if (k >= Number.MAX_SAFE_INTEGER) {
 			// i. Let error be ThrowCompletion(a newly created TypeError object).
-			var error = new TypeError("k greater than or equal to MAX_SAFE_INTEGER");
+			var error = ThrowCompletion(new TypeError("k greater than or equal to MAX_SAFE_INTEGER"));
 			// ii. Return ? IteratorClose(iteratorRecord, error).
 			return IteratorClose(iteratorRecord, error);
 		}
@@ -40,7 +40,7 @@ function GroupBy(items, callbackfn, keyCoercion) {
 			key = Call(callbackfn, undefined, [value, k]);
 		} catch (err) {
 			// f. IfAbruptCloseIterator(key, iteratorRecord).
-			return IteratorClose(iteratorRecord, err);
+			return IteratorClose(iteratorRecord, ThrowCompletion(err));
 		}
 		// g. If keyCoercion is property, then
 		if (keyCoercion === "property") {
@@ -49,7 +49,7 @@ function GroupBy(items, callbackfn, keyCoercion) {
 				key = ToPropertyKey(key);
 			} catch (err) {
 				// ii. IfAbruptCloseIterator(key, iteratorRecord).
-				return IteratorClose(iteratorRecord, err);
+				return IteratorClose(iteratorRecord, ThrowCompletion(err));
 			}
 		}
 		// h. Else,
diff --git a/polyfills/_ESAbstract/IteratorClose/polyfill.js b/polyfills/_ESAbstract/IteratorClose/polyfill.js
@@ -1,21 +1,32 @@
 /* global GetMethod, Type, Call */
 // 7.4.6. IteratorClose ( iteratorRecord, completion )
 function IteratorClose(iteratorRecord, completion) { // eslint-disable-line no-unused-vars
+	function complete() {
+		if (completion["[[Type]]"] === "throw") {
+			throw completion["[[Value]]"];
+		}
+		return completion["[[Value]]"];
+	}
+
 	// 1. Assert: Type(iteratorRecord.[[Iterator]]) is Object.
 	if (Type(iteratorRecord['[[Iterator]]']) !== 'object') {
 		throw new Error(Object.prototype.toString.call(iteratorRecord['[[Iterator]]']) + 'is not an Object.');
 	}
 	// 2. Assert: completion is a Completion Record.
-	// Ignoring this step as there is no way to check if something is a Completion Record in userland JavaScript.
-
 	// 3. Let iterator be iteratorRecord.[[Iterator]].
 	var iterator = iteratorRecord['[[Iterator]]'];
 	// 4. Let return be ? GetMethod(iterator, "return").
 	// We name it  returnMethod because return is a keyword and can not be used as an identifier (E.G. variable name, function name etc).
-	var returnMethod = GetMethod(iterator, "return");
+	var returnMethod;
+	try {
+		returnMethod = GetMethod(iterator, "return");
+	} catch (error) {
+		complete();
+		throw error;
+	}
 	// 5. If return is undefined, return Completion(completion).
 	if (returnMethod === undefined) {
-		return completion;
+		return complete();
 	}
 	// 6. Let innerResult be Call(return, iterator, « »).
 	try {
@@ -24,9 +35,7 @@ function IteratorClose(iteratorRecord, completion) { // eslint-disable-line no-u
 		var innerException = error;
 	}
 	// 7. If completion.[[Type]] is throw, return Completion(completion).
-	if (completion) {
-		return completion;
-	}
+	complete();
 	// 8. If innerResult.[[Type]] is throw, return Completion(innerResult).
 	if (innerException) {
 		throw innerException;
@@ -36,5 +45,5 @@ function IteratorClose(iteratorRecord, completion) { // eslint-disable-line no-u
 		throw new TypeError("Iterator's return method returned a non-object.");
 	}
 	// 10. Return Completion(completion).
-	return completion;
+	return complete();
 }
diff --git a/polyfills/_ESAbstract/NormalCompletion/config.toml b/polyfills/_ESAbstract/NormalCompletion/config.toml
@@ -0,0 +1,19 @@
+dependencies = []
+spec = "https://tc39.es/ecma262/#sec-normalcompletion"
+
+[browsers]
+android = "*"
+bb = "*"
+chrome = "*"
+edge = "*"
+edge_mob = "*"
+firefox = "*"
+firefox_mob = "*"
+ie = "*"
+ie_mob = "*"
+opera = "*"
+op_mob = "*"
+op_mini = "*"
+safari = "*"
+ios_saf = "*"
+samsung_mob = "*"
diff --git a/polyfills/_ESAbstract/NormalCompletion/polyfill.js b/polyfills/_ESAbstract/NormalCompletion/polyfill.js
@@ -0,0 +1,8 @@
+// 6.2.4.1 NormalCompletion ( value )
+// eslint-disable-next-line no-unused-vars
+function NormalCompletion(value) {
+	return {
+		"[[Type]]": "normal",
+		"[[Value]]": value
+	};
+}
diff --git a/polyfills/_ESAbstract/ThrowCompletion/config.toml b/polyfills/_ESAbstract/ThrowCompletion/config.toml
diff --git a/polyfills/_ESAbstract/ThrowCompletion/polyfill.js b/polyfills/_ESAbstract/ThrowCompletion/polyfill.js

Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,7 @@ dependencies = [`
`26`	`26`	`"_ESAbstract.ToObject",`
`27`	`27`	`"_ESAbstract.ToLength",`
`28`	`28`	`"_ESAbstract.Get",`
	`29`	`+ "_ESAbstract.ThrowCompletion",`
`29`	`30`	`"Map",`
`30`	`31`	`"Set"`
`31`	`32`	`]`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-/* global CreateIterResultObject, CreateMethodProperty, GetIterator, IsCallable, IteratorClose, IteratorStep, IteratorValue, OrdinaryCreateFromConstructor, SameValueZero, Symbol */`
	`1`	`+/* global CreateIterResultObject, CreateMethodProperty, GetIterator, IsCallable, IteratorClose, IteratorStep, IteratorValue, OrdinaryCreateFromConstructor, SameValueZero, Symbol, ThrowCompletion */`
`2`	`2`	`(function (global) {`
`3`	`3`	`// Deleted set items mess with iterator pointers, so rather than removing them mark them as deleted. Can't use undefined or null since those both valid keys so use a private symbol.`
`4`	`4`	`var undefMarker = Symbol('undef');`
`@@ -52,7 +52,7 @@`
`52`	`52`	`adder.call(set, nextValue);`
`53`	`53`	`} catch (e) {`
`54`	`54`	`// e. If status is an abrupt completion, return ? IteratorClose(iteratorRecord, status).`
`55`		`- return IteratorClose(iteratorRecord, e);`
	`55`	`+ return IteratorClose(iteratorRecord, ThrowCompletion(e));`
`56`	`56`	`}`
`57`	`57`	`}`
`58`	`58`	`} catch (e) {`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ dependencies = [`
`12`	`12`	`"_ESAbstract.IsArray",`
`13`	`13`	`"_ESAbstract.Type",`
`14`	`14`	`"_ESAbstract.SameValue",`
	`15`	`+ "_ESAbstract.ThrowCompletion",`
`15`	`16`	`"Symbol",`
`16`	`17`	`"Symbol.toStringTag",`
`17`	`18`	`]`