Remove EvaluationRoutine to simplify block evaluation handling (#43)

I am not quite sure what the point of `EvaluationRoutine` is.
I suspect it was to make sure primitives can be handled uniformly with
the block evaluation.
But that seems not very beneficial, and makes the code more complex.

So, this PR removes `EvaluationRoutine`.

src/unitTests/CloneObjectsTest.cpp

@@ -192,9 +192,6 @@ void CloneObjectsTest::testCloneEvaluationPrimitive() {
                                  clone->signature);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("holder differs!!", orig->holder,
                                  clone->holder);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("empty differs!!", orig->empty, clone->empty);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("routine differs!!", orig->routine,
-                                 clone->routine);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("numberOfArguments differs!!",
                                  orig->numberOfArguments,
                                  clone->numberOfArguments);

src/unitTests/WalkObjectsTest.cpp

@@ -42,7 +42,7 @@ static const size_t NoOfFields_Class = 4 + NoOfFields_Object;
 static const size_t NoOfFields_Frame = 3 + NoOfFields_Array;
 static const size_t NoOfFields_Block = 2 + NoOfFields_Object;
 static const size_t NoOfFields_Primitive = NoOfFields_Invokable;
-static const size_t NoOfFields_EvaluationPrimitive = 1 + NoOfFields_Primitive;
+static const size_t NoOfFields_EvaluationPrimitive = NoOfFields_Invokable;
 
 static vector<gc_oop_t> walkedObjects;
 /*
@@ -88,7 +88,6 @@ void WalkObjectsTest::testWalkEvaluationPrimitive() {
 
     CPPUNIT_ASSERT(WalkerHasFound(tmp_ptr(evPrim->GetSignature())));
     CPPUNIT_ASSERT(WalkerHasFound(tmp_ptr(evPrim->GetHolder())));
-    CPPUNIT_ASSERT(WalkerHasFound(tmp_ptr(evPrim)));
     CPPUNIT_ASSERT_EQUAL(NoOfFields_EvaluationPrimitive, walkedObjects.size());
 }
 

src/vm/Universe.cpp

@@ -504,7 +504,7 @@ VMClass* Universe::GetBlockClassWithArgs(long numberOfArguments) {
     VMSymbol* name = SymbolFor(Str.str());
     VMClass* result = LoadClassBasic(name, nullptr);
 
-    result->AddInstancePrimitive(new (GetHeap<HEAP_CLS>(), 0)
+    result->AddInstanceInvokable(new (GetHeap<HEAP_CLS>(), 0)
                                      VMEvaluationPrimitive(numberOfArguments));
 
     SetGlobal(name, result);

src/vmobjects/ObjectFormats.h

@@ -145,7 +145,7 @@ class GCInteger        : public GCAbstractObject { public: typedef VMInteger
 class GCInvokable      : public GCAbstractObject { public: typedef VMInvokable      Loaded; };
 class GCMethod         : public GCInvokable      { public: typedef VMMethod         Loaded; };
 class GCPrimitive      : public GCInvokable      { public: typedef VMPrimitive      Loaded; };
-class GCEvaluationPrimitive : public GCPrimitive { public: typedef VMEvaluationPrimitive Loaded; };
+class GCEvaluationPrimitive : public GCInvokable { public: typedef VMEvaluationPrimitive Loaded; };
 class GCSafePrimitive  : public GCInvokable      { public: typedef VMSafePrimitive  Loaded; };
 class GCSafeUnaryPrimitive  : public GCSafePrimitive { public: typedef VMSafeUnaryPrimitive  Loaded; };
 class GCSafeBinaryPrimitive  : public GCSafePrimitive { public: typedef VMSafeBinaryPrimitive  Loaded; };

src/vmobjects/VMClass.cpp

@@ -40,7 +40,6 @@
 #include "VMArray.h"
 #include "VMInvokable.h"
 #include "VMObject.h"
-#include "VMPrimitive.h"
 #include "VMSymbol.h"
 
 const size_t VMClass::VMClassNumberOfFields = 4;
@@ -96,13 +95,6 @@ bool VMClass::AddInstanceInvokable(VMInvokable* ptr) {
     return true;
 }
 
-void VMClass::AddInstancePrimitive(VMPrimitive* ptr) {
-    if (AddInstanceInvokable(ptr)) {
-        // cout << "Warn: Primitive "<<ptr->GetSignature<<" is not in class
-        // definition for class " << name->GetStdString() << endl;
-    }
-}
-
 VMSymbol* VMClass::GetInstanceFieldName(long index) const {
     long numSuperInstanceFields = numberOfSuperInstanceFields();
     if (index >= numSuperInstanceFields) {

src/vmobjects/VMClass.h

@@ -63,7 +63,6 @@ class VMClass : public VMObject {
     VMInvokable* LookupInvokable(VMSymbol*) const;
     long LookupFieldIndex(VMSymbol*) const;
     bool AddInstanceInvokable(VMInvokable*);
-    void AddInstancePrimitive(VMPrimitive*);
     VMSymbol* GetInstanceFieldName(long) const;
     size_t GetNumberOfInstanceFields() const;
     bool HasPrimitives() const;

src/vmobjects/VMEvaluationPrimitive.cpp

@@ -38,12 +38,11 @@
 #include "ObjectFormats.h"
 #include "VMBlock.h"
 #include "VMFrame.h"
-#include "VMPrimitive.h"
 #include "VMSymbol.h"
 
 VMEvaluationPrimitive::VMEvaluationPrimitive(size_t argc)
-    : VMPrimitive(computeSignatureString(argc)), numberOfArguments(argc) {
-    SetRoutine(new EvaluationRoutine(this), false);
+    : VMInvokable(computeSignatureString(argc)), numberOfArguments(argc) {
+    write_barrier(this, load_ptr(signature));
 }
 
 VMEvaluationPrimitive* VMEvaluationPrimitive::CloneForMovingGC() const {
@@ -53,8 +52,7 @@ VMEvaluationPrimitive* VMEvaluationPrimitive::CloneForMovingGC() const {
 }
 
 void VMEvaluationPrimitive::WalkObjects(walk_heap_fn walk) {
-    VMPrimitive::WalkObjects(walk);
-    static_cast<EvaluationRoutine*>(routine)->WalkObjects(walk);
+    VMInvokable::WalkObjects(walk);
 }
 
 VMSymbol* VMEvaluationPrimitive::computeSignatureString(long argc) {
@@ -84,12 +82,10 @@ VMSymbol* VMEvaluationPrimitive::computeSignatureString(long argc) {
     return SymbolFor(signatureString);
 }
 
-void EvaluationRoutine::Invoke(Interpreter* interp, VMFrame* frame) {
-    VMEvaluationPrimitive* prim = load_ptr(evalPrim);
-
+void VMEvaluationPrimitive::Invoke(Interpreter* interp, VMFrame* frame) {
     // Get the block (the receiver) from the stack
-    long numArgs = prim->GetNumberOfArguments();
-    VMBlock* block = static_cast<VMBlock*>(frame->GetStackElement(numArgs - 1));
+    VMBlock* block =
+        static_cast<VMBlock*>(frame->GetStackElement(numberOfArguments - 1));
 
     // Get the context of the block...
     VMFrame* context = block->GetContext();
@@ -100,18 +96,14 @@ void EvaluationRoutine::Invoke(Interpreter* interp, VMFrame* frame) {
     NewFrame->SetContext(context);
 }
 
-void EvaluationRoutine::WalkObjects(walk_heap_fn walk) {
-    evalPrim = static_cast<GCEvaluationPrimitive*>(walk(evalPrim));
-}
-
 std::string VMEvaluationPrimitive::AsDebugString() const {
     return "VMEvaluationPrimitive(" + to_string(numberOfArguments) + ")";
 }
 
 #define INVALID_INT_MARKER 9002002002002002002
 
 void VMEvaluationPrimitive::MarkObjectAsInvalid() {
-    VMPrimitive::MarkObjectAsInvalid();
+    VMInvokable::MarkObjectAsInvalid();
     numberOfArguments = INVALID_INT_MARKER;
 }
 

src/vmobjects/VMEvaluationPrimitive.h

@@ -28,7 +28,7 @@
 
 #include "VMPrimitive.h"
 
-class VMEvaluationPrimitive : public VMPrimitive {
+class VMEvaluationPrimitive : public VMInvokable {
 public:
     typedef GCEvaluationPrimitive Stored;
 
@@ -38,15 +38,17 @@ class VMEvaluationPrimitive : public VMPrimitive {
 
     StdString AsDebugString() const override;
 
-    int64_t GetNumberOfArguments() { return numberOfArguments; }
-
     inline size_t GetObjectSize() const override {
         return sizeof(VMEvaluationPrimitive);
     }
 
+    VMClass* GetClass() const final { return load_ptr(primitiveClass); }
+
     void MarkObjectAsInvalid() override;
     bool IsMarkedInvalid() const override;
 
+    void Invoke(Interpreter* interp, VMFrame* frm) override;
+
 private:
     static VMSymbol* computeSignatureString(long argc);
     void evaluationRoutine(Interpreter*, VMFrame*);
@@ -55,20 +57,3 @@ class VMEvaluationPrimitive : public VMPrimitive {
 
     size_t numberOfArguments;
 };
-
-class EvaluationRoutine : public PrimitiveRoutine {
-private:
-    GCEvaluationPrimitive* evalPrim;
-
-public:
-    EvaluationRoutine(VMEvaluationPrimitive* prim)
-        : PrimitiveRoutine(),
-          // the store without barrier is fine here,
-          // because it's a cyclic structure with `prim` itself,
-          // which will be store in another object,
-          // which will then have a barrier
-          evalPrim(store_with_separate_barrier(prim)) {};
-    void WalkObjects(walk_heap_fn);
-    bool isClassSide() override { return false; }
-    void Invoke(Interpreter* interp, VMFrame* frame) override;
-};

src/vmobjects/VMPrimitive.h

@@ -38,7 +38,7 @@ class VMPrimitive : public VMInvokable {
 
     VMPrimitive(VMSymbol* sig);
 
-    VMClass* GetClass() const override { return load_ptr(primitiveClass); }
+    VMClass* GetClass() const final { return load_ptr(primitiveClass); }
 
     inline size_t GetObjectSize() const override { return sizeof(VMPrimitive); }
 

src/vmobjects/VMSafePrimitive.h

@@ -8,7 +8,7 @@ class VMSafePrimitive : public VMInvokable {
 
     VMSafePrimitive(VMSymbol* sig) : VMInvokable(sig) {}
 
-    VMClass* GetClass() const override { return load_ptr(primitiveClass); }
+    VMClass* GetClass() const final { return load_ptr(primitiveClass); }
 
     inline size_t GetObjectSize() const override {
         return sizeof(VMSafePrimitive);
@@ -32,8 +32,6 @@ class VMSafeUnaryPrimitive : public VMSafePrimitive {
         write_barrier(this, sig);
     }
 
-    VMClass* GetClass() const override { return load_ptr(primitiveClass); }
-
     inline size_t GetObjectSize() const override {
         return sizeof(VMSafeUnaryPrimitive);
     }
@@ -62,8 +60,6 @@ class VMSafeBinaryPrimitive : public VMSafePrimitive {
         write_barrier(this, sig);
     }
 
-    VMClass* GetClass() const override { return load_ptr(primitiveClass); }
-
     inline size_t GetObjectSize() const override {
         return sizeof(VMSafeBinaryPrimitive);
     }
@@ -92,8 +88,6 @@ class VMSafeTernaryPrimitive : public VMSafePrimitive {
         write_barrier(this, sig);
     }
 
-    VMClass* GetClass() const override { return load_ptr(primitiveClass); }
-
     inline size_t GetObjectSize() const override {
         return sizeof(VMSafeTernaryPrimitive);
     }