Skip to content

Commit efee57c

Browse files
alexmarkovCommit Queue
authored andcommitted
[vm] Replace unreachable 'throw' code with Never/Stop
Change core library '_throwNew' helpers to return Never. Since https://dart-review.googlesource.com/c/sdk/+/405604 flow graph builder automatically appends Stop and closes fragment after appending a call with return type Never. So additional 'throw' after calling '_throwNew' are no longer needed and can be removed. Also, cleanup unused FlowGraphBuilder::ThrowTypeError() and replace 'throw' with shorter Stop at the end of 'case'. TEST=ci Change-Id: I0a5109fd693c0463c53e70e5dd261f8080bf1132 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406080 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent ea62736 commit efee57c

File tree

6 files changed

+26
-79
lines changed

6 files changed

+26
-79
lines changed

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionBody(
679679
} else if (dart_function.is_external()) {
680680
body += ThrowNoSuchMethodError(TokenPosition::kNoSource, dart_function,
681681
/*incompatible_arguments=*/false);
682-
body += ThrowException(TokenPosition::kNoSource); // Close graph.
682+
ASSERT(body.is_closed());
683683
} else if (has_body) {
684684
body += BuildStatement();
685685
}
@@ -1699,10 +1699,6 @@ Fragment StreamingFlowGraphBuilder::StringInterpolateSingle(
16991699
return flow_graph_builder_->StringInterpolateSingle(position);
17001700
}
17011701

1702-
Fragment StreamingFlowGraphBuilder::ThrowTypeError() {
1703-
return flow_graph_builder_->ThrowTypeError();
1704-
}
1705-
17061702
Fragment StreamingFlowGraphBuilder::LoadInstantiatorTypeArguments() {
17071703
return flow_graph_builder_->LoadInstantiatorTypeArguments();
17081704
}
@@ -2113,7 +2109,7 @@ Fragment StreamingFlowGraphBuilder::BuildVariableGetImpl(
21132109
already_assigned += flow_graph_builder_->ThrowLateInitializationError(
21142110
position, "_throwLocalAssignedDuringInitialization",
21152111
variable->name());
2116-
already_assigned += Goto(join);
2112+
ASSERT(already_assigned.is_closed());
21172113
}
21182114
} else {
21192115
// Late non-final variable. Store the initializer result.
@@ -2126,7 +2122,7 @@ Fragment StreamingFlowGraphBuilder::BuildVariableGetImpl(
21262122
Fragment initialize(is_uninitialized);
21272123
initialize += flow_graph_builder_->ThrowLateInitializationError(
21282124
position, "_throwLocalNotInitialized", variable->name());
2129-
initialize += Goto(join);
2125+
ASSERT(initialize.is_closed());
21302126
}
21312127
}
21322128

@@ -2193,7 +2189,7 @@ Fragment StreamingFlowGraphBuilder::BuildVariableSetImpl(
21932189
Fragment already_initialized(is_initialized);
21942190
already_initialized += flow_graph_builder_->ThrowLateInitializationError(
21952191
position, "_throwLocalAlreadyInitialized", variable->name());
2196-
already_initialized += Goto(join);
2192+
ASSERT(already_initialized.is_closed());
21972193
}
21982194

21992195
instructions = Fragment(instructions.entry, join);
@@ -4641,11 +4637,6 @@ Fragment StreamingFlowGraphBuilder::BuildAssertStatement(
46414637
Class::ZoneHandle(Z, Library::LookupCoreClass(Symbols::AssertionError()));
46424638
ASSERT(!klass.IsNull());
46434639

4644-
// Build equivalent of `throw _AssertionError._throwNew(start, end, message)`
4645-
// expression. We build throw (even through _throwNew already throws) because
4646-
// call is not a valid last instruction for the block. Blocks can only
4647-
// terminate with explicit control flow instructions (Branch, Goto, Return
4648-
// or Throw).
46494640
Fragment otherwise_fragment(otherwise);
46504641
if (CompilerState::Current().is_aot()) {
46514642
// When in AOT, figure out start line, end line, line fragment needed for
@@ -4701,8 +4692,8 @@ Fragment StreamingFlowGraphBuilder::BuildAssertStatement(
47014692
otherwise_fragment +=
47024693
StaticCall(condition_start_offset, target, 3, ICData::kStatic);
47034694
}
4704-
otherwise_fragment += ThrowException(TokenPosition::kNoSource);
47054695
otherwise_fragment += Drop();
4696+
ASSERT(otherwise_fragment.is_closed());
47064697

47074698
return Fragment(instructions.entry, then);
47084699
}
@@ -4982,16 +4973,9 @@ Fragment StreamingFlowGraphBuilder::BuildSwitchCase(SwitchHelper* helper,
49824973
body_fragment += Drop();
49834974
}
49844975

4985-
// TODO(http://dartbug.com/50595): The CFE does not insert breaks for
4986-
// unterminated cases which never reach the end of their control flow.
4987-
// If the CFE inserts synthesized breaks, we can add an assert here instead.
49884976
if (!is_default && body_fragment.is_open() &&
49894977
(case_index < (helper->case_count() - 1))) {
4990-
const auto& error =
4991-
String::ZoneHandle(Z, Symbols::New(thread(), "Unreachable code."));
4992-
body_fragment += Constant(error);
4993-
body_fragment += ThrowException(TokenPosition::kNoSource);
4994-
body_fragment += Drop();
4978+
body_fragment += B->Stop("Unreachable end of case");
49954979
}
49964980

49974981
// If there is an implicit fall-through we have one [SwitchCase] and

runtime/vm/compiler/frontend/kernel_binary_flowgraph.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
226226
Fragment StoreStaticField(TokenPosition position, const Field& field);
227227
Fragment StringInterpolate(TokenPosition position);
228228
Fragment StringInterpolateSingle(TokenPosition position);
229-
Fragment ThrowTypeError();
230229
Fragment LoadInstantiatorTypeArguments();
231230
Fragment LoadFunctionTypeArguments();
232231
Fragment InstantiateType(const AbstractType& type);

runtime/vm/compiler/frontend/kernel_to_il.cc

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,7 @@ Fragment FlowGraphBuilder::ThrowLateInitializationError(
528528
StaticCall(TokenPosition::Synthetic(position.Pos()), throw_new,
529529
/* argument_count = */ 1, ICData::kStatic);
530530
instructions += Drop();
531+
ASSERT(instructions.is_closed());
531532

532533
return instructions;
533534
}
@@ -566,7 +567,7 @@ Fragment FlowGraphBuilder::StoreLateField(const Field& field,
566567
already_initialized += ThrowLateInitializationError(
567568
position, "_throwFieldAlreadyInitialized",
568569
String::ZoneHandle(Z, field.name()));
569-
already_initialized += Goto(join);
570+
ASSERT(already_initialized.is_closed());
570571
}
571572

572573
instructions = Fragment(instructions.entry, join);
@@ -711,47 +712,6 @@ Fragment FlowGraphBuilder::StringInterpolate(TokenPosition position) {
711712
return instructions;
712713
}
713714

714-
Fragment FlowGraphBuilder::ThrowTypeError() {
715-
const Class& klass =
716-
Class::ZoneHandle(Z, Library::LookupCoreClass(Symbols::TypeError()));
717-
ASSERT(!klass.IsNull());
718-
GrowableHandlePtrArray<const String> pieces(Z, 3);
719-
pieces.Add(Symbols::TypeError());
720-
pieces.Add(Symbols::Dot());
721-
pieces.Add(H.DartSymbolObfuscate("_create"));
722-
723-
const Function& constructor = Function::ZoneHandle(
724-
Z, klass.LookupConstructorAllowPrivate(
725-
String::ZoneHandle(Z, Symbols::FromConcatAll(thread_, pieces))));
726-
ASSERT(!constructor.IsNull());
727-
728-
const String& url = H.DartString(
729-
parsed_function_->function().ToLibNamePrefixedQualifiedCString(),
730-
Heap::kOld);
731-
732-
Fragment instructions;
733-
734-
// Create instance of _TypeError
735-
instructions += AllocateObject(TokenPosition::kNoSource, klass, 0);
736-
LocalVariable* instance = MakeTemporary();
737-
738-
// Call _TypeError._create constructor.
739-
instructions += LoadLocal(instance); // this
740-
instructions += Constant(url); // url
741-
instructions += NullConstant(); // line
742-
instructions += IntConstant(0); // column
743-
instructions += Constant(H.DartSymbolPlain("Malformed type.")); // message
744-
745-
instructions += StaticCall(TokenPosition::kNoSource, constructor,
746-
/* argument_count = */ 5, ICData::kStatic);
747-
instructions += Drop();
748-
749-
// Throw the exception
750-
instructions += ThrowException(TokenPosition::kNoSource);
751-
752-
return instructions;
753-
}
754-
755715
Fragment FlowGraphBuilder::ThrowNoSuchMethodError(TokenPosition position,
756716
const Function& target,
757717
bool incompatible_arguments,
@@ -818,6 +778,7 @@ Fragment FlowGraphBuilder::ThrowNoSuchMethodError(TokenPosition position,
818778
instructions += NullConstant(); // argumentNames
819779
instructions += StaticCall(position, throw_function, /* argument_count = */ 7,
820780
ICData::kNoRebind);
781+
ASSERT(instructions.is_closed());
821782
return instructions;
822783
}
823784

@@ -2859,7 +2820,7 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecordFieldGetter(
28592820
throw_nsm += ThrowNoSuchMethodError(TokenPosition::kNoSource, function,
28602821
/*incompatible_arguments=*/false,
28612822
/*receiver_pushed=*/true);
2862-
throw_nsm += ThrowException(TokenPosition::kNoSource); // Close graph.
2823+
ASSERT(throw_nsm.is_closed());
28632824

28642825
// There is no prologue code for a record field getter.
28652826
PrologueInfo prologue_info(-1, -1);

runtime/vm/compiler/frontend/kernel_to_il.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
259259
intptr_t type_args_len = 0);
260260
Fragment StringInterpolateSingle(TokenPosition position);
261261
Fragment StringInterpolate(TokenPosition position);
262-
Fragment ThrowTypeError();
263262

264263
// [incompatible_arguments] should be true if the NSM is due to a mismatch
265264
// between the provided arguments and the function signature.

sdk/lib/_internal/vm/lib/errors_patch.dart

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,17 @@ class _AssertionError extends Error implements AssertionError {
4141
// out of the script. It expects a Dart stack frame from class
4242
// _AssertionError. Thus we need a Dart stub that calls the native code.
4343
@pragma("vm:entry-point", "call")
44-
static _throwNew(int assertionStart, int assertionEnd, Object? message) {
44+
static Never _throwNew(
45+
int assertionStart,
46+
int assertionEnd,
47+
Object? message,
48+
) {
4549
_doThrowNew(assertionStart, assertionEnd, message);
4650
}
4751

4852
@pragma("vm:entry-point", "call")
4953
@pragma("vm:external-name", "AssertionError_throwNewSource")
50-
external static _throwNewSource(
54+
external static Never _throwNewSource(
5155
String failedAssertion,
5256
String? scriptUrl,
5357
int line,
@@ -56,7 +60,7 @@ class _AssertionError extends Error implements AssertionError {
5660
);
5761

5862
@pragma("vm:external-name", "AssertionError_throwNew")
59-
external static _doThrowNew(
63+
external static Never _doThrowNew(
6064
int assertionStart,
6165
int assertionEnd,
6266
Object? message,
@@ -110,7 +114,7 @@ class _TypeError extends Error implements TypeError {
110114

111115
@pragma("vm:entry-point", "call")
112116
@pragma("vm:external-name", "TypeError_throwNew")
113-
external static _throwNew(
117+
external static Never _throwNew(
114118
int location,
115119
Object srcValue,
116120
_Type dstType,
@@ -136,7 +140,7 @@ class _InternalError {
136140
@patch
137141
@pragma("vm:entry-point")
138142
class UnsupportedError {
139-
static _throwNew(String msg) {
143+
static Never _throwNew(String msg) {
140144
throw new UnsupportedError(msg);
141145
}
142146
}
@@ -145,7 +149,7 @@ class UnsupportedError {
145149
@pragma("vm:entry-point")
146150
class StateError {
147151
@pragma("vm:entry-point")
148-
static _throwNew(String msg) {
152+
static Never _throwNew(String msg) {
149153
throw new StateError(msg);
150154
}
151155
}
@@ -163,15 +167,15 @@ class NoSuchMethodError {
163167

164168
NoSuchMethodError._withInvocation(this._receiver, this._invocation);
165169

166-
static void _throwNewInvocation(Object? receiver, Invocation invocation) {
170+
static Never _throwNewInvocation(Object? receiver, Invocation invocation) {
167171
throw new NoSuchMethodError.withInvocation(receiver, invocation);
168172
}
169173

170174
// The compiler emits a call to _throwNew when it cannot resolve a static
171175
// method at compile time. The receiver is actually the literal class of the
172176
// unresolved method.
173177
@pragma("vm:entry-point", "call")
174-
static void _throwNew(
178+
static Never _throwNew(
175179
Object receiver,
176180
String memberName,
177181
int invocationType,

sdk/lib/_internal/vm/lib/internal_patch.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,22 +205,22 @@ bool isSentinel(dynamic value) => throw UnsupportedError('isSentinel');
205205
@patch
206206
class LateError {
207207
@pragma("vm:entry-point")
208-
static _throwFieldAlreadyInitialized(String fieldName) {
208+
static Never _throwFieldAlreadyInitialized(String fieldName) {
209209
throw new LateError.fieldAI(fieldName);
210210
}
211211

212212
@pragma("vm:entry-point")
213-
static _throwLocalNotInitialized(String localName) {
213+
static Never _throwLocalNotInitialized(String localName) {
214214
throw new LateError.localNI(localName);
215215
}
216216

217217
@pragma("vm:entry-point")
218-
static _throwLocalAlreadyInitialized(String localName) {
218+
static Never _throwLocalAlreadyInitialized(String localName) {
219219
throw new LateError.localAI(localName);
220220
}
221221

222222
@pragma("vm:entry-point")
223-
static _throwLocalAssignedDuringInitialization(String localName) {
223+
static Never _throwLocalAssignedDuringInitialization(String localName) {
224224
throw new LateError.localADI(localName);
225225
}
226226
}

0 commit comments

Comments
 (0)