Eagerly create local var descriptors for artificially created methods (they cannot be regularly par…

runtime/vm/compiler.cc

Index: runtime/vm/compiler.cc
===================================================================
--- runtime/vm/compiler.cc	(revision 45593)
+++ runtime/vm/compiler.cc	(working copy)
@@ -1134,14 +1134,21 @@
   const Function& function = Function::Handle(code.function());
   ParsedFunction* parsed_function = new ParsedFunction(
       Thread::Current(), Function::ZoneHandle(function.raw()));
-  LocalVarDescriptors& var_descs = LocalVarDescriptors::Handle();
+  LocalVarDescriptors& var_descs =
+      LocalVarDescriptors::Handle(code.var_descriptors());
+  ASSERT(var_descs.IsNull());

[hausner, 2015/05/07 16:45:29]

This would fail for regexp function, right? They have eagerly precomputed var
descriptors.
Either move this assertion to the else branch or move the IsIrregexpFunction()
check to the beginning of this function.

[srdjan, 2015/05/07 18:57:55]

This method should not be called if local var descriptor exists already, only if
GetLocalVarDescriptors determines that one is needed. Converted
IsIrregexpFunction to an assert

   if (function.IsIrregexpFunction()) {
     UNREACHABLE();  // Special parsing needed, not yet implemented.
   } else {
-    Parser::ParseFunction(parsed_function);
-    parsed_function->AllocateVariables();
-    var_descs =
-        parsed_function->node_sequence()->scope()->GetVarDescriptors(function);
+    LongJumpScope jump;
+    if (setjmp(*jump.Set()) == 0) {
+      Parser::ParseFunction(parsed_function);
+      parsed_function->AllocateVariables();
+      var_descs = parsed_function->node_sequence()->scope()->
+          GetVarDescriptors(function);
+    } else {
+      UNREACHABLE();

[hausner, 2015/05/07 16:45:29]

Why is the setjmp needed if there is never an exception?

[srdjan, 2015/05/07 18:57:55]

Removed. Was useful for debugging as an error in Parser created a mess. Added
comment.

+    }
   }
   code.set_var_descriptors(var_descs);

[hausner, 2015/05/07 16:45:29]

ASSERT here that the computed var_descs are not null?

[srdjan, 2015/05/07 18:57:56]

Done.

 }
@@ -1194,6 +1201,14 @@
 }
 
 
+static void CreateLocalVarDescriptors(const ParsedFunction& parsed_function) {
+  const Function& func = parsed_function.function();
+  LocalVarDescriptors& var_descs = LocalVarDescriptors::Handle();
+  var_descs = parsed_function.node_sequence()->scope()->GetVarDescriptors(func);
+  Code::Handle(func.unoptimized_code()).set_var_descriptors(var_descs);
+}
+
+
 RawObject* Compiler::EvaluateStaticInitializer(const Field& field) {
   ASSERT(field.is_static());
   // The VM sets the field's value to transiton_sentinel prior to
@@ -1213,6 +1228,8 @@
                                 parsed_function,
                                 false,
                                 Isolate::kNoDeoptId);
+    // Eagerly create local var descriptors.
+    CreateLocalVarDescriptors(*parsed_function);
 
     // Invoke the function to evaluate the expression.
     const Function& initializer = parsed_function->function();
@@ -1282,6 +1299,8 @@
                                 false,
                                 Isolate::kNoDeoptId);
 
+    // Eagerly create local var descriptors.
+    CreateLocalVarDescriptors(*parsed_function);
     const Object& result = PassiveObject::Handle(
         DartEntry::InvokeFunction(func, Object::empty_array()));
     return result.raw();