Fixed issues with Message not being initialized.

A couple specialized constructors were added that minimize the number of initializations that are then assigned to. This ensures that the Message instance is fully initialized before being used and avoids an extra call to Message::setup. Effective C++ suggests the compiler may be able to emit better code in this case. Overall, the code for the send_* instructions is significantly cleaner.
rubinius · Feb 14, 2009 · 5daf53c · 5daf53c
1 parent 7fe8a1c
commit 5daf53c
Show file tree

Hide file tree

Showing 16 changed files with 179 additions and 165 deletions.
diff --git a/spec/tags/frozen/core/bignum/div_tags.txt b/spec/tags/frozen/core/bignum/div_tags.txt
diff --git a/spec/tags/frozen/core/bignum/divide_tags.txt b/spec/tags/frozen/core/bignum/divide_tags.txt
diff --git a/spec/tags/frozen/core/bignum/divmod_tags.txt b/spec/tags/frozen/core/bignum/divmod_tags.txt
diff --git a/spec/tags/frozen/core/fixnum/div_tags.txt b/spec/tags/frozen/core/fixnum/div_tags.txt
diff --git a/spec/tags/frozen/core/fixnum/divide_tags.txt b/spec/tags/frozen/core/fixnum/divide_tags.txt
diff --git a/spec/tags/frozen/core/fixnum/divmod_tags.txt b/spec/tags/frozen/core/fixnum/divmod_tags.txt
diff --git a/spec/tags/frozen/core/fixnum/modulo_tags.txt b/spec/tags/frozen/core/fixnum/modulo_tags.txt
diff --git a/spec/tags/frozen/language/constants_tags.txt b/spec/tags/frozen/language/constants_tags.txt
@@ -10,6 +10,3 @@ critical:Constant resolution within methods with statically assigned constants d
 critical:Constant resolution within methods with dynamically assigned constants does not search the lexical scope of the caller
 critical:Constant resolution within methods with dynamically assigned constants does not search the lexical scope of qualifying modules
 fails:Literal (A::X) constant resolution sends #const_missing to the original class or module scope
-fails(only on debug):Literal (A::X) constant resolution raises a NameError if no constant is defined in the search path
-fails(only on debug):Literal (A::X) constant resolution with statically assigned constants does not search the singleton class of the class or module
-fails(only on debug):Literal (A::X) constant resolution with dynamically assigned constants does not search the singleton class of the class or module
diff --git a/spec/tags/frozen/language/eigenclass_tags.txt b/spec/tags/frozen/language/eigenclass_tags.txt
@@ -1,4 +1,3 @@
 critical:A constant on an eigenclass is not defined in the eigenclass opener's scope
 critical:A constant on an eigenclass cannot be accessed via object::CONST
 critical:A constant on an eigenclass is not preserved when the object is duped
-fails(only on debug):A constant on an eigenclass raises a NameError for anonymous_module::CONST
diff --git a/spec/tags/frozen/language/metaclass_tags.txt b/spec/tags/frozen/language/metaclass_tags.txt
@@ -1,4 +1,3 @@
 critical:A constant on a metaclass is not defined in the metaclass opener's scope
 critical:A constant on a metaclass cannot be accessed via object::CONST
 critical:A constant on a metaclass is not preserved when the object is duped
-fails(only on debug):A constant on a metaclass raises a NameError for anonymous_module::CONST
diff --git a/vm/helpers.cpp b/vm/helpers.cpp
@@ -106,6 +106,21 @@ namespace rubinius {
       return reinterpret_cast<LookupTableAssociation*>(Qnil);
     }
 
