Optimize away most implicit const getter functions

runtime/vm/parser.cc

Index: runtime/vm/parser.cc
===================================================================
--- runtime/vm/parser.cc	(revision 10092)
+++ runtime/vm/parser.cc	(working copy)
@@ -2529,15 +2529,26 @@
   Function& getter = Function::Handle();
   Function& setter = Function::Handle();
   Field& class_field = Field::Handle();
+  Instance& init_value = Instance::Handle();
   while (true) {
     bool has_initializer = CurrentToken() == Token::kASSIGN;
+    bool have_simple_literal = false;

[regis, 2012/07/31 23:19:54]

has_simple_literal?

[hausner, 2012/07/31 23:34:03]

Done.

     if (has_initializer) {
       ConsumeToken();
+      init_value = Object::sentinel();
       // For static final fields, the initialization expression
       // will be parsed through the kConstImplicitGetter method
       // invocation/compilation.
       // For instance fields, the expression is parsed when a constructor
       // is compiled.
+      // For static fields with very simple initializer expressions
+      // (e.g. a literal number or string) we optimize away the
+      // kConstImplicitGetter and initialize the field here.
+
+      if (field->has_static && (field->has_final || field->has_const) &&
+          (LookaheadToken(1) == Token::kSEMICOLON)) {
+        have_simple_literal = IsSimpleLiteral(*field->type, &init_value);
+      }
       SkipExpr();
     } else {
       if (field->has_const || (field->has_static && field->has_final)) {
@@ -2564,14 +2575,17 @@
     // For static final fields, set value to "uninitialized" and
     // create a kConstImplicitGetter getter method.
     if (field->has_static && has_initializer) {
-      class_field.set_value(Instance::Handle(Object::sentinel()));
-      String& getter_name = String::Handle(Field::GetterSymbol(*field->name));
-      getter = Function::New(getter_name, RawFunction::kConstImplicitGetter,
-                             field->has_static, field->has_final, false,
-                             /* is_abstract */ false,
-                             field->name_pos);
-      getter.set_result_type(*field->type);
-      members->AddFunction(getter);
+      class_field.set_value(init_value);
+      if (!have_simple_literal) {
+        String& getter_name = String::Handle(Field::GetterSymbol(*field->name));
+        getter = Function::New(getter_name, RawFunction::kConstImplicitGetter,
+                               field->has_static, field->has_final,
+                               /* is_abstract */ false,
+                               /* is_external */ false,
+                               field->name_pos);
+        getter.set_result_type(*field->type);
+        members->AddFunction(getter);
+      }
     }
 
     // For instance fields, we create implicit getter and setter methods.
@@ -3519,14 +3533,22 @@
     library_.AddObject(field, var_name);
     if (CurrentToken() == Token::kASSIGN) {
       ConsumeToken();
+      Instance& field_value = Instance::Handle(Object::sentinel());
+      bool have_simple_literal = false;
+      if ((is_final || is_const) && (LookaheadToken(1) == Token::kSEMICOLON)) {
+        have_simple_literal = IsSimpleLiteral(type, &field_value);
+      }
       SkipExpr();
-      field.set_value(Instance::Handle(Object::sentinel()));
-      // Create a static const getter.
-      String& getter_name = String::ZoneHandle(Field::GetterSymbol(var_name));
-      getter = Function::New(getter_name, RawFunction::kConstImplicitGetter,
-                             is_static, is_final, false, false, name_pos);
-      getter.set_result_type(type);
-      top_level->functions.Add(getter);
+      field.set_value(field_value);
+      if (!have_simple_literal) {
+        // Create a static const getter.
+        String& getter_name = String::ZoneHandle(Field::GetterSymbol(var_name));
+        getter = Function::New(getter_name, RawFunction::kConstImplicitGetter,
+                               is_static, is_final, false, false, name_pos);
+        getter.set_result_type(type);
+        top_level->functions.Add(getter);
+      }
+
     } else if (is_final || is_const) {
       ErrorMsg(name_pos, "missing initializer for final or const variable");
     }
@@ -4468,6 +4490,36 @@
 }
 
 
+bool Parser::IsSimpleLiteral(const AbstractType& type, Instance* value) {
+  bool no_check = type.IsDynamicType();
+  if ((CurrentToken() == Token::kINTEGER) &&
+      (no_check || type.IsIntInterface() || type.IsNumberInterface())) {
+    *value = CurrentIntegerLiteral();
+    return true;
+  } else if ((CurrentToken() == Token::kDOUBLE) &&
+      (no_check || type.IsDoubleInterface() || type.IsNumberInterface())) {
+    *value = CurrentDoubleLiteral();
+    return true;
+  } else if ((CurrentToken() == Token::kSTRING) &&
+      (no_check || type.IsStringInterface())) {
+    *value = CurrentLiteral()->raw();

[regis, 2012/07/31 23:19:54]

Not specific to this change list, but it feels strange that CurrentLiteral()
does not return a raw pointer as CurrentIntegerLiteral() and
CurrentDoubleLiteral() both do, and also that it is not called
CurrentStringLiteral().

[hausner, 2012/07/31 23:34:03]

True. CurrentLiteral used to return a pointer to a handle that already existed
in the token stream, so we could avoid allocating an unnecessary handle. Somehow
this code changed. I think Siva has already looked at this and concluded that
returning a raw pointer is less ideal but I don't remember why.

+    return true;
+  } else if ((CurrentToken() == Token::kTRUE) &&
+      (no_check || type.IsBoolInterface())) {
+    *value = Bool::True();
+    return true;
+  } else if ((CurrentToken() == Token::kFALSE) &&
+      (no_check || type.IsBoolInterface())) {
+    *value = Bool::False();
+    return true;
+  } else if (CurrentToken() == Token::kNULL) {
+    *value = Instance::null();
+    return true;
+  }
+  return false;
+}
+
+
 // Returns true if the current token is kIDENT or a pseudo-keyword.
 bool Parser::IsIdentifier() {
   return Token::IsIdentifier(CurrentToken());