Scope tree serialization and ScopeIterator cleanup.

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 053ff26f3c114f6a32f5df391c9231a45e27a6c5..4481ddce520e9c3e243d334743a0aeeee42ac3f6 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -471,9 +471,12 @@ class SaveScope BASE_EMBEDDED {
       : parser_(parser),
         previous_top_scope_(parser->top_scope_) {
     parser->top_scope_ = scope;
+    scope->SetSourceBegStatementPos(parser_->scanner().location().beg_pos);

[Kevin Millikin (Chromium), 2011/10/05 08:43:36]

I'm leery of setting this implicitly.  It's too easy for someone who just wants
to modify the parser to misunderstand what's going on.

If we commit this, we must have cctests that will break if the wrong positions
are recorded.

It's a bit less convenient, but better, to have explicit calls in the parser to
set the positions.

[Steven, 2011/10/06 19:09:27]

Doing it explicitly now as it also gives me more control over which positions I
actually record. Also added a cctest for that.
On 2011/10/05 08:43:36, Kevin Millikin wrote:

   }
 
   ~SaveScope() {
+    parser_->top_scope_->SetSourceEndStatementPos(
+        parser_->scanner().location().beg_pos);
     parser_->top_scope_ = previous_top_scope_;
   }
 
@@ -3883,6 +3886,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> function_name,
                                   type,
                                   has_duplicate_parameters);
   function_literal->set_function_token_position(function_token_position);
+  scope->SetSourceBegStatementPos(start_pos);

[Kevin Millikin (Chromium), 2011/10/05 08:43:36]

It's a bit warty that some scopes have positions set explicitly and some
implicitly.

I recommend always doing it explicitly as described above.

If it should be implicit, then it should also be that way here (e.g., by having
the LexicalScope constructor and destructor set the start and end positions).

This code can also be improved.  Since the scope records start/end positions, we
don't really need redundant local variables for them.

We also might get away with not passing them as extra parameters to
FunctionLiteral (or even storing them in FunctionLiteral) since they're
redundantly on the scope.

[Steven, 2011/10/06 19:09:27]

Removed the local variables and removed them from FunctionLiteral.
On 2011/10/05 08:43:36, Kevin Millikin wrote:

+  scope->SetSourceEndStatementPos(end_pos);
 
   if (fni_ != NULL && should_infer_name) fni_->AddFunction(function_literal);
   return function_literal;