Introduce scopes to keep track of catch blocks at compile time.

src/full-codegen.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 383 matching lines @@
 }
 
 
 int FullCodeGenerator::SlotOffset(Slot* slot) {
   ASSERT(slot != NULL);
   // Offset is negative because higher indexes are at lower addresses.
   int offset = -slot->index() * kPointerSize;
   // Adjust by a (parameter or local) base offset.
   switch (slot->type()) {
     case Slot::PARAMETER:
-      offset += (scope()->num_parameters() + 1) * kPointerSize;
+      offset += (info_->scope()->num_parameters() + 1) * kPointerSize;

[Mads Ager (chromium), 2011/06/30 12:08:32]

Do you need this change? Don't you have a scope here already so you can just use
the scope() accessor?

[Kevin Millikin (Chromium), 2011/06/30 12:30:06]

This definitely needs to be the function's scope (info_->scope()), not the
current scope which might be a catch scope, where num_parameters() == 0.

[Mads Ager (chromium), 2011/06/30 12:47:44]

Should we create an accessor for that then: function_scope and scope?

       break;
     case Slot::LOCAL:
       offset += JavaScriptFrameConstants::kLocal0Offset;
       break;
     case Slot::CONTEXT:
     case Slot::LOOKUP:
       UNREACHABLE();
   }
   return offset;
 }
@@ skipping 684 matching lines @@
   // executing the catch block. The catch block has been rewritten
   // to introduce a new scope to bind the catch variable and to remove
   // that scope again afterwards.
 
   Label try_handler_setup, catch_entry, done;
   __ Call(&try_handler_setup);
   // Try handler code, exception in result register.
 
   // Extend the context before executing the catch block.
   { Comment cmnt(masm_, "[ Extend catch context");
-    __ Push(stmt->name());
+    __ Push(stmt->variable()->name());
     __ push(result_register());
     PushFunctionArgumentForContextAllocation();
     __ CallRuntime(Runtime::kPushCatchContext, 3);
     StoreToFrameField(StandardFrameConstants::kContextOffset,
                       context_register());
   }
 
+  Scope* saved_scope = scope();

[Mads Ager (chromium), 2011/06/30 12:08:32]

Could use a comment? We could create a scope object for this if we need it in
more places (but it doesn't look like it is?)

[Kevin Millikin (Chromium), 2011/06/30 12:30:06]

It's only used here and I couldn't think of a comment that was more obvious than
the code itself.  Suggestions?

[Mads Ager (chromium), 2011/06/30 12:47:44]

Not sure anything is really needed. You are right that the code is pretty self
explanatory. I was thinking something like:

"Use the scope extended with the catch variable for compiling the catch block."

to match the "Extend" comment above which is nice, but doesn't say anything more
than the code either.

+  scope_ = stmt->scope();
+  ASSERT(scope_->declarations()->is_empty());
   Visit(stmt->catch_block());
+  scope_ = saved_scope;
   __ jmp(&done);
 
   // Try block code. Sets up the exception handler chain.
   __ bind(&try_handler_setup);
   {
     TryCatch try_block(this, &catch_entry);
     __ PushTryHandler(IN_JAVASCRIPT, TRY_CATCH_HANDLER);
     Visit(stmt->try_block());
     __ PopTryHandler();
   }
@@ skipping 181 matching lines @@
   }
 
   return false;
 }
 
 
 #undef __
 
 
 } }  // namespace v8::internal