Skip to content

Commit 1b37ea7

Browse files
GeorgNeisCommit Bot
GeorgNeis
authored and
Commit Bot
committed
[compiler] Remove error-prone GotoIfException
... in favor of CodeAssembler's ScopedExceptionHandler. Also remove unused exception arguments from some iterator related methods. Bug: v8:10187 Change-Id: I8eb7dfd4eb339e4f566970efa5757c3771926ba6 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2060496 Commit-Queue: Georg Neis <neis@chromium.org> Auto-Submit: Georg Neis <neis@chromium.org> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{#66306}
1 parent 5d7f29a commit 1b37ea7

11 files changed

Lines changed: 108 additions & 285 deletions

‎src/builtins/builtins-async-iterator-gen.cc‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
namespace v8 {
1414
namespace internal {
1515

16-
using compiler::Node;
17-
1816
namespace {
1917
class AsyncFromSyncBuiltinsAssembler : public AsyncBuiltinsAssembler {
2018
public:
@@ -130,9 +128,12 @@ void AsyncFromSyncBuiltinsAssembler::Generate_AsyncFromSyncIteratorMethod(
130128
BIND(&if_isnotundefined);
131129
}
132130

133-
const TNode<Object> iter_result = CallJS(
134-
CodeFactory::Call(isolate()), context, method, sync_iterator, sent_value);
135-
GotoIfException(iter_result, &reject_promise, &var_exception);
131+
TNode<Object> iter_result;
132+
{
133+
ScopedExceptionHandler handler(this, &reject_promise, &var_exception);
134+
iter_result = CallJS(CodeFactory::Call(isolate()), context, method,
135+
sync_iterator, sent_value);
136+
}
136137

137138
TNode<Object> value;
138139
TNode<Oddball> done;
@@ -144,10 +145,13 @@ void AsyncFromSyncBuiltinsAssembler::Generate_AsyncFromSyncIteratorMethod(
144145
CSA_ASSERT(this, IsConstructor(promise_fun));
145146

146147
// Let valueWrapper be PromiseResolve(%Promise%, « value »).
147-
const TNode<Object> value_wrapper = CallBuiltin(
148-
Builtins::kPromiseResolve, native_context, promise_fun, value);
149148
// IfAbruptRejectPromise(valueWrapper, promiseCapability).
150-
GotoIfException(value_wrapper, &reject_promise, &var_exception);
149+
TNode<Object> value_wrapper;
150+
{
151+
ScopedExceptionHandler handler(this, &reject_promise, &var_exception);
152+
value_wrapper = CallBuiltin(Builtins::kPromiseResolve, native_context,
153+
promise_fun, value);
154+
}
151155

152156
// Let onFulfilled be a new built-in function object as defined in
153157
// Async Iterator Value Unwrap Functions.
@@ -200,17 +204,17 @@ AsyncFromSyncBuiltinsAssembler::LoadIteratorResult(
200204

201205
BIND(&if_slowpath);
202206
{
207+
ScopedExceptionHandler handler(this, if_exception, var_exception);
208+
203209
// Let nextDone be IteratorComplete(nextResult).
204210
// IfAbruptRejectPromise(nextDone, promiseCapability).
205211
const TNode<Object> done =
206212
GetProperty(context, iter_result, factory()->done_string());
207-
GotoIfException(done, if_exception, var_exception);
208213

209214
// Let nextValue be IteratorValue(nextResult).
210215
// IfAbruptRejectPromise(nextValue, promiseCapability).
211216
const TNode<Object> value =
212217
GetProperty(context, iter_result, factory()->value_string());
213-
GotoIfException(value, if_exception, var_exception);
214218

215219
var_value = value;
216220
var_done = done;

‎src/builtins/builtins-collections-gen.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,7 @@ void BaseCollectionsAssembler::AddConstructorEntry(
150150
TNode<Object> add_function, TNode<Object> key_value,
151151
Label* if_may_have_side_effects, Label* if_exception,
152152
TVariable<Object>* var_exception) {
153-
compiler::CodeAssemblerScopedExceptionHandler handler(this, if_exception,
154-
var_exception);
153+
compiler::ScopedExceptionHandler handler(this, if_exception, var_exception);
155154
CSA_ASSERT(this, Word32BinaryNot(IsTheHole(key_value)));
156155
if (variant == kMap || variant == kWeakMap) {
157156
TorqueStructKeyValuePair pair =

‎src/builtins/builtins-generator-gen.cc‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,16 @@ void GeneratorBuiltinsAssembler::InnerResume(
5656
SmiConstant(resume_mode));
5757

5858
// Resume the {receiver} using our trampoline.
59+
// Close the generator if there was an exception.
5960
TVARIABLE(Object, var_exception);
6061
Label if_exception(this, Label::kDeferred), if_final_return(this);
61-
TNode<Object> result = CallStub(CodeFactory::ResumeGenerator(isolate()),
62-
context, value, receiver);
63-
// Make sure we close the generator if there was an exception.
64-
GotoIfException(result, &if_exception, &var_exception);
62+
TNode<Object> result;
63+
{
64+
compiler::ScopedExceptionHandler handler(this, &if_exception,
65+
&var_exception);
66+
result = CallStub(CodeFactory::ResumeGenerator(isolate()), context, value,
67+
receiver);
68+
}
6569

6670
// If the generator is not suspended (i.e., its state is 'executing'),
6771
// close it and wrap the return value in IteratorResult.

‎src/builtins/builtins-iterator-gen.cc‎

Lines changed: 37 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -23,68 +23,52 @@ TNode<Object> IteratorBuiltinsAssembler::GetIteratorMethod(
2323
return GetProperty(context, object, factory()->iterator_symbol());
2424
}
2525

26-
IteratorRecord IteratorBuiltinsAssembler::GetIterator(
27-
TNode<Context> context, TNode<Object> object, Label* if_exception,
28-
TVariable<Object>* exception) {
26+
IteratorRecord IteratorBuiltinsAssembler::GetIterator(TNode<Context> context,
27+
TNode<Object> object) {
2928
TNode<Object> method = GetIteratorMethod(context, object);
30-
return GetIterator(context, object, method, if_exception, exception);
29+
return GetIterator(context, object, method);
3130
}
3231

33-
IteratorRecord IteratorBuiltinsAssembler::GetIterator(
34-
TNode<Context> context, TNode<Object> object, TNode<Object> method,
35-
Label* if_exception, TVariable<Object>* exception) {
36-
GotoIfException(method, if_exception, exception);
37-
32+
IteratorRecord IteratorBuiltinsAssembler::GetIterator(TNode<Context> context,
33+
TNode<Object> object,
34+
TNode<Object> method) {
3835
Label if_not_callable(this, Label::kDeferred), if_callable(this);
3936
GotoIf(TaggedIsSmi(method), &if_not_callable);
4037
Branch(IsCallable(CAST(method)), &if_callable, &if_not_callable);
4138

4239
BIND(&if_not_callable);
43-
{
44-
TNode<Object> ret =
45-
CallRuntime(Runtime::kThrowIteratorError, context, object);
46-
GotoIfException(ret, if_exception, exception);
47-
Unreachable();
48-
}
40+
CallRuntime(Runtime::kThrowIteratorError, context, object);
41+
Unreachable();
4942

5043
BIND(&if_callable);
5144
{
52-
Callable callable = CodeFactory::Call(isolate());
53-
TNode<Object> iterator = CallJS(callable, context, method, object);
54-
GotoIfException(iterator, if_exception, exception);
45+
TNode<Object> iterator =
46+
CallJS(CodeFactory::Call(isolate()), context, method, object);
5547

5648
Label get_next(this), if_notobject(this, Label::kDeferred);
5749
GotoIf(TaggedIsSmi(iterator), &if_notobject);
5850
Branch(IsJSReceiver(CAST(iterator)), &get_next, &if_notobject);
5951

6052
BIND(&if_notobject);
61-
{
62-
TNode<Object> ret =
63-
CallRuntime(Runtime::kThrowSymbolIteratorInvalid, context);
64-
GotoIfException(ret, if_exception, exception);
65-
Unreachable();
66-
}
53+
CallRuntime(Runtime::kThrowSymbolIteratorInvalid, context);
54+
Unreachable();
6755

6856
BIND(&get_next);
69-
const TNode<Object> next =
57+
TNode<Object> next =
7058
GetProperty(context, iterator, factory()->next_string());
71-
GotoIfException(next, if_exception, exception);
72-
7359
return IteratorRecord{TNode<JSReceiver>::UncheckedCast(iterator),
7460
TNode<Object>::UncheckedCast(next)};
7561
}
7662
}
7763

7864
TNode<JSReceiver> IteratorBuiltinsAssembler::IteratorStep(
7965
TNode<Context> context, const IteratorRecord& iterator, Label* if_done,
80-
base::Optional<TNode<Map>> fast_iterator_result_map, Label* if_exception,
81-
TVariable<Object>* exception) {
66+
base::Optional<TNode<Map>> fast_iterator_result_map) {
8267
DCHECK_NOT_NULL(if_done);
8368
// 1. a. Let result be ? Invoke(iterator, "next", « »).
8469
Callable callable = CodeFactory::Call(isolate());
8570
TNode<Object> result =
8671
CallJS(callable, context, iterator.next, iterator.object);
87-
GotoIfException(result, if_exception, exception);
8872

8973
// 3. If Type(result) is not Object, throw a TypeError exception.
9074
Label if_notobject(this, Label::kDeferred), return_result(this);
@@ -117,26 +101,20 @@ TNode<JSReceiver> IteratorBuiltinsAssembler::IteratorStep(
117101
// 2. Return ToBoolean(? Get(iterResult, "done")).
118102
TNode<Object> done =
119103
GetProperty(context, heap_object_result, factory()->done_string());
120-
GotoIfException(done, if_exception, exception);
121104
BranchIfToBooleanIsTrue(done, if_done, &return_result);
122105
}
123106

124107
BIND(&if_notobject);
125-
{
126-
TNode<Object> ret =
127-
CallRuntime(Runtime::kThrowIteratorResultNotAnObject, context, result);
128-
GotoIfException(ret, if_exception, exception);
129-
Unreachable();
130-
}
108+
CallRuntime(Runtime::kThrowIteratorResultNotAnObject, context, result);
109+
Unreachable();
131110

132111
BIND(&return_result);
133112
return CAST(heap_object_result);
134113
}
135114

136115
TNode<Object> IteratorBuiltinsAssembler::IteratorValue(
137116
TNode<Context> context, TNode<JSReceiver> result,
138-
base::Optional<TNode<Map>> fast_iterator_result_map, Label* if_exception,
139-
TVariable<Object>* exception) {
117+
base::Optional<TNode<Map>> fast_iterator_result_map) {
140118
Label exit(this);
141119
TVARIABLE(Object, var_value);
142120
if (fast_iterator_result_map) {
@@ -151,13 +129,8 @@ TNode<Object> IteratorBuiltinsAssembler::IteratorValue(
151129
}
152130

153131
// Generic iterator result case:
154-
{
155-
TNode<Object> value =
156-
GetProperty(context, result, factory()->value_string());
157-
GotoIfException(value, if_exception, exception);
158-
var_value = value;
159-
Goto(&exit);
160-
}
132+
var_value = GetProperty(context, result, factory()->value_string());
133+
Goto(&exit);
161134

162135
BIND(&exit);
163136
return var_value.value();
@@ -174,23 +147,24 @@ void IteratorBuiltinsAssembler::IteratorCloseOnException(
174147
CSA_ASSERT(this, IsJSReceiver(iterator.object));
175148

176149
// Let return be ? GetMethod(iterator, "return").
177-
TNode<Object> method =
178-
GetProperty(context, iterator.object, factory()->return_string());
179-
GotoIfException(method, if_exception, exception);
150+
TNode<Object> method;
151+
{
152+
compiler::ScopedExceptionHandler handler(this, if_exception, exception);
153+
method = GetProperty(context, iterator.object, factory()->return_string());
154+
}
180155

181156
// If return is undefined, return Completion(completion).
182157
GotoIf(Word32Or(IsUndefined(method), IsNull(method)), if_exception);
183158

184159
{
185160
// Let innerResult be Call(return, iterator, « »).
186-
// If an exception occurs, the original exception remains bound
187-
TNode<Object> inner_result =
188-
CallJS(CodeFactory::Call(isolate()), context, method, iterator.object);
189-
GotoIfException(inner_result, if_exception, nullptr);
190-
191-
// (If completion.[[Type]] is throw) return Completion(completion).
192-
Goto(if_exception);
161+
// If an exception occurs, the original exception remains bound.
162+
compiler::ScopedExceptionHandler handler(this, if_exception, nullptr);
163+
CallJS(CodeFactory::Call(isolate()), context, method, iterator.object);
193164
}
165+
166+
// (If completion.[[Type]] is throw) return Completion(completion).
167+
Goto(if_exception);
194168
}
195169

196170
void IteratorBuiltinsAssembler::IteratorCloseOnException(
@@ -317,10 +291,13 @@ TNode<JSArray> IteratorBuiltinsAssembler::StringListFromIterable(
317291
{
318292
// 1. Let error be ThrowCompletion(a newly created TypeError object).
319293
TVARIABLE(Object, var_exception);
320-
TNode<Object> ret = CallRuntime(
321-
Runtime::kThrowTypeError, context,
322-
SmiConstant(MessageTemplate::kIterableYieldedNonString), next_value);
323-
GotoIfException(ret, &if_exception, &var_exception);
294+
{
295+
compiler::ScopedExceptionHandler handler(this, &if_exception,
296+
&var_exception);
297+
CallRuntime(Runtime::kThrowTypeError, context,
298+
SmiConstant(MessageTemplate::kIterableYieldedNonString),
299+
next_value);
300+
}
324301
Unreachable();
325302

326303
// 2. Return ? IteratorClose(iteratorRecord, error).

‎src/builtins/builtins-iterator-gen.h‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,9 @@ class IteratorBuiltinsAssembler : public CodeStubAssembler {
2424

2525
// https://tc39.github.io/ecma262/#sec-getiterator --- never used for
2626
// @@asyncIterator.
27+
IteratorRecord GetIterator(TNode<Context> context, TNode<Object> object);
2728
IteratorRecord GetIterator(TNode<Context> context, TNode<Object> object,
28-
Label* if_exception = nullptr,
29-
TVariable<Object>* exception = nullptr);
30-
IteratorRecord GetIterator(TNode<Context> context, TNode<Object> object,
31-
TNode<Object> method,
32-
Label* if_exception = nullptr,
33-
TVariable<Object>* exception = nullptr);
29+
TNode<Object> method);
3430

3531
// https://tc39.github.io/ecma262/#sec-iteratorstep
3632
// If the iterator is done, goto {if_done}, otherwise returns an iterator
@@ -39,9 +35,7 @@ class IteratorBuiltinsAssembler : public CodeStubAssembler {
3935
// object, loaded from the native context.
4036
TNode<JSReceiver> IteratorStep(
4137
TNode<Context> context, const IteratorRecord& iterator, Label* if_done,
42-
base::Optional<TNode<Map>> fast_iterator_result_map = base::nullopt,
43-
Label* if_exception = nullptr, TVariable<Object>* exception = nullptr);
44-
38+
base::Optional<TNode<Map>> fast_iterator_result_map = base::nullopt);
4539
TNode<JSReceiver> IteratorStep(
4640
TNode<Context> context, const IteratorRecord& iterator,
4741
base::Optional<TNode<Map>> fast_iterator_result_map, Label* if_done) {
@@ -54,8 +48,7 @@ class IteratorBuiltinsAssembler : public CodeStubAssembler {
5448
// object, loaded from the native context.
5549
TNode<Object> IteratorValue(
5650
TNode<Context> context, TNode<JSReceiver> result,
57-
base::Optional<TNode<Map>> fast_iterator_result_map = base::nullopt,
58-
Label* if_exception = nullptr, TVariable<Object>* exception = nullptr);
51+
base::Optional<TNode<Map>> fast_iterator_result_map = base::nullopt);
5952

6053
// https://tc39.github.io/ecma262/#sec-iteratorclose
6154
void IteratorCloseOnException(TNode<Context> context,

0 commit comments

Comments
 (0)