[kernel] Don't use kernel ast nodes as keys

runtime/vm/kernel_to_il.cc

Index: runtime/vm/kernel_to_il.cc
diff --git a/runtime/vm/kernel_to_il.cc b/runtime/vm/kernel_to_il.cc
index 081c7a128e1940b894a3fe0a08248fec49c51a60..7924465818f5ae9905422c9b8a64417af81c4bee 100644
--- a/runtime/vm/kernel_to_il.cc
+++ b/runtime/vm/kernel_to_il.cc
@@ -58,10 +58,12 @@ static void DiscoverEnclosingElements(Zone* zone,
 }
 
 
-void ScopeBuilder::EnterScope(TreeNode* node, TokenPosition start_position) {
+template <typename NewScopeType>
+void ScopeBuilder::EnterScope(NewScopeType* node,

[Kevin Millikin (Google), 2017/03/28 06:42:23]

Instead of the template functions I'd rather add offset to every Kernel
TreeNode, set it to a default of -1 or some other invalid value, and assert that
it's valid here.

[jensj, 2017/03/28 08:56:28]

Done.

+                              TokenPosition start_position) {
   scope_ = new (Z) LocalScope(scope_, depth_.function_, depth_.loop_);
   scope_->set_begin_token_pos(start_position);
-  result_->scopes.Insert(node, scope_);
+  result_->scopes.Insert(node->kernel_file_offset(), scope_);
 }
 
 
@@ -100,7 +102,7 @@ void ScopeBuilder::AddParameter(VariableDeclaration* declaration,
     variable->set_is_final();
   }
   scope_->InsertParameterAt(pos, variable);
-  result_->locals.Insert(declaration, variable);
+  result_->locals.Insert(declaration->kernel_file_offset(), variable);
 
   // The default value may contain 'let' bindings for which the constant
   // evaluator needs scope bindings.
@@ -185,7 +187,8 @@ void ScopeBuilder::AddIteratorVariable() {
 
 
 void ScopeBuilder::LookupVariable(VariableDeclaration* declaration) {
-  LocalVariable* variable = result_->locals.Lookup(declaration);
+  LocalVariable* variable =
+      result_->locals.Lookup(declaration->kernel_file_offset());
   if (variable == NULL) {
     // We have not seen a declaration of the variable, so it must be the
     // case that we are compiling a nested function and the variable is
@@ -195,7 +198,7 @@ void ScopeBuilder::LookupVariable(VariableDeclaration* declaration) {
     const dart::String& name = H.DartSymbol(declaration->name());
     variable = current_function_scope_->parent()->LookupVariable(name, true);
     ASSERT(variable != NULL);
-    result_->locals.Insert(declaration, variable);
+    result_->locals.Insert(declaration->kernel_file_offset(), variable);
   }
   if (variable->owner()->function_level() < scope_->function_level()) {
     // We call `LocalScope->CaptureVariable(variable)` in two scenarios for two
@@ -249,7 +252,7 @@ void ScopeBuilder::AddVariable(VariableDeclaration* declaration) {
     variable->set_is_final();
   }
   scope_->AddVariable(variable);
-  result_->locals.Insert(declaration, variable);
+  result_->locals.Insert(declaration->kernel_file_offset(), variable);
 }
 
 
@@ -482,7 +485,8 @@ void ScopeBuilder::VisitVariableSet(VariableSet* node) {
 }
 
 
-void ScopeBuilder::HandleLocalFunction(TreeNode* parent,
+template <typename FunctionType>
+void ScopeBuilder::HandleLocalFunction(FunctionType* parent,
                                        FunctionNode* function) {
   LocalScope* saved_function_scope = current_function_scope_;
   FunctionNode* saved_function_node = current_function_node_;
@@ -492,7 +496,7 @@ void ScopeBuilder::HandleLocalFunction(TreeNode* parent,
   current_function_scope_ = scope_;
   current_function_node_ = function;
   if (depth_.function_ == 1) {
-    FunctionScope function_scope = {function, scope_};
+    FunctionScope function_scope = {function->kernel_file_offset(), scope_};
     result_->function_scopes.Add(function_scope);
   }
   AddParameters(function);
@@ -2010,10 +2014,11 @@ Fragment FlowGraphBuilder::TranslateFinallyFinalizers(
 }
 
 
-Fragment FlowGraphBuilder::EnterScope(TreeNode* node, bool* new_context) {
+Fragment FlowGraphBuilder::EnterScope(int64_t kernel_offset,

[Kevin Millikin (Google), 2017/03/28 06:42:22]

It's a little confusing that this is sometimes 'kernel_offset' and sometimes
'kernel_file_offset'.  I suggest something uniformly 'kernel_offset'.

Anyway, can't you just pass the node here and use node->kernel_offset() in this
function, instead of having to have ->kernel_offset() at every call site for
EnterScope and ExitScope?

[jensj, 2017/03/28 08:56:28]

Done.

+                                      bool* new_context) {
   Fragment instructions;
   const intptr_t context_size =
-      scopes_->scopes.Lookup(node)->num_context_variables();
+      scopes_->scopes.Lookup(kernel_offset)->num_context_variables();
   if (context_size > 0) {
     instructions += PushContext(context_size);
     instructions += Drop();
@@ -2025,10 +2030,10 @@ Fragment FlowGraphBuilder::EnterScope(TreeNode* node, bool* new_context) {
 }
 
 
-Fragment FlowGraphBuilder::ExitScope(TreeNode* node) {
+Fragment FlowGraphBuilder::ExitScope(int64_t kernel_offset) {
   Fragment instructions;
   const intptr_t context_size =
-      scopes_->scopes.Lookup(node)->num_context_variables();
+      scopes_->scopes.Lookup(kernel_offset)->num_context_variables();
   if (context_size > 0) {
     instructions += PopContext();
   }
@@ -2881,7 +2886,7 @@ intptr_t FlowGraphBuilder::CurrentTryIndex() {
 
 dart::LocalVariable* FlowGraphBuilder::LookupVariable(
     VariableDeclaration* var) {
-  LocalVariable* local = scopes_->locals.Lookup(var);
+  LocalVariable* local = scopes_->locals.Lookup(var->kernel_file_offset());
   ASSERT(local != NULL);
   return local;
 }
@@ -4086,11 +4091,11 @@ Fragment FlowGraphBuilder::TranslateInitializers(
       dart::Field& field =
           dart::Field::ZoneHandle(Z, H.LookupFieldByKernelField(kernel_field));
 
-      EnterScope(kernel_field);
+      EnterScope(kernel_field->kernel_file_offset());
       instructions += LoadLocal(scopes_->this_variable);
       instructions += TranslateExpression(init);
       instructions += StoreInstanceFieldGuarded(field, true);
-      ExitScope(kernel_field);
+      ExitScope(kernel_field->kernel_file_offset());
     }
   }
 
@@ -5375,13 +5380,13 @@ void FlowGraphBuilder::VisitEmptyStatement(EmptyStatement* node) {
 void FlowGraphBuilder::VisitBlock(Block* node) {
   Fragment instructions;
 
-  instructions += EnterScope(node);
+  instructions += EnterScope(node->kernel_file_offset());
   List<Statement>& statements = node->statements();
   for (intptr_t i = 0; (i < statements.length()) && instructions.is_open();
        ++i) {
     instructions += TranslateStatement(statements[i]);
   }
-  instructions += ExitScope(node);
+  instructions += ExitScope(node->kernel_file_offset());
 
   fragment_ = instructions;
 }
@@ -5558,7 +5563,7 @@ void FlowGraphBuilder::VisitForStatement(ForStatement* node) {
   Fragment declarations;
 
   bool new_context = false;
-  declarations += EnterScope(node, &new_context);
+  declarations += EnterScope(node->kernel_file_offset(), &new_context);
 
   List<VariableDeclaration>& variables = node->variables();
   for (intptr_t i = 0; i < variables.length(); ++i) {
@@ -5604,7 +5609,7 @@ void FlowGraphBuilder::VisitForStatement(ForStatement* node) {
   Fragment loop(declarations.entry, loop_exit);
   --loop_depth_;
 
-  loop += ExitScope(node);
+  loop += ExitScope(node->kernel_file_offset());
 
   fragment_ = loop;
 }
@@ -5633,7 +5638,7 @@ void FlowGraphBuilder::VisitForInStatement(ForInStatement* node) {
   condition += BranchIfTrue(&body_entry, &loop_exit);
 
   Fragment body(body_entry);
-  body += EnterScope(node);
+  body += EnterScope(node->kernel_file_offset());
   body += LoadLocal(iterator);
   body += PushArgument();
   const dart::String& current_getter = dart::String::ZoneHandle(
@@ -5643,7 +5648,7 @@ void FlowGraphBuilder::VisitForInStatement(ForInStatement* node) {
       StoreLocal(TokenPosition::kNoSource, LookupVariable(node->variable()));
   body += Drop();
   body += TranslateStatement(node->body());
-  body += ExitScope(node);
+  body += ExitScope(node->kernel_file_offset());
 
   if (body.is_open()) {
     JoinEntryInstr* join = BuildJoinEntry();
@@ -6090,7 +6095,7 @@ void FlowGraphBuilder::VisitTryCatch(class TryCatch* node) {
 
     Fragment catch_handler_body;
 
-    catch_handler_body += EnterScope(catch_clause);
+    catch_handler_body += EnterScope(catch_clause->kernel_file_offset());
 
     if (catch_clause->exception() != NULL) {
       catch_handler_body += LoadLocal(CurrentException());
@@ -6122,7 +6127,7 @@ void FlowGraphBuilder::VisitTryCatch(class TryCatch* node) {
 
       // Note: ExitScope adjusts context_depth_ so even if catch_handler_body
       // is closed we still need to execute ExitScope for its side effect.
-      catch_handler_body += ExitScope(catch_clause);
+      catch_handler_body += ExitScope(catch_clause->kernel_file_offset());
       if (catch_handler_body.is_open()) {
         catch_handler_body += Goto(after_try);
       }
@@ -6256,7 +6261,10 @@ Fragment FlowGraphBuilder::TranslateFunctionNode(FunctionNode* node,
   // function and token position.
   Function& function = Function::ZoneHandle(Z);
   for (intptr_t i = 0; i < scopes_->function_scopes.length(); ++i) {
-    if (scopes_->function_scopes[i].function != node) continue;
+    if (scopes_->function_scopes[i].kernelFileOffset !=
+        node->kernel_file_offset()) {
+      continue;
+    }
 
     TokenPosition position = node->position();
     if (parent->IsFunctionDeclaration()) {