Minor cleanup

runtime/vm/parser.cc

 // Copyright (c) 2012, 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 "vm/parser.h"
 
 #include "vm/bigint_operations.h"
 #include "vm/class_finalizer.h"
 #include "vm/compiler.h"
 #include "vm/compiler_stats.h"
@@ skipping 242 matching lines @@
 
 // For parsing a compilation unit.
 Parser::Parser(const Script& script, const Library& library)
     : script_(script),
       tokens_iterator_(TokenStream::Handle(script.tokens()), 0),
       token_kind_(Token::kILLEGAL),
       current_block_(NULL),
       is_top_level_(false),
       current_member_(NULL),
       allow_function_literals_(true),
-      current_function_(Function::Handle()),
+      parsed_function_(NULL),
       innermost_function_(Function::Handle()),
       current_class_(Class::Handle()),
       library_(library),
-      try_blocks_list_(NULL),
-      expression_temp_(NULL),
-      saved_current_context_(NULL) {
+      try_blocks_list_(NULL) {
   ASSERT(tokens_iterator_.IsValid());
   ASSERT(!library.IsNull());
 }
 
 
 // For parsing a function.
 Parser::Parser(const Script& script,
-               const Function& function,
+               ParsedFunction* parsed_function,
                intptr_t token_position)
     : script_(script),
       tokens_iterator_(TokenStream::Handle(script.tokens()), token_position),
       token_kind_(Token::kILLEGAL),
       current_block_(NULL),
       is_top_level_(false),
       current_member_(NULL),
       allow_function_literals_(true),
-      current_function_(function),
-      innermost_function_(Function::Handle(function.raw())),
-      current_class_(Class::Handle(current_function_.Owner())),
+      parsed_function_(parsed_function),
+      innermost_function_(Function::Handle(parsed_function->function().raw())),
+      current_class_(Class::Handle(parsed_function->function().Owner())),
       library_(Library::Handle(current_class_.library())),
-      try_blocks_list_(NULL),
-      expression_temp_(NULL),
-      saved_current_context_(NULL) {
+      try_blocks_list_(NULL) {
   ASSERT(tokens_iterator_.IsValid());
-  ASSERT(!function.IsNull());
+  ASSERT(!current_function().IsNull());
   if (FLAG_enable_type_checks) {
     EnsureExpressionTemp();
   }
 }
 
 
 bool Parser::SetAllowFunctionLiterals(bool value) {
   bool current_value = allow_function_literals_;
   allow_function_literals_ = value;
   return current_value;
 }
 
 
 const Function& Parser::current_function() const {
-  return current_function_;
+  ASSERT(parsed_function() != NULL);

[hausner, 2013/02/12 19:34:04]

You make it illegal to call current_function() when parsing top-level.
Previously, Function::null() was returned. I take it we didn't depend on this.

[siva, 2013/02/12 20:47:07]

Initially I implemented this to check if parsed_function is null and return
Function::null(). Then I figured if nobody uses it that way why pay the price
for a check. So I added an assertion that this function does not expect
parsed_function to NULL. If we find cases where we need to access
current_function I can convert the assert to a check.

So far the test suites did not trip the ASSERT.

On 2013/02/12 19:34:04, hausner wrote:

+  return parsed_function()->function();
 }
 
 
 const Function& Parser::innermost_function() const {
   return innermost_function_;
 }
 
 
 const Class& Parser::current_class() const {
   return current_class_;
@@ skipping 403 matching lines @@
 }
 
 
 void Parser::ParseFunction(ParsedFunction* parsed_function) {
   TimerScope timer(FLAG_compiler_stats, &CompilerStats::parser_timer);
   Isolate* isolate = Isolate::Current();
   ASSERT(isolate->long_jump_base()->IsSafeToJump());
   ASSERT(parsed_function != NULL);
   const Function& func = parsed_function->function();
   const Script& script = Script::Handle(isolate, func.script());
-  Parser parser(script, func, func.token_pos());
+  Parser parser(script, parsed_function, func.token_pos());
   SequenceNode* node_sequence = NULL;
   Array& default_parameter_values = Array::ZoneHandle(isolate, Array::null());
   switch (func.kind()) {
     case RawFunction::kRegularFunction:
     case RawFunction::kClosureFunction:
     case RawFunction::kGetterFunction:
     case RawFunction::kSetterFunction:
     case RawFunction::kConstructor:
       // The call to a redirecting factory is redirected.
       ASSERT(!func.IsRedirectingFactory());
@@ skipping 14 matching lines @@
       node_sequence = parser.ParseMethodExtractor(func);
       break;
     default:
       UNREACHABLE();
   }
 
   if (!HasReturnNode(node_sequence)) {
     // Add implicit return node.
     node_sequence->Add(new ReturnNode(func.end_token_pos()));
   }
-  if (parser.expression_temp_ != NULL) {
-    parsed_function->set_expression_temp_var(parser.expression_temp_);
-  }
   if (parsed_function->has_expression_temp_var()) {
     node_sequence->scope()->AddVariable(parsed_function->expression_temp_var());
   }
-  if (parser.saved_current_context_ != NULL) {
-    parsed_function->set_saved_current_context_var(
-        parser.saved_current_context_);
+  if (parsed_function->has_saved_current_context_var()) {
     node_sequence->scope()->AddVariable(
         parsed_function->saved_current_context_var());
   }
   parsed_function->SetNodeSequence(node_sequence);
 
   // The instantiator may be required at run time for generic type checks or
   // allocation of generic types.
   if (parser.IsInstantiatorRequired()) {
     // In the case of a local function, only set the instantiator if the
     // receiver (or type arguments parameter of a factory) was captured.
@@ skipping 5989 matching lines @@
       AstNode* expr = ParseExpr(kAllowConst, kConsumeCascades);
       list->Add(expr);
     }
     expressions = list;
   }
   return expressions;
 }
 
 
 const LocalVariable* Parser::GetIncrementTempLocal() {
-  if (expression_temp_ == NULL) {
-    expression_temp_ = ParsedFunction::CreateExpressionTempVar(
+  if (!parsed_function()->has_expression_temp_var()) {
+    LocalVariable* temp = ParsedFunction::CreateExpressionTempVar(
         current_function().token_pos());
-    ASSERT(expression_temp_ != NULL);
+    ASSERT(temp != NULL);
+    parsed_function()->set_expression_temp_var(temp);
   }
-  return expression_temp_;
+  ASSERT(parsed_function()->has_expression_temp_var());
+  return parsed_function()->expression_temp_var();
 }
 
 
 void Parser::EnsureExpressionTemp() {
   // Temporary used later by the flow_graph_builder.
   GetIncrementTempLocal();
 }
 
 
 void Parser::EnsureSavedCurrentContext() {
   // Used later by the flow_graph_builder to save current context.
-  if (saved_current_context_ == NULL) {
-    // Allocate a local variable to save the current context when we call into
-    // any closure function as the call will destroy the current context.
-    saved_current_context_ =
+  if (!parsed_function()->has_saved_current_context_var()) {
+    LocalVariable* temp =
         new LocalVariable(current_function().token_pos(),
                           Symbols::SavedCurrentContextVar(),
                           Type::ZoneHandle(Type::DynamicType()));
+    ASSERT(temp != NULL);
+    parsed_function()->set_saved_current_context_var(temp);
   }
 }
 
 
 LocalVariable* Parser::CreateTempConstVariable(intptr_t token_pos,
                                                const char* s) {
   char name[64];
   OS::SNPrint(name, 64, ":%s%"Pd, s, token_pos);
   LocalVariable* temp =
       new LocalVariable(token_pos,
@@ skipping 2964 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart