[VM] Make the encoding of variable descriptors use variable indices
This decouples the scope building from knowing about frame layout.
Issue https://github.com/dart-lang/sdk/issues/33274
Change-Id: I9058b242fb24f859f54d6e3660a5972df86d39e6
Reviewed-on: https://dart-review.googlesource.com/59093
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
diff --git a/runtime/vm/ast_printer_test.cc b/runtime/vm/ast_printer_test.cc
index 22a73ec..7f73a8d 100644
--- a/runtime/vm/ast_printer_test.cc
+++ b/runtime/vm/ast_printer_test.cc
@@ -19,7 +19,7 @@
LocalVariable* v = new LocalVariable(
kPos, kPos, String::ZoneHandle(Symbols::New(thread, "wurscht")),
Type::ZoneHandle(Type::DynamicType()));
- v->set_index(VariableIndex::From(5));
+ v->set_index(VariableIndex(5));
AstPrinter ast_printer;
LoadLocalNode* ll = new LoadLocalNode(kPos, v);
ReturnNode* r = new ReturnNode(kPos, ll);
diff --git a/runtime/vm/ast_test.cc b/runtime/vm/ast_test.cc
index 72fa92d..302c1d9 100644
--- a/runtime/vm/ast_test.cc
+++ b/runtime/vm/ast_test.cc
@@ -22,7 +22,7 @@
EXPECT(!ll->IsLiteralNode());
LoadLocalNode* lln = ll->AsLoadLocalNode();
EXPECT(NULL != lln);
- v->set_index(VariableIndex::From(1));
+ v->set_index(VariableIndex(1));
EXPECT_EQ(1, v->index().value());
LocalVariable* p =
@@ -30,7 +30,7 @@
String::ZoneHandle(Symbols::New(thread, "p")),
Type::ZoneHandle(Type::DynamicType()));
EXPECT(!p->HasIndex());
- p->set_index(VariableIndex::From(-1));
+ p->set_index(VariableIndex(-1));
EXPECT(p->HasIndex());
EXPECT_EQ(-1, p->index().value());
diff --git a/runtime/vm/compiler/frontend/flow_graph_builder.cc b/runtime/vm/compiler/frontend/flow_graph_builder.cc
index 5a9a9e7..d950842 100644
--- a/runtime/vm/compiler/frontend/flow_graph_builder.cc
+++ b/runtime/vm/compiler/frontend/flow_graph_builder.cc
@@ -2117,7 +2117,7 @@
new (Z) LocalVariable(TokenPosition::kNoSource, TokenPosition::kNoSource,
String::ZoneHandle(Z, Symbols::New(T, name)),
*value->Type()->ToAbstractType());
- var->set_index(VariableIndex::From(index));
+ var->set_index(VariableIndex(index));
return var;
}
@@ -2133,7 +2133,7 @@
Append(for_value);
ASSERT(!node->TempAt(i)->HasIndex() ||
(node->TempAt(i)->index().value() == GetCurrentTempLocalIndex()));
- node->TempAt(i)->set_index(VariableIndex::From(GetCurrentTempLocalIndex()));
+ node->TempAt(i)->set_index(VariableIndex(GetCurrentTempLocalIndex()));
}
}
diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
index bc2cde0..23c595e 100644
--- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
+++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
@@ -1444,7 +1444,7 @@
result_->this_variable =
MakeVariable(TokenPosition::kNoSource, TokenPosition::kNoSource,
Symbols::This(), klass_type);
- result_->this_variable->set_index(VariableIndex::From(0));
+ result_->this_variable->set_index(VariableIndex(0));
result_->this_variable->set_is_captured();
enclosing_scope = new (Z) LocalScope(NULL, 0, 0);
enclosing_scope->set_context_level(0);
diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc
index 8d96948..4d9b1c6 100644
--- a/runtime/vm/compiler/frontend/kernel_to_il.cc
+++ b/runtime/vm/compiler/frontend/kernel_to_il.cc
@@ -1970,8 +1970,8 @@
symbol_name, Object::dynamic_type());
// Set the index relative to the base of the expression stack including
// outgoing arguments.
- variable->set_index(VariableIndex::From(
- -parsed_function_->num_stack_locals() - pending_argument_count_ - index));
+ variable->set_index(VariableIndex(-parsed_function_->num_stack_locals() -
+ pending_argument_count_ - index));
// The value has uses as if it were a local variable. Mark the definition
// as used so that its temp index will not be cleared (causing it to never
diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc
index c48a51f..e922ec77 100644
--- a/runtime/vm/debugger.cc
+++ b/runtime/vm/debugger.cc
@@ -741,15 +741,16 @@
if (!live_frame_) {
ASSERT(kind == RawLocalVarDescriptors::kContextVar);
}
+ const auto variable_index = VariableIndex(var_info.index());
if (kind == RawLocalVarDescriptors::kStackVar) {
- return GetStackVar(var_info.index());
+ return GetStackVar(variable_index);
} else {
ASSERT(kind == RawLocalVarDescriptors::kContextVar);
if (!live_frame_) {
ASSERT(!ctx_.IsNull());
- return ctx_.At(var_info.index());
+ return ctx_.At(variable_index.value());
}
- return GetContextVar(var_info.scope_id, var_info.index());
+ return GetContextVar(var_info.scope_id, variable_index.value());
}
}
}
@@ -968,7 +969,8 @@
OS::PrintErr("\tFound saved current ctx at index %d\n",
var_info.index());
}
- obj = GetStackVar(var_info.index());
+ const auto variable_index = VariableIndex(var_info.index());
+ obj = GetStackVar(variable_index);
if (obj.IsClosure()) {
ASSERT(function().name() == Symbols::Call().raw());
ASSERT(function().IsInvokeFieldDispatcher());
@@ -993,11 +995,12 @@
var_descriptors_.GetInfo(i, &var_info);
if (var_descriptors_.GetName(i) == Symbols::AsyncOperation().raw()) {
const int8_t kind = var_info.kind();
+ const auto variable_index = VariableIndex(var_info.index());
if (kind == RawLocalVarDescriptors::kStackVar) {
- return GetStackVar(var_info.index());
+ return GetStackVar(variable_index);
} else {
ASSERT(kind == RawLocalVarDescriptors::kContextVar);
- return GetContextVar(var_info.scope_id, var_info.index());
+ return GetContextVar(var_info.scope_id, variable_index.value());
}
}
}
@@ -1108,8 +1111,9 @@
// can be in a number of places in the caller's frame depending on how many
// were actually supplied at the call site, but they are copied to a fixed
// place in the callee's frame.
+
return GetVariableValue(
- LocalVarAddress(fp(), (kFirstLocalSlotFromFp - index)));
+ LocalVarAddress(fp(), FrameSlotForVariableIndex(-index)));
} else {
intptr_t reverse_index = num_parameters - index;
return GetVariableValue(ParamAddress(fp(), reverse_index));
@@ -1121,7 +1125,8 @@
return GetParameter(0);
}
-RawObject* ActivationFrame::GetStackVar(intptr_t slot_index) {
+RawObject* ActivationFrame::GetStackVar(VariableIndex variable_index) {
+ const intptr_t slot_index = FrameSlotForVariableIndex(variable_index.value());
if (deopt_frame_.IsNull()) {
return GetVariableValue(LocalVarAddress(fp(), slot_index));
} else {
@@ -1217,11 +1222,12 @@
*visible_end_token_pos = var_info.end_pos;
ASSERT(value != NULL);
const int8_t kind = var_info.kind();
+ const auto variable_index = VariableIndex(var_info.index());
if (kind == RawLocalVarDescriptors::kStackVar) {
- *value = GetStackVar(var_info.index());
+ *value = GetStackVar(variable_index);
} else {
ASSERT(kind == RawLocalVarDescriptors::kContextVar);
- *value = GetContextVar(var_info.scope_id, var_info.index());
+ *value = GetContextVar(var_info.scope_id, variable_index.value());
}
}
diff --git a/runtime/vm/debugger.h b/runtime/vm/debugger.h
index 83d077f..fedfed1 100644
--- a/runtime/vm/debugger.h
+++ b/runtime/vm/debugger.h
@@ -10,6 +10,7 @@
#include "vm/kernel_isolate.h"
#include "vm/object.h"
#include "vm/port.h"
+#include "vm/scopes.h"
#include "vm/service_event.h"
#include "vm/simulator.h"
@@ -392,7 +393,7 @@
}
}
- RawObject* GetStackVar(intptr_t slot_index);
+ RawObject* GetStackVar(VariableIndex var_index);
RawObject* GetContextVar(intptr_t ctxt_level, intptr_t slot_index);
uword pc_;
diff --git a/runtime/vm/object_test.cc b/runtime/vm/object_test.cc
index bc8d793..ed8d624 100644
--- a/runtime/vm/object_test.cc
+++ b/runtime/vm/object_test.cc
@@ -2477,9 +2477,9 @@
EXPECT_EQ(4, local_scope->num_variables()); // ta, a, b, c.
EXPECT_EQ(3, local_scope->NumCapturedVariables()); // ta, a, c.
- const VariableIndex first_parameter_index = VariableIndex::From(0);
+ const VariableIndex first_parameter_index(0);
const int num_parameters = 0;
- const VariableIndex first_local_index = VariableIndex::From(-1);
+ const VariableIndex first_local_index(-1);
bool found_captured_vars = false;
VariableIndex next_index = parent_scope->AllocateVariables(
first_parameter_index, num_parameters, first_local_index, NULL,
diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc
index ba1f60e..80b7358 100644
--- a/runtime/vm/parser.cc
+++ b/runtime/vm/parser.cc
@@ -350,7 +350,7 @@
} else {
raw_parameter->set_index(
- VariableIndex::From(function().NumParameters() - param));
+ VariableIndex(function().NumParameters() - param));
}
}
raw_parameters_.Add(raw_parameter);
@@ -382,24 +382,24 @@
// parameters to copy.
if (num_opt_params == 0) {
parameter_index_start = first_parameter_index_ =
- VariableIndex::From(num_params);
- reamining_local_variables_start = VariableIndex::From(0);
+ VariableIndex(num_params);
+ reamining_local_variables_start = VariableIndex(0);
} else {
- parameter_index_start = first_parameter_index_ = VariableIndex::From(0);
- reamining_local_variables_start = VariableIndex::From(-num_params);
+ parameter_index_start = first_parameter_index_ = VariableIndex(0);
+ reamining_local_variables_start = VariableIndex(-num_params);
}
}
if (function_type_arguments_ != NULL && num_opt_params > 0) {
reamining_local_variables_start =
- VariableIndex::From(reamining_local_variables_start.value() - 1);
+ VariableIndex(reamining_local_variables_start.value() - 1);
}
// Allocate parameters and local variables, either in the local frame or
// in the context(s).
bool found_captured_variables = false;
VariableIndex first_local_index =
- VariableIndex::From(parameter_index_start.value() > 0 ? 0 : -num_params);
+ VariableIndex(parameter_index_start.value() > 0 ? 0 : -num_params);
VariableIndex next_free_index = scope->AllocateVariables(
parameter_index_start, num_params, first_local_index, NULL,
&found_captured_variables);
@@ -426,7 +426,7 @@
ASSERT(num_params == RegExpMacroAssembler::kParamCount);
// Compute start indices to parameters and locals, and the number of
// parameters to copy.
- first_parameter_index_ = VariableIndex::From(num_params);
+ first_parameter_index_ = VariableIndex(num_params);
// Frame indices are relative to the frame pointer and are decreasing.
num_stack_locals_ = num_stack_locals;
diff --git a/runtime/vm/regexp_assembler_ir.cc b/runtime/vm/regexp_assembler_ir.cc
index b6656d1..64753cb 100644
--- a/runtime/vm/regexp_assembler_ir.cc
+++ b/runtime/vm/regexp_assembler_ir.cc
@@ -149,7 +149,7 @@
// All generated functions are expected to have a current-context variable.
// This variable is unused in irregexp functions.
parsed_function_->current_context_var()->set_index(
- VariableIndex::From(GetNextLocalIndex()));
+ VariableIndex(GetNextLocalIndex()));
// Create local variables and parameters.
stack_ = Local(Symbols::stack());
@@ -343,7 +343,7 @@
name, Object::dynamic_type());
intptr_t param_frame_index = kParamCount - index;
- local->set_index(VariableIndex::From(param_frame_index));
+ local->set_index(VariableIndex(param_frame_index));
return local;
}
@@ -352,7 +352,7 @@
LocalVariable* local =
new (Z) LocalVariable(TokenPosition::kNoSource, TokenPosition::kNoSource,
name, Object::dynamic_type());
- local->set_index(VariableIndex::From(GetNextLocalIndex()));
+ local->set_index(VariableIndex(GetNextLocalIndex()));
return local;
}
diff --git a/runtime/vm/scopes.cc b/runtime/vm/scopes.cc
index af7c42a..0af2a71 100644
--- a/runtime/vm/scopes.cc
+++ b/runtime/vm/scopes.cc
@@ -179,7 +179,7 @@
}
}
variable->set_index(
- VariableIndex::From((*context_owner)->num_context_variables_++));
+ VariableIndex((*context_owner)->num_context_variables_++));
}
VariableIndex LocalScope::AllocateVariables(VariableIndex first_parameter_index,
@@ -208,12 +208,12 @@
// A captured parameter has a slot allocated in the frame and one in the
// context, where it gets copied to. The parameter index reflects the
// context allocation index.
- next_index = VariableIndex::From(next_index.value() - 1);
+ next_index = VariableIndex(next_index.value() - 1);
AllocateContextVariable(parameter, &context_owner);
*found_captured_variables = true;
} else {
parameter->set_index(next_index);
- next_index = VariableIndex::From(next_index.value() - 1);
+ next_index = VariableIndex(next_index.value() - 1);
}
}
// No overlapping of parameters and locals.
@@ -227,7 +227,7 @@
*found_captured_variables = true;
} else {
variable->set_index(next_index);
- next_index = VariableIndex::From(next_index.value() - 1);
+ next_index = VariableIndex(next_index.value() - 1);
}
}
pos++;
@@ -237,7 +237,7 @@
LocalScope* child = this->child();
while (child != NULL) {
// Ignored, since no parameters.
- const VariableIndex dummy_parameter_index = VariableIndex::From(0);
+ const VariableIndex dummy_parameter_index(0);
// No parameters in children scopes.
const int num_parameters_in_child = 0;
@@ -377,7 +377,7 @@
desc.info.declaration_pos = TokenPosition::kMinSource;
desc.info.begin_pos = TokenPosition::kMinSource;
desc.info.end_pos = TokenPosition::kMinSource;
- desc.info.set_index(FrameSlotForVariable(var));
+ desc.info.set_index(var->index().value());
vars->Add(desc);
} else if (!IsFilteredIdentifier(var->name())) {
// This is a regular Dart variable, either stack-based or captured.
@@ -388,12 +388,11 @@
ASSERT(var->owner() != NULL);
ASSERT(var->owner()->context_level() >= 0);
desc.info.scope_id = var->owner()->context_level();
- desc.info.set_index(var->index().value());
} else {
desc.info.set_kind(RawLocalVarDescriptors::kStackVar);
desc.info.scope_id = *scope_id;
- desc.info.set_index(FrameSlotForVariable(var));
}
+ desc.info.set_index(var->index().value());
desc.info.declaration_pos = var->declaration_token_pos();
desc.info.begin_pos = var->token_pos();
desc.info.end_pos = var->owner()->end_token_pos();
@@ -629,7 +628,7 @@
AbstractType::ZoneHandle(context_scope.TypeAt(i)));
}
variable->set_is_captured();
- variable->set_index(VariableIndex::From(context_scope.ContextIndexAt(i)));
+ variable->set_index(VariableIndex(context_scope.ContextIndexAt(i)));
if (context_scope.IsFinalAt(i)) {
variable->set_is_final();
}
diff --git a/runtime/vm/scopes.h b/runtime/vm/scopes.h
index d8bd50e..15548dc 100644
--- a/runtime/vm/scopes.h
+++ b/runtime/vm/scopes.h
@@ -57,9 +57,7 @@
public:
static const int kInvalidIndex = std::numeric_limits<int>::min();
- VariableIndex() : value_(kInvalidIndex) {}
-
- static VariableIndex From(int index) { return VariableIndex(index); }
+ explicit VariableIndex(int value = kInvalidIndex) : value_(value) {}
int operator==(const VariableIndex& other) { return value_ == other.value_; }
@@ -68,8 +66,6 @@
int value() const { return value_; }
private:
- explicit VariableIndex(int value) : value_(value) {}
-
int value_;
};