+    Object* const_missing(STATE, Module* under, Symbol* sym, CallFrame* call_frame) {
+      Message msg(state,
+                  G(sym_const_missing),
+                  under,
+                  1,
+                  Qnil,
+                  under->lookup_begin(state));
+
+      Array* args = Array::create(state, 1);
+      args->set(state, 0, sym);
+      msg.set_arguments(state, args);
+
+      return msg.send(state, call_frame);
+    }
+
     Object* locate_method_on(STATE, Object* recv, Symbol* name, Object* priv) {
       Message msg(state);
 

diff --git a/vm/helpers.hpp b/vm/helpers.hpp
@@ -18,6 +18,7 @@ namespace rubinius {
     Object* const_get(VM*, Module* under, Symbol* name, bool* found);
     Object* const_get(VM*, CallFrame* call_frame, Symbol* name, bool* found);
     LookupTableAssociation* const_get_association(VM*, CallFrame* call_frame, Symbol* name, bool* found);
+    Object* const_missing(VM*, Module* under, Symbol* sym, CallFrame* call_frame);
     void const_set(VM*, CallFrame* call_frame, Symbol* name, Object* val);
     void const_set(VM*, Module* mod, Symbol* name, Object* val);
 

diff --git a/vm/instructions.rb b/vm/instructions.rb
@@ -672,16 +672,7 @@ def find_const(index)
     Symbol* sym = as<Symbol>(call_frame->cm->literals()->at(state, index));
     Object* res = Helpers::const_get(state, under, sym, &found);
     if(!found) {
-      Message msg;
-      msg.recv = under;
-      msg.name = state->symbol("const_missing");
-      msg.block = Qnil;
-      msg.lookup_from = msg.recv->lookup_begin(state);
-      Array* args = Array::create(state, 1);
-      args->set(state, 0, sym);
-      msg.set_arguments(state, args);
-
-      res = msg.send(state, call_frame);
+      res = Helpers::const_missing(state, under, sym, call_frame);
     } else if(kind_of<Autoload>(res)) {
       Message msg;
       msg.recv = res;
@@ -2270,21 +2261,14 @@ def push_const(index)
     Symbol* sym = as<Symbol>(call_frame->cm->literals()->at(state, index));
     Object* res = Helpers::const_get(state, call_frame, sym, &found);
     if(!found) {
-      Message msg;
+      Module* under;
       StaticScope* scope = call_frame->cm->scope();
       if(scope->nil_p()) {
-        msg.recv = G(object);
+        under = G(object);
       } else {
-        msg.recv = scope->module();
+        under = scope->module();
       }
-      msg.name = state->symbol("const_missing");
-      msg.block = Qnil;
-      msg.lookup_from = msg.recv->lookup_begin(state);
-      Array* args = Array::create(state, 1);
-      args->set(state, 0, sym);
-      msg.set_arguments(state, args);
-
-      res = msg.send(state, call_frame);
+      res = Helpers::const_missing(state, under, sym, call_frame);
     } else if(kind_of<Autoload>(res)) {
       Message msg;
       msg.recv = res;
@@ -2368,21 +2352,14 @@ def push_const_fast(symbol_index, association_index)
         call_frame->cm->literals()->put(state, association_index, assoc);
         res = assoc->value();
       } else {
-        Message msg;
+        Module* under;
         StaticScope* scope = call_frame->cm->scope();
         if(scope->nil_p()) {
-          msg.recv = G(object);
+          under = G(object);
         } else {
-          msg.recv = scope->module();
+          under = scope->module();
         }
-        msg.name = state->symbol("const_missing");
-        msg.block = Qnil;
-        msg.lookup_from = msg.recv->lookup_begin(state);
-        Array* args = Array::create(state, 1);
-        args->set(state, 0, sym);
-        msg.set_arguments(state, args);
-
-        res = msg.send(state, call_frame);
+        res = Helpers::const_missing(state, under, sym, call_frame);
       }
     } else {
       LookupTableAssociation* real_assoc = as<LookupTableAssociation>(assoc);
@@ -3051,27 +3028,24 @@ def test_rotate
 
   def send_method(index)
     <<-CODE
-    Message msg;
-
-    msg.setup(
-      vmm->sendsites[index].get(),
-      stack_top(),
-      call_frame,
-      0);
-
-    msg.block = Qnil;
-
-    msg.priv = ALLOW_PRIVATE();
-    msg.lookup_from = msg.recv->lookup_begin(state);
-    msg.name = msg.send_site->name();
+    Object* recv = stack_top();
+    SendSite* ss = vmm->sendsites[index].get();
+    Message msg(state,
+                ss,
+                ss->name(),
+                recv,
+                call_frame,
+                0,
+                Qnil,
+                ALLOW_PRIVATE(),
+                recv->lookup_begin(state));
 
     SET_ALLOW_PRIVATE(false);
 
     Object* ret = msg.send_site->performer(state, call_frame, msg);
     stack_pop();
 
     HANDLE_EXCEPTION(ret);
-
     stack_push(ret);
     CODE
   end
@@ -3136,24 +3110,23 @@ def test_send_method
 
   def send_stack(index, count)
     <<-CODE
-    Message msg;
-
-    msg.setup(
-      vmm->sendsites[index].get(),
-      stack_back(count),
-      call_frame,
-      count);
-
-    msg.block = Qnil;
-
-    msg.priv = ALLOW_PRIVATE();
-    msg.lookup_from = msg.recv->lookup_begin(state);
-    msg.name = msg.send_site->name();
+    Object* recv = stack_back(count);
+    SendSite* ss = vmm->sendsites[index].get();
+    Message msg(state,
+                ss,
+                ss->name(),
+                recv,
+                call_frame,
+                count,
+                Qnil,
+                ALLOW_PRIVATE(),
+                recv->lookup_begin(state));
 
     SET_ALLOW_PRIVATE(false);
 
     Object* ret = msg.send_site->performer(state, call_frame, msg);
     call_frame->clear_stack(count + 1);
+
     HANDLE_EXCEPTION(ret);
     stack_push(ret);
     CODE
@@ -3223,24 +3196,24 @@ def test_send_stack
 
   def send_stack_with_block(index, count)
     <<-CODE
-    Message msg;
-
-    msg.block = stack_pop();
-
-    msg.setup(
-      vmm->sendsites[index].get(),
-      stack_back(count),
-      call_frame,
-      count);
-
-    msg.priv = ALLOW_PRIVATE();
-    msg.lookup_from = msg.recv->lookup_begin(state);
-    msg.name = msg.send_site->name();
+    Object* block = stack_pop();
+    Object* recv  = stack_back(count);
+    SendSite* ss  = vmm->sendsites[index].get();
+    Message msg(state,
+                ss,
+                ss->name(),
+                recv,
+                call_frame,
+                count,
+                block,
+                ALLOW_PRIVATE(),
+                recv->lookup_begin(state));
 
     SET_ALLOW_PRIVATE(false);
 
     Object* ret = msg.send_site->performer(state, call_frame, msg);
     call_frame->clear_stack(count + 1);
+
     HANDLE_EXCEPTION(ret);
     stack_push(ret);
     CODE
@@ -3317,16 +3290,19 @@ def test_send_stack_with_block
 
   def send_stack_with_splat(index, count)
     <<-CODE
-    Message msg;
-
-    msg.block = stack_pop();
-    Object* ary = stack_pop();
-
-    msg.setup(
-      vmm->sendsites[index].get(),
-      stack_back(count), /* receiver */
-      call_frame,
-      count);
+    Object* block = stack_pop();
+    Object* ary   = stack_pop();
+    Object* recv  = stack_back(count);
+    SendSite* ss  = vmm->sendsites[index].get();
+    Message msg(state,
+                ss,
+                ss->name(),
+                recv,
+                call_frame,
+                count,
+                block,
+                ALLOW_PRIVATE(),
+                recv->lookup_begin(state));
 
     if(!ary->nil_p()) {
       if(CALL_FLAGS() & #{CALL_FLAG_CONCAT}) {
@@ -3336,15 +3312,12 @@ def send_stack_with_splat(index, count)
       }
     }
 
-    msg.priv = ALLOW_PRIVATE();
-    msg.lookup_from = msg.recv->lookup_begin(state);
-    msg.name = msg.send_site->name();
-
     SET_CALL_FLAGS(0);
     SET_ALLOW_PRIVATE(false);
 
     Object* ret = msg.send_site->performer(state, call_frame, msg);
     call_frame->clear_stack(count + 1);
+
     HANDLE_EXCEPTION(ret);
     stack_push(ret);
     CODE
@@ -3417,24 +3390,23 @@ def test_send_stack_with_splat
 
   def send_super_stack_with_block(index, count)
     <<-CODE
-    Message msg;
-
-    msg.block = stack_pop();
-
-    msg.setup(
-      vmm->sendsites[index].get(),
-      call_frame->self(),
-      call_frame,
-      count);
-
-    msg.priv = TRUE;
-    msg.lookup_from = call_frame->module()->superclass();
-    msg.name = msg.send_site->name();
+    Object* block = stack_pop();
+    SendSite* ss  = vmm->sendsites[index].get();
+    Message msg(state,
+                ss,
+                ss->name(),
+                call_frame->self(),
+                call_frame,
+                count,
+                block,
+                true,
+                call_frame->module()->superclass());
 
     SET_ALLOW_PRIVATE(false);
 
     Object* ret = msg.send_site->performer(state, call_frame, msg);
     call_frame->clear_stack(count);
+
     HANDLE_EXCEPTION(ret);
     stack_push(ret);
     CODE
@@ -3554,16 +3526,18 @@ def test_send_super_stack_with_block
 
   def send_super_stack_with_splat(index, count)
     <<-CODE
-    Message msg;
-
-    msg.block = stack_pop();
-    Object* ary = stack_pop();
-
-    msg.setup(
-      vmm->sendsites[index].get(),
-      call_frame->self(),
-      call_frame,
-      count);
+    Object* block = stack_pop();
+    Object* ary   = stack_pop();
+    SendSite* ss  = vmm->sendsites[index].get();
+    Message msg(state,
+                ss,
+                ss->name(),
+                call_frame->self(),
+                call_frame,
+                count,
+                block,
+                true,
+                call_frame->module()->superclass());
 
     if(!ary->nil_p()) {
       if(CALL_FLAGS() & #{CALL_FLAG_CONCAT}) {
@@ -3573,10 +3547,6 @@ def send_super_stack_with_splat(index, count)
       }
     }
 
-    msg.priv = TRUE;  // TODO: how do we test this?
-    msg.lookup_from = call_frame->module()->superclass();
-    msg.name = msg.send_site->name();
-
     SET_CALL_FLAGS(0);
     SET_ALLOW_PRIVATE(false);
 

diff --git a/vm/llvm/instructions.cpp b/vm/llvm/instructions.cpp
@@ -88,11 +88,16 @@ puts io.string
 CODE
 
   Object* send_slowly(STATE, VMMethod* vmm, CallFrame* const call_frame, Symbol* name, size_t args) {
-    Message msg;
-    msg.setup(static_cast<SendSite*>(Qnil), stack_back(args), call_frame, args);
-    msg.name = name;
-    msg.lookup_from = msg.recv->lookup_begin(state);
-    msg.block = Qnil;
+    Object* recv = stack_back(args);
+    Message msg(NULL,
+                static_cast<SendSite*>(Qnil),
+                name,
+                recv,
+                call_frame,
+                args,
+                Qnil,
+                false,
+                recv->lookup_begin(state));
 
     return msg.send(state, call_frame);
   }