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

runtime/vm/kernel_to_il.cc

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include <map>
 #include <set>
 #include <string>
 
 #include "vm/kernel_to_il.h"
 
@@ skipping 40 matching lines @@
     } else if ((*outermost_node)->IsConstructor()) {
       parent = Constructor::Cast(*outermost_node)->parent();
     } else if ((*outermost_node)->IsField()) {
       parent = Field::Cast(*outermost_node)->parent();
     }
     if (parent != NULL && parent->IsClass()) *klass = Class::Cast(parent);
   }
 }
 
 
-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_);
 }
 
 
 void ScopeBuilder::ExitScope(TokenPosition end_position) {
   scope_->set_end_token_pos(end_position);
   scope_ = scope_->parent();
 }
 
 
 LocalVariable* ScopeBuilder::MakeVariable(TokenPosition declaration_pos,
@@ skipping 18 matching lines @@
 
 void ScopeBuilder::AddParameter(VariableDeclaration* declaration,
                                 intptr_t pos) {
   LocalVariable* variable = MakeVariable(
       declaration->position(), declaration->position(),
       H.DartSymbol(declaration->name()), T.TranslateVariableType(declaration));
   if (declaration->IsFinal()) {
     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.
   Expression* defaultValue = declaration->initializer();
   if (defaultValue != NULL) {
     defaultValue->AcceptExpressionVisitor(this);
   }
 }
 
 
@@ skipping 64 matching lines @@
   LocalVariable* iterator =
       MakeVariable(TokenPosition::kNoSource, TokenPosition::kNoSource,
                    GenerateName(":iterator", depth_.for_in_ - 1),
                    AbstractType::dynamic_type());
   current_function_scope_->AddVariable(iterator);
   result_->iterator_variables.Add(iterator);
 }
 
 
 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
     // declared in an outer scope.  In that case, look it up in the scope by
     // name and add it to the variable map to simplify later lookup.
     ASSERT(current_function_scope_->parent() != NULL);
     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
     // different reasons:
     //   Scenario 1:
     //       We need to know which variables defined in this function
     //       are closed over by nested closures in order to ensure we will
     //       create a [Context] object of appropriate size and store captured
     //       variables there instead of the stack.
     //   Scenario 2:
@@ skipping 33 matching lines @@
   const dart::String& name = declaration->name()->is_empty()
                                  ? GenerateName(":var", name_index_++)
                                  : H.DartSymbol(declaration->name());
   LocalVariable* variable =
       MakeVariable(declaration->position(), declaration->end_position(), name,
                    T.TranslateVariableType(declaration));
   if (declaration->IsFinal()) {
     variable->set_is_final();
   }
   scope_->AddVariable(variable);
-  result_->locals.Insert(declaration, variable);
+  result_->locals.Insert(declaration->kernel_file_offset(), variable);
 }
 
 
 static bool IsStaticInitializer(const Function& function, Zone* zone) {
   return (function.kind() == RawFunction::kImplicitStaticFinalGetter) &&
          dart::String::Handle(zone, function.name())
              .StartsWith(Symbols::InitPrefix());
 }
 
 
@@ skipping 212 matching lines @@
   LookupVariable(node->variable());
 }
 
 
 void ScopeBuilder::VisitVariableSet(VariableSet* node) {
   LookupVariable(node->variable());
   node->VisitChildren(this);
 }
 
 
-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_;
   ScopeBuilder::DepthState saved_depth_state = depth_;
   depth_ = DepthState(depth_.function_ + 1);
   EnterScope(parent, function->position());
   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);
   VisitFunctionNode(function);
   ExitScope(function->end_position());
   depth_ = saved_depth_state;
   current_function_scope_ = saved_function_scope;
   current_function_node_ = saved_function_node;
 }
 
@@ skipping 1497 matching lines @@
 
   try_finally_block_ = saved_block;
   try_catch_block_ = saved_try_catch_block;
   context_depth_ = saved_depth;
   try_depth_ = saved_try_depth;
 
   return instructions;
 }
 
 
-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();
     if (new_context != NULL) {
       *new_context = true;
     }
   }
   return instructions;
 }
 
 
-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();
   }
   return instructions;
 }
 
 
 Fragment FlowGraphBuilder::LoadContextAt(int depth) {
   intptr_t delta = context_depth_ - depth;
   ASSERT(delta >= 0);
@@ skipping 832 matching lines @@
   if (try_catch_block_ == NULL) {
     return CatchClauseNode::kInvalidTryIndex;
   } else {
     return try_catch_block_->try_index();
   }
 }
 
 
 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;
 }
 
 
 void FlowGraphBuilder::SetTempIndex(Definition* definition) {
   definition->set_temp_index(
       stack_ == NULL ? 0 : stack_->definition()->temp_index() + 1);
 }
 
@@ skipping 1184 matching lines @@
   //   class A {
   //     var x = (expr);
   //   }
   for (intptr_t i = 0; i < kernel_klass->fields().length(); i++) {
     Field* kernel_field = kernel_klass->fields()[i];
     Expression* init = kernel_field->initializer();
     if (!kernel_field->IsStatic() && init != NULL) {
       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());
     }
   }
 
   // These to come from:
   //   class A {
   //     var x;
   //     var y;
   //     A(this.x) : super(expr), y = (expr);
   //   }
   for (intptr_t i = 0; i < initializers->length(); i++) {
@@ skipping 1264 matching lines @@
 
 
 void FlowGraphBuilder::VisitEmptyStatement(EmptyStatement* node) {
   fragment_ = Fragment();
 }
 
 
 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;
 }
 
 
 void FlowGraphBuilder::VisitReturnStatement(ReturnStatement* node) {
   bool inside_try_finally = try_finally_block_ != NULL;
 
   Fragment instructions = node->expression() == NULL
                               ? NullConstant()
@@ skipping 156 matching lines @@
 
   fragment_ = Fragment(new (Z) GotoInstr(join), loop_exit);
   --loop_depth_;
 }
 
 
 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) {
     declarations += TranslateStatement(variables[i]);
   }
 
   ++loop_depth_;
   bool negate = false;
   Fragment condition = node->condition() == NULL
                            ? Constant(Bool::True())
@@ skipping 25 matching lines @@
     Fragment loop(join);
     loop += CheckStackOverflow();
     loop += condition;
   } else {
     declarations += condition;
   }
 
   Fragment loop(declarations.entry, loop_exit);
   --loop_depth_;
 
-  loop += ExitScope(node);
+  loop += ExitScope(node->kernel_file_offset());
 
   fragment_ = loop;
 }
 
 
 void FlowGraphBuilder::VisitForInStatement(ForInStatement* node) {
   Fragment instructions = TranslateExpression(node->iterable());
   instructions += PushArgument();
 
   const dart::String& iterator_getter = dart::String::ZoneHandle(
       Z, dart::Field::GetterSymbol(Symbols::Iterator()));
   instructions += InstanceCall(node->iterable()->position(), iterator_getter,
                                Token::kGET, 1);
   LocalVariable* iterator = scopes_->iterator_variables[for_in_depth_];
   instructions += StoreLocal(TokenPosition::kNoSource, iterator);
   instructions += Drop();
 
   ++for_in_depth_;
   ++loop_depth_;
   Fragment condition = LoadLocal(iterator);
   condition += PushArgument();
   condition += InstanceCall(node->iterable()->position(), Symbols::MoveNext(),
                             Token::kILLEGAL, 1);
   TargetEntryInstr* body_entry;
   TargetEntryInstr* loop_exit;
   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(
       Z, dart::Field::GetterSymbol(Symbols::Current()));
   body += InstanceCall(node->position(), current_getter, Token::kGET, 1);
   body +=
       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();
     instructions += Goto(join);
     body += Goto(join);
 
     Fragment loop(join);
     loop += CheckStackOverflow();
     loop += condition;
   } else {
@@ skipping 426 matching lines @@
     }
   }
   Fragment catch_body =
       CatchBlockEntry(handler_types, try_handler_index, needs_stacktrace);
   // Fill in the body of the catch.
   for (intptr_t i = 0; i < node->catches().length(); i++) {
     Catch* catch_clause = node->catches()[i];
 
     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());
       catch_handler_body += StoreLocal(
           TokenPosition::kNoSource, LookupVariable(catch_clause->exception()));
       catch_handler_body += Drop();
     }
     if (catch_clause->stack_trace() != NULL) {
       catch_handler_body += LoadLocal(CurrentStackTrace());
       catch_handler_body +=
@@ skipping 11 matching lines @@
     }
 
     {
       CatchBlock block(this, CurrentException(), CurrentStackTrace(),
                        try_handler_index);
 
       catch_handler_body += TranslateStatement(catch_clause->body());
 
       // 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);
       }
     }
 
     if (type_guard != NULL) {
       if (type_guard->IsMalformed()) {
         catch_body += ThrowTypeError();
         catch_body += Drop();
       } else {
@@ skipping 113 matching lines @@
   fragment_ = continuation;
 }
 
 
 Fragment FlowGraphBuilder::TranslateFunctionNode(FunctionNode* node,
                                                  TreeNode* parent) {
   // The VM has a per-isolate table of functions indexed by the enclosing
   // 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()) {
       position = FunctionDeclaration::Cast(parent)->position();
     }
     if (!position.IsReal()) {
       // Positions has to be unique in regards to the parent.
       // A non-real at this point is probably -1, we cannot blindly use that
       // as others might use it too. Create a new dummy non-real TokenPosition.
       position = TokenPosition(i).ToSynthetic();
@@ skipping 188 matching lines @@
     thread->clear_sticky_error();
     return error.raw();
   }
 }
 
 
 }  // namespace kernel
 }  // namespace dart
 
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)