[Interpreter] Avoid accessing Isolate from during bytecode generation.

src/interpreter/bytecode-generator.cc

Index: src/interpreter/bytecode-generator.cc
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index 48f0c3c354f0c59cbcf51d83005a14b2d7839943..ac9086d74d7eafa87e80affc87368d33046ba8a6 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -600,9 +600,8 @@ class BytecodeGenerator::TestResultScope final : public ExpressionResultScope {
 // Used to build a list of global declaration initial value pairs.
 class BytecodeGenerator::GlobalDeclarationsBuilder final : public ZoneObject {
  public:
-  GlobalDeclarationsBuilder(Isolate* isolate, Zone* zone)
-      : isolate_(isolate),
-        declarations_(0, zone),
+  explicit GlobalDeclarationsBuilder(Zone* zone)
+      : declarations_(0, zone),
         constant_pool_entry_(0),
         has_constant_pool_entry_(false) {}
 
@@ -619,14 +618,14 @@ class BytecodeGenerator::GlobalDeclarationsBuilder final : public ZoneObject {
   Handle<FixedArray> AllocateDeclarationPairs(CompilationInfo* info) {
     DCHECK(has_constant_pool_entry_);
     int array_index = 0;
-    Handle<FixedArray> pairs = isolate_->factory()->NewFixedArray(
+    Handle<FixedArray> pairs = info->isolate()->factory()->NewFixedArray(
         static_cast<int>(declarations_.size() * 2), TENURED);
     for (std::pair<FeedbackVectorSlot, FunctionLiteral*> declaration :
          declarations_) {
       FunctionLiteral* func = declaration.second;
       Handle<Object> initial_value;
       if (func == nullptr) {
-        initial_value = isolate_->factory()->undefined_value();
+        initial_value = info->isolate()->factory()->undefined_value();
       } else {
         initial_value =
             Compiler::GetSharedFunctionInfo(func, info->script(), info);
@@ -657,12 +656,27 @@ class BytecodeGenerator::GlobalDeclarationsBuilder final : public ZoneObject {
   bool empty() { return declarations_.empty(); }
 
  private:
-  Isolate* isolate_;
   ZoneVector<std::pair<FeedbackVectorSlot, FunctionLiteral*>> declarations_;
   size_t constant_pool_entry_;
   bool has_constant_pool_entry_;
 };
 
+class DisallowIsolateAccessScope {

[Michael Starzinger, 2016/08/16 09:10:22]

Just for my own clarification mostly: How much worse would it be to remove the
Isolate field from the BytecodeGenerator and explicitly pass it into
FinalizeBytecode instead. Would that allow us to get rid of this helper class
and the "isolate_access_allowed" flag? Feel free to disregard in case I am
missing something obvious.

[rmcilroy, 2016/08/16 11:24:54]

That works, done.

+ public:
+  explicit DisallowIsolateAccessScope(BytecodeGenerator* generator)
+      : generator_(generator), previous_(generator->isolate_access_allowed()) {
+    generator_->set_isolate_access_allowed(false);
+  }
+
+  ~DisallowIsolateAccessScope() {
+    generator_->set_isolate_access_allowed(previous_);
+  }
+
+ private:
+  BytecodeGenerator* generator_;
+  bool previous_;
+};
+
 BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
     : isolate_(info->isolate()),
       zone_(info->zone()),
@@ -673,8 +687,8 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
           info->SourcePositionRecordingMode())),
       info_(info),
       scope_(info->scope()),
-      globals_builder_(new (zone()) GlobalDeclarationsBuilder(info->isolate(),
-                                                              info->zone())),
+      isolate_access_allowed_(true),
+      globals_builder_(new (zone()) GlobalDeclarationsBuilder(info->zone())),
       global_declarations_(0, info->zone()),
       function_literals_(0, info->zone()),
       native_function_literals_(0, info->zone()),
@@ -684,7 +698,9 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info)
       register_allocator_(nullptr),
       generator_resume_points_(info->literal()->yield_count(), info->zone()),
       generator_state_(),
-      loop_depth_(0) {
+      loop_depth_(0),
+      home_object_symbol_(info->isolate()->factory()->home_object_symbol()),
+      prototype_string_(info->isolate()->factory()->prototype_string()) {
   InitializeAstVisitor(isolate()->stack_guard()->real_climit());
 }
 
@@ -698,7 +714,7 @@ Handle<BytecodeArray> BytecodeGenerator::MakeBytecode() {
 
   if (HasStackOverflow()) return Handle<BytecodeArray>();
 
-  return scope.CloseAndEscape(builder()->ToBytecodeArray());
+  return scope.CloseAndEscape(builder()->ToBytecodeArray(isolate()));
 }
 
 void BytecodeGenerator::FinalizeBytecode() {
@@ -736,6 +752,7 @@ void BytecodeGenerator::GenerateBytecode() {
   DisallowHeapAllocation no_allocation;
   DisallowHandleAllocation no_handles;
   DisallowHandleDereference no_deref;
+  DisallowIsolateAccessScope no_isolate(this);
 
   // Initialize the incoming context.
   ContextScope incoming_context(this, scope(), false);
@@ -1022,7 +1039,7 @@ void BytecodeGenerator::VisitDeclarations(
 
   // Push and reset globals builder.
   global_declarations_.push_back(globals_builder());
-  globals_builder_ = new (zone()) GlobalDeclarationsBuilder(isolate(), zone());
+  globals_builder_ = new (zone()) GlobalDeclarationsBuilder(zone());
 }
 
 void BytecodeGenerator::VisitStatements(ZoneList<Statement*>* statements) {
@@ -1494,11 +1511,10 @@ void BytecodeGenerator::VisitClassLiteral(ClassLiteral* expr) {
   register_allocator()->PrepareForConsecutiveAllocations(2);
   Register literal = register_allocator()->NextConsecutiveRegister();
   Register prototype = register_allocator()->NextConsecutiveRegister();
-  Handle<String> name = isolate()->factory()->prototype_string();
   FeedbackVectorSlot slot = expr->PrototypeSlot();
   builder()
       ->StoreAccumulatorInRegister(literal)
-      .LoadNamedProperty(literal, name, feedback_index(slot))
+      .LoadNamedProperty(literal, prototype_string(), feedback_index(slot))
       .StoreAccumulatorInRegister(prototype);
 
   VisitClassLiteralProperties(expr, literal, prototype);
@@ -1616,7 +1632,7 @@ void BytecodeGenerator::VisitClassLiteralStaticPrototypeWithComputedName(
     Register key) {
   BytecodeLabel done;
   builder()
-      ->LoadLiteral(isolate()->factory()->prototype_string())
+      ->LoadLiteral(prototype_string())
       .CompareOperation(Token::Value::EQ_STRICT, key)
       .JumpIfFalse(&done)
       .CallRuntime(Runtime::kThrowStaticPrototypeError, Register(0), 0)
@@ -3169,7 +3185,7 @@ void BytecodeGenerator::VisitNewLocalFunctionContext() {
     builder()
         ->LoadAccumulatorWithRegister(Register::function_closure())
         .StoreAccumulatorInRegister(closure)
-        .LoadLiteral(scope->GetScopeInfo(isolate()))
+        .LoadLiteral(scope->scope_info())
         .StoreAccumulatorInRegister(scope_info)
         .CallRuntime(Runtime::kNewScriptContext, closure, 2);
   } else {
@@ -3217,7 +3233,7 @@ void BytecodeGenerator::VisitNewLocalBlockContext(Scope* scope) {
   Register closure = register_allocator()->NextConsecutiveRegister();
 
   builder()
-      ->LoadLiteral(scope->GetScopeInfo(isolate()))
+      ->LoadLiteral(scope->scope_info())
       .StoreAccumulatorInRegister(scope_info);
   VisitFunctionClosureForContext();
   builder()
@@ -3277,11 +3293,11 @@ void BytecodeGenerator::VisitSetHomeObject(Register value, Register home_object,
                                            int slot_number) {
   Expression* expr = property->value();
   if (FunctionLiteral::NeedsHomeObject(expr)) {
-    Handle<Name> name = isolate()->factory()->home_object_symbol();
     FeedbackVectorSlot slot = property->GetSlot(slot_number);
     builder()
         ->LoadAccumulatorWithRegister(home_object)
-        .StoreNamedProperty(value, name, feedback_index(slot), language_mode());
+        .StoreNamedProperty(value, home_object_symbol(), feedback_index(slot),
+                            language_mode());
   }
 }