From 312fc2c0842a380d3379d686992c346adc403f0f Mon Sep 17 00:00:00 2001 From: Julian Garn Date: Wed, 14 Sep 2022 13:40:50 +0200 Subject: [PATCH] Fail earlier Spec change: https://github.com/tc39/proposal-iterator-helpers/pull/232 --- .../AsyncIteratorFunctionBuiltinsTest.java | 9 ++++++++- .../builtins/IteratorFunctionBuiltinsTest.java | 7 +++++++ .../builtins/AsyncIteratorFunctionBuiltins.java | 15 +++++++++++++++ .../js/builtins/IteratorFunctionBuiltins.java | 13 ++++++++++++- 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/AsyncIteratorFunctionBuiltinsTest.java b/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/AsyncIteratorFunctionBuiltinsTest.java index 9badf02ff9d..8caa8303600 100644 --- a/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/AsyncIteratorFunctionBuiltinsTest.java +++ b/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/AsyncIteratorFunctionBuiltinsTest.java @@ -76,7 +76,7 @@ public void testFrom() { Context.Builder builder = JSTest.newContextBuilder(); builder.option(JSContextOptions.ITERATOR_HELPERS_NAME, "true"); try (Context context = builder.build()) { - Value result = context.eval(JavaScriptLanguage.ID, "AsyncIterator.from({[Symbol.asyncIterator]: async () => ({next: () => ({done: true})})})"); + Value result = context.eval(JavaScriptLanguage.ID, "AsyncIterator.from({[Symbol.asyncIterator]: () => ({next: () => ({done: true})})})"); Assert.assertTrue(result.hasMembers()); Assert.assertTrue(result.hasMember("next")); Assert.assertTrue(result.getMember("next").canExecute()); @@ -99,6 +99,13 @@ public void testFrom() { result = context.eval(JavaScriptLanguage.ID, "var x = (async function* test(){})(); AsyncIterator.from(x) === x"); Assert.assertTrue(result.asBoolean()); + + try { + context.eval(JavaScriptLanguage.ID, "AsyncIterator.from({[Symbol.asyncIterator]: async () => ({next: () => ({done: true})})})"); + Assert.fail("No exception thrown"); + } catch (PolyglotException e) { + Assert.assertTrue(e.getMessage().startsWith("TypeError: ")); + } } } } diff --git a/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/IteratorFunctionBuiltinsTest.java b/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/IteratorFunctionBuiltinsTest.java index 3a14b00074f..760e9f4eb86 100644 --- a/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/IteratorFunctionBuiltinsTest.java +++ b/graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/builtins/IteratorFunctionBuiltinsTest.java @@ -91,6 +91,13 @@ public void testFrom() { result = context.eval(JavaScriptLanguage.ID, "var x = [].values(); Iterator.from(x) === x"); Assert.assertTrue(result.asBoolean()); + + try { + context.eval(JavaScriptLanguage.ID, "Iterator.from({[Symbol.iterator]: async () => ({next: () => ({done: true})})})"); + Assert.fail("No exception thrown"); + } catch (PolyglotException e) { + Assert.assertTrue(e.getMessage().startsWith("TypeError: ")); + } } } } diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/AsyncIteratorFunctionBuiltins.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/AsyncIteratorFunctionBuiltins.java index e131dd1e129..95b7ba780e9 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/AsyncIteratorFunctionBuiltins.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/AsyncIteratorFunctionBuiltins.java @@ -42,6 +42,7 @@ import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.profiles.BranchProfile; import com.oracle.truffle.api.profiles.ConditionProfile; import com.oracle.truffle.js.nodes.access.GetIteratorDirectNode; import com.oracle.truffle.js.nodes.access.GetIteratorNode; @@ -49,6 +50,8 @@ import com.oracle.truffle.js.nodes.binary.InstanceofNode.OrdinaryHasInstanceNode; import com.oracle.truffle.js.nodes.function.JSBuiltin; import com.oracle.truffle.js.nodes.function.JSBuiltinNode; +import com.oracle.truffle.js.nodes.unary.IsCallableNode; +import com.oracle.truffle.js.runtime.Errors; import com.oracle.truffle.js.runtime.JSContext; import com.oracle.truffle.js.runtime.Symbol; import com.oracle.truffle.js.runtime.builtins.BuiltinEnum; @@ -98,8 +101,10 @@ public abstract static class JSAsyncIteratorFromNode extends JSBuiltinNode { @Child private OrdinaryHasInstanceNode ordinaryHasInstanceNode; @Child private GetIteratorDirectNode getIteratorDirectNode; + @Child private IsCallableNode isCallableNode; private final ConditionProfile usingIteratorProfile = ConditionProfile.createBinaryProfile(); + private final BranchProfile errorProfile = BranchProfile.create(); public JSAsyncIteratorFromNode(JSContext context, JSBuiltin builtin) { super(context, builtin); @@ -120,6 +125,16 @@ protected JSDynamicObject asyncIteratorFrom(Object arg) { iteratorRecord = getIteratorNode.execute(arg); + if (isCallableNode == null) { + CompilerDirectives.transferToInterpreterAndInvalidate(); + isCallableNode = insert(IsCallableNode.create()); + } + + if (!isCallableNode.executeBoolean(iteratorRecord.getNextMethod())) { + errorProfile.enter(); + throw Errors.createTypeErrorCallableExpected(); + } + if (ordinaryHasInstanceNode == null) { CompilerDirectives.transferToInterpreterAndInvalidate(); ordinaryHasInstanceNode = insert(OrdinaryHasInstanceNode.create(getContext())); diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/IteratorFunctionBuiltins.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/IteratorFunctionBuiltins.java index a4358f6d443..f81745b57f2 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/IteratorFunctionBuiltins.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/IteratorFunctionBuiltins.java @@ -41,6 +41,7 @@ package com.oracle.truffle.js.builtins; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.profiles.BranchProfile; import com.oracle.truffle.api.profiles.ConditionProfile; import com.oracle.truffle.js.builtins.IteratorFunctionBuiltinsFactory.JSIteratorFromNodeGen; import com.oracle.truffle.js.nodes.access.GetIteratorDirectNode; @@ -49,6 +50,8 @@ import com.oracle.truffle.js.nodes.binary.InstanceofNode.OrdinaryHasInstanceNode; import com.oracle.truffle.js.nodes.function.JSBuiltin; import com.oracle.truffle.js.nodes.function.JSBuiltinNode; +import com.oracle.truffle.js.nodes.unary.IsCallableNode; +import com.oracle.truffle.js.runtime.Errors; import com.oracle.truffle.js.runtime.JSContext; import com.oracle.truffle.js.runtime.Symbol; import com.oracle.truffle.js.runtime.builtins.BuiltinEnum; @@ -99,8 +102,10 @@ public abstract static class JSIteratorFromNode extends JSBuiltinNode { @Child private GetIteratorNode getIteratorNode; @Child private GetIteratorDirectNode getIteratorDirectNode; @Child private OrdinaryHasInstanceNode ordinaryHasInstanceNode; + @Child private IsCallableNode isCallableNode; private final ConditionProfile usingIteratorProfile = ConditionProfile.createBinaryProfile(); + private final BranchProfile errorProfile = BranchProfile.create(); public JSIteratorFromNode(JSContext context, JSBuiltin builtin) { super(context, builtin); @@ -108,16 +113,22 @@ public JSIteratorFromNode(JSContext context, JSBuiltin builtin) { this.getIteratorNode = GetIteratorNode.create(getContext()); this.ordinaryHasInstanceNode = OrdinaryHasInstanceNode.create(getContext()); this.getIteratorDirectNode = GetIteratorDirectNode.create(getContext()); + this.isCallableNode = IsCallableNode.create(); } @Specialization protected JSDynamicObject iteratorFrom(Object arg) { - IteratorRecord iteratorRecord = null; + IteratorRecord iteratorRecord; Object usingIterator = getIteratorMethodNode.executeWithTarget(arg); if (usingIteratorProfile.profile(usingIterator != Undefined.instance)) { iteratorRecord = getIteratorNode.execute(arg); + if (!isCallableNode.executeBoolean(iteratorRecord.getNextMethod())) { + errorProfile.enter(); + throw Errors.createTypeErrorCallableExpected(); + } + boolean hasInstance = ordinaryHasInstanceNode.executeBoolean(iteratorRecord.getIterator(), getRealm().getIteratorConstructor()); if (hasInstance) { return iteratorRecord.getIterator();