Adapt VMInvokable hierarchy to not be VMObjects (#29)

Since SOM changed and does not expose all the details it used to, we do
not need to keep the various fields as SOM-level values.
This drastically simplifies things and avoids GC issues.
With the copying and generational GC, we do have to be very careful
around cloning objects and walking objects that were already moved.
However, if we use VMIntegers to represent crucial info, that's rather
hairy.
So, we just don't do that any longer.

src/compiler/MethodGenerationContext.cpp

@@ -47,26 +47,22 @@ MethodGenerationContext::MethodGenerationContext() :
 VMMethod* MethodGenerationContext::Assemble() {
     // create a method instance with the given number of bytecodes and literals
     size_t numLiterals = literals.size();
-
-    VMMethod* meth = GetUniverse()->NewMethod(signature, bytecode.size(),
-            numLiterals);
-
-    // populate the fields that are immediately available
     size_t numLocals = locals.size();
-    meth->SetNumberOfLocals(numLocals);
-
-    meth->SetMaximumNumberOfStackElements(maxStackDepth);
+    VMMethod* meth = GetUniverse()->NewMethod(signature, bytecode.size(),
+            numLiterals, numLocals, maxStackDepth);
 
     // copy literals into the method
     for (int i = 0; i < numLiterals; i++) {
         vm_oop_t l = literals[i];
         meth->SetIndexableField(i, l);
     }
+
     // copy bytecodes into method
     size_t bc_size = bytecode.size();
     for (size_t i = 0; i < bc_size; i++) {
         meth->SetBytecode(i, bytecode[i]);
     }
+
     // return the method - the holder field is to be set later on!
     return meth;
 }

src/misc/debug.cpp

@@ -1,4 +1,6 @@
-#include "../vmobjects/AbstractObject.h"
+#include <string>
+
+#include "../vmobjects/ObjectFormats.h"
 #include "../vmobjects/VMClass.h"
 #include "../vmobjects/VMSymbol.h"
 #include "debug.h"

src/unitTests/CloneObjectsTest.cpp

@@ -111,7 +111,7 @@ void CloneObjectsTest::testCloneArray() {
 
 void CloneObjectsTest::testCloneBlock() {
     VMSymbol* methodSymbol = NewSymbol("someMethod");
-    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0);
+    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0, 0, 0);
     VMBlock* orig = GetUniverse()->NewBlock(method,
             GetUniverse()->GetInterpreter()->GetFrame(),
             method->GetNumberOfArguments());
@@ -128,9 +128,6 @@ void CloneObjectsTest::testClonePrimitive() {
     VMSymbol* primitiveSymbol = NewSymbol("myPrimitive");
     VMPrimitive* orig = VMPrimitive::GetEmptyPrimitive(primitiveSymbol, false);
     VMPrimitive* clone = orig->Clone();
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("class differs!!", orig->clazz, clone->clazz);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("objectSize differs!!", orig->objectSize, clone->objectSize);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("numberOfFields differs!!", orig->numberOfFields, clone->numberOfFields);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("signature differs!!", orig->signature, clone->signature);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("holder differs!!", orig->holder, clone->holder);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("empty differs!!", orig->empty, clone->empty);
@@ -141,9 +138,6 @@ void CloneObjectsTest::testCloneEvaluationPrimitive() {
     VMEvaluationPrimitive* orig = new (GetHeap<HEAP_CLS>()) VMEvaluationPrimitive(1);
     VMEvaluationPrimitive* clone = orig->Clone();
 
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("class differs!!", orig->clazz, clone->clazz);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("objectSize differs!!", orig->objectSize, clone->objectSize);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("numberOfFields differs!!", orig->numberOfFields, clone->numberOfFields);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("signature differs!!", orig->signature, clone->signature);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("holder differs!!", orig->holder, clone->holder);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("empty differs!!", orig->empty, clone->empty);
@@ -153,7 +147,7 @@ void CloneObjectsTest::testCloneEvaluationPrimitive() {
 
 void CloneObjectsTest::testCloneFrame() {
     VMSymbol* methodSymbol = NewSymbol("frameMethod");
-    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0);
+    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0, 0, 0);
     VMFrame* orig = GetUniverse()->NewFrame(nullptr, method);
     VMFrame* context = orig->Clone();
     orig->SetContext(context);
@@ -174,29 +168,20 @@ void CloneObjectsTest::testCloneFrame() {
 
 void CloneObjectsTest::testCloneMethod() {
     VMSymbol* methodSymbol = NewSymbol("myMethod");
-    VMMethod* orig = GetUniverse()->NewMethod(methodSymbol, 0, 0);
+    VMMethod* orig = GetUniverse()->NewMethod(methodSymbol, 0, 0, 0, 0);
     VMMethod* clone = orig->Clone();
 
     CPPUNIT_ASSERT((intptr_t)orig != (intptr_t)clone);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("class differs!!", orig->clazz, clone->clazz);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("objectSize differs!!", orig->objectSize, clone->objectSize);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("numberOfFields differs!!", orig->numberOfFields, clone->numberOfFields);
-
     CPPUNIT_ASSERT_EQUAL_MESSAGE("numberOfLocals differs!!",
-            INT_VAL(load_ptr(orig->numberOfLocals)),
-            INT_VAL(load_ptr(clone->numberOfLocals)));
+            orig->numberOfLocals, clone->numberOfLocals);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("bcLength differs!!",
-            INT_VAL(load_ptr(orig->bcLength)),
-            INT_VAL(load_ptr(clone->bcLength)));
+            orig->bcLength, clone->bcLength);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("maximumNumberOfStackElements differs!!",
-            INT_VAL(load_ptr(orig->maximumNumberOfStackElements)),
-            INT_VAL(load_ptr(clone->maximumNumberOfStackElements)));
+            orig->maximumNumberOfStackElements, clone->maximumNumberOfStackElements);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("numberOfArguments differs!!",
-            INT_VAL(load_ptr(orig->numberOfArguments)),
-            INT_VAL(load_ptr(clone->numberOfArguments)));
+            orig->numberOfArguments, clone->numberOfArguments);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("numberOfConstants differs!!",
-            INT_VAL(load_ptr(orig->numberOfConstants)),
-            INT_VAL(load_ptr(clone->numberOfConstants)));
+            orig->numberOfConstants, clone->numberOfConstants);
 
     CPPUNIT_ASSERT_EQUAL_MESSAGE("GetHolder() differs!!", orig->GetHolder(), clone->GetHolder());
     CPPUNIT_ASSERT_EQUAL_MESSAGE("GetSignature() differs!!", orig->GetSignature(), clone->GetSignature());

src/unitTests/WalkObjectsTest.cpp

@@ -34,8 +34,8 @@ static const size_t NoOfFields_Double = 0;
 static const size_t NoOfFields_Integer = 0;
 static const size_t NoOfFields_BigInteger = 0;
 static const size_t NoOfFields_Array = NoOfFields_Object;
-static const size_t NoOfFields_Invokable = 2 + NoOfFields_Object;
-static const size_t NoOfFields_Method = 5 + NoOfFields_Invokable;
+static const size_t NoOfFields_Invokable = 2;
+static const size_t NoOfFields_Method = NoOfFields_Invokable;
 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;
@@ -80,14 +80,13 @@ void WalkObjectsTest::testWalkEvaluationPrimitive() {
     walkedObjects.clear();
 
     VMEvaluationPrimitive* evPrim = new (GetHeap<HEAP_CLS>()) VMEvaluationPrimitive(1);
+    evPrim->SetHolder(load_ptr(classClass));
     evPrim->WalkObjects(collectMembers);
 
-    CPPUNIT_ASSERT(WalkerHasFound(evPrim->numberOfArguments));
-    CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(evPrim->GetClass())));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(evPrim->GetSignature())));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(evPrim->GetHolder())));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(evPrim)));
-    CPPUNIT_ASSERT_EQUAL(NoOfFields_EvaluationPrimitive + 1, walkedObjects.size());
+    CPPUNIT_ASSERT_EQUAL(NoOfFields_EvaluationPrimitive, walkedObjects.size());
 }
 
 void WalkObjectsTest::testWalkObject() {
@@ -136,26 +135,25 @@ void WalkObjectsTest::testWalkPrimitive() {
     walkedObjects.clear();
     VMSymbol* primitiveSymbol = NewSymbol("myPrimitive");
     VMPrimitive* prim = VMPrimitive::GetEmptyPrimitive(primitiveSymbol, false);
+    prim->SetHolder(load_ptr(methodClass));
 
     prim->WalkObjects(collectMembers);
     CPPUNIT_ASSERT_EQUAL(NoOfFields_Primitive, walkedObjects.size());
-    CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(prim->GetClass())));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(prim->GetSignature())));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(prim->GetHolder())));
 }
 
 void WalkObjectsTest::testWalkFrame() {
     walkedObjects.clear();
     VMSymbol* methodSymbol = NewSymbol("frameMethod");
-    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0);
+    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0, 0, 0);
     VMFrame* frame = GetUniverse()->NewFrame(nullptr, method);
     frame->SetPreviousFrame(frame->Clone());
     frame->SetContext(frame->Clone());
     VMInteger* dummyArg = GetUniverse()->NewInteger(1111);
     frame->SetArgument(0, 0, dummyArg);
     frame->WalkObjects(collectMembers);
 
-    // CPPUNIT_ASSERT(WalkerHasFound(frame->GetClass()));  // VMFrame does no longer have a SOM representation
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(frame->GetPreviousFrame())));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(frame->GetContext())));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(frame->GetMethod())));
@@ -169,16 +167,11 @@ void WalkObjectsTest::testWalkFrame() {
 void WalkObjectsTest::testWalkMethod() {
     walkedObjects.clear();
     VMSymbol* methodSymbol = NewSymbol("myMethod");
-    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0);
+    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0, 0, 0);
+    method->SetHolder(load_ptr(symbolClass));
     method->WalkObjects(collectMembers);
 
-    CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(method->GetClass())));
     //the following fields had no getters -> had to become friend
-    CPPUNIT_ASSERT(WalkerHasFound(method->numberOfLocals));
-    CPPUNIT_ASSERT(WalkerHasFound(method->bcLength));
-    CPPUNIT_ASSERT(WalkerHasFound(method->maximumNumberOfStackElements));
-    CPPUNIT_ASSERT(WalkerHasFound(method->numberOfArguments));
-    CPPUNIT_ASSERT(WalkerHasFound(method->numberOfConstants));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(method->GetHolder())));
     CPPUNIT_ASSERT(WalkerHasFound(_store_ptr(method->GetSignature())));
     CPPUNIT_ASSERT_EQUAL(NoOfFields_Method, walkedObjects.size());
@@ -187,7 +180,7 @@ void WalkObjectsTest::testWalkMethod() {
 void WalkObjectsTest::testWalkBlock() {
     walkedObjects.clear();
     VMSymbol* methodSymbol = NewSymbol("someMethod");
-    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0);
+    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0, 0, 0);
     VMBlock* block = GetUniverse()->NewBlock(method,
             GetUniverse()->GetInterpreter()->GetFrame(),
             method->GetNumberOfArguments());

src/unitTests/WriteBarrierTest.cpp

@@ -67,7 +67,7 @@ void WriteBarrierTest::testWriteBlock() {
     GetHeap<HEAP_CLS>()->writeBarrierCalledOn.clear();
 
     VMSymbol* methodSymbol = NewSymbol("someMethod");
-    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0);
+    VMMethod* method = GetUniverse()->NewMethod(methodSymbol, 0, 0, 0, 0);
     VMBlock* block = GetUniverse()->NewBlock(method,
             GetUniverse()->GetInterpreter()->GetFrame(),
             method->GetNumberOfArguments());
@@ -115,8 +115,6 @@ void WriteBarrierTest::testWriteMethod() {
     VMMethod* method = GetUniverse()->GetInterpreter()->GetFrame()->GetMethod()->Clone();
     method->SetHolder(load_ptr(integerClass));
     TEST_WB_CALLED("VMMethod failed to call writeBarrier on SetHolder", method, load_ptr(integerClass));
-    method->SetSignature(method->GetSignature());
-    TEST_WB_CALLED("VMMethod failed to call writeBarrier on SetSignature", method, method->GetSignature());
 }
 
 void WriteBarrierTest::testWriteEvaluationPrimitive() {
@@ -128,7 +126,6 @@ void WriteBarrierTest::testWriteEvaluationPrimitive() {
     GetHeap<HEAP_CLS>()->writeBarrierCalledOn.clear();
     VMEvaluationPrimitive* evPrim = new (GetHeap<HEAP_CLS>()) VMEvaluationPrimitive(1);
     TEST_WB_CALLED("VMEvaluationPrimitive failed to call writeBarrier when creating", evPrim, evPrim->GetClass());
-    TEST_WB_CALLED("VMEvaluationPrimitive failed to call writeBarrier when creating", evPrim, load_ptr(evPrim->numberOfArguments));
 }
 
 void WriteBarrierTest::testWriteClass() {

src/vm/IsValidObject.cpp

@@ -104,7 +104,7 @@ void obtain_vtables_of_known_classes(VMSymbol* className) {
     VMInteger* i  = new (GetHeap<HEAP_CLS>()) VMInteger(0);
     vt_integer    = *(void**) i;
 
-    VMMethod* mth = new (GetHeap<HEAP_CLS>()) VMMethod(0, 0);
+    VMMethod* mth = new (GetHeap<HEAP_CLS>()) VMMethod(nullptr, 0, 0, 0, 0);
     vt_method     = *(void**) mth;
     vt_object     = *(void**) nilObject;
 

src/vm/Universe.cpp

@@ -256,10 +256,8 @@ Universe::Universe() {
 }
 
 VMMethod* Universe::createBootstrapMethod(VMClass* holder, long numArgsOfMsgSend) {
-    VMMethod* bootstrapMethod = NewMethod(SymbolFor("bootstrap"), 1, 0);
+    VMMethod* bootstrapMethod = NewMethod(SymbolFor("bootstrap"), 1, 0, 0, numArgsOfMsgSend);
     bootstrapMethod->SetBytecode(0, BC_HALT);
-    bootstrapMethod->SetNumberOfLocals(0);
-    bootstrapMethod->SetMaximumNumberOfStackElements(numArgsOfMsgSend);
     bootstrapMethod->SetHolder(holder);
     return bootstrapMethod;
 }
@@ -475,7 +473,7 @@ VMClass* Universe::GetBlockClassWithArgs(long numberOfArguments) {
     VMSymbol* name = SymbolFor(Str.str());
     VMClass* result = LoadClassBasic(name, nullptr);
 
-    result->AddInstancePrimitive(new (GetHeap<HEAP_CLS>()) VMEvaluationPrimitive(numberOfArguments) );
+    result->AddInstancePrimitive(new (GetHeap<HEAP_CLS>()) VMEvaluationPrimitive(numberOfArguments));
 
     SetGlobal(name, result);
     blockClassesByNoOfArgs[numberOfArguments] = _store_ptr(result);
@@ -807,19 +805,16 @@ void Universe::WalkGlobals(walk_heap_fn walk) {
 }
 
 VMMethod* Universe::NewMethod(VMSymbol* signature,
-        size_t numberOfBytecodes, size_t numberOfConstants) const {
-    //Method needs space for the bytecodes and the pointers to the constants
-    long additionalBytes = PADDED_SIZE(numberOfBytecodes + numberOfConstants*sizeof(VMObject*));
+        size_t numberOfBytecodes, size_t numberOfConstants, size_t numLocals, size_t maxStackDepth) const {
+    // method needs space for the bytecodes and the pointers to the constants
+    size_t additionalBytes = PADDED_SIZE(numberOfBytecodes + numberOfConstants*sizeof(VMObject*));
 //#if GC_TYPE==GENERATIONAL
 //    VMMethod* result = new (GetHeap<HEAP_CLS>(),additionalBytes, true) 
 //                VMMethod(numberOfBytecodes, numberOfConstants);
 //#else
-    VMMethod* result = new (GetHeap<HEAP_CLS>(),additionalBytes)
-    VMMethod(numberOfBytecodes, numberOfConstants);
+    VMMethod* result = new (GetHeap<HEAP_CLS>(), additionalBytes)
+        VMMethod(signature, numberOfBytecodes, numberOfConstants, numLocals, maxStackDepth);
 //#endif
-    result->SetClass(load_ptr(methodClass));
-
-    result->SetSignature(signature);
 
     LOG_ALLOCATION("VMMethod", result->GetObjectSize());
     return result;

src/vm/Universe.h

@@ -75,7 +75,7 @@ class Universe {
     VMBlock* NewBlock(VMMethod*, VMFrame*, long);
     VMClass* NewClass(VMClass*) const;
     VMFrame* NewFrame(VMFrame*, VMMethod*) const;
-    VMMethod* NewMethod(VMSymbol*, size_t, size_t) const;
+    VMMethod* NewMethod(VMSymbol*, size_t numberOfBytecodes, size_t numberOfConstants, size_t numLocals, size_t maxStackDepth) const;
     VMObject* NewInstance(VMClass*) const;
     VMObject* NewInstanceWithoutFields() const;
     VMInteger* NewInteger(int64_t) const;

src/vmobjects/AbstractObject.h

@@ -31,15 +31,17 @@ class Interpreter;
 class AbstractVMObject: public VMObjectBase {
 public:
     typedef GCAbstractObject Stored;
-    
+
     virtual int64_t GetHash() const;
     virtual VMClass* GetClass() const = 0;
     virtual AbstractVMObject* Clone() const = 0;
-    virtual void Send(Interpreter*, StdString, vm_oop_t*, long);
+            void Send(Interpreter*, StdString, vm_oop_t*, long);
+
+    /** Size in bytes of the object. */
     virtual size_t GetObjectSize() const = 0;
-    
+
     virtual void MarkObjectAsInvalid() = 0;
-    
+
     virtual StdString AsDebugString() const = 0;
 
     AbstractVMObject() {
@@ -63,7 +65,7 @@ class AbstractVMObject: public VMObjectBase {
 
     long GetFieldIndex(VMSymbol* fieldName) const;
 
-    inline virtual void WalkObjects(walk_heap_fn) {
+    virtual void WalkObjects(walk_heap_fn) {
         return;
     }
 

src/vmobjects/ObjectFormats.h

@@ -119,7 +119,7 @@ class GCArray  : public GCObject         { public: typedef VMArray  Loaded; };
 class GCBlock  : public GCObject         { public: typedef VMBlock  Loaded; };
 class GCDouble : public GCAbstractObject { public: typedef VMDouble Loaded; };
 class GCInteger : public GCAbstractObject { public: typedef VMInteger Loaded; };
-class GCInvokable : public GCObject      { public: typedef VMInvokable Loaded; };
+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; };

src/vmobjects/VMClass.cpp

@@ -263,8 +263,7 @@ void VMClass::setPrimitives(const std::string& cname, bool classSide) {
                 }
 
                 // set routine
-                thePrimitive->SetRoutine(routine);
-                thePrimitive->SetEmpty(false);
+                thePrimitive->SetRoutine(routine, false);
             } else {
                 if (anInvokable->IsPrimitive() && current == this) {
                     if (!routine || routine->isClassSide() == classSide) {

src/vmobjects/VMEvaluationPrimitive.cpp

@@ -25,6 +25,7 @@
  */
 
 #include <cassert>
+#include <cstddef>
 #include <string>
 
 #include "../memory/Heap.h"
@@ -39,10 +40,8 @@
 #include "VMPrimitive.h"
 #include "VMSymbol.h"
 
-VMEvaluationPrimitive::VMEvaluationPrimitive(long argc) : VMPrimitive(computeSignatureString(argc)) {
-    SetRoutine(new EvaluationRoutine(this));
-    SetEmpty(false);
-    store_ptr(numberOfArguments, NEW_INT(argc));
+VMEvaluationPrimitive::VMEvaluationPrimitive(size_t argc) : VMPrimitive(computeSignatureString(argc)), numberOfArguments(argc) {
+    SetRoutine(new EvaluationRoutine(this), false);
 }
 
 VMEvaluationPrimitive* VMEvaluationPrimitive::Clone() const {
@@ -52,7 +51,6 @@ VMEvaluationPrimitive* VMEvaluationPrimitive::Clone() const {
 
 void VMEvaluationPrimitive::WalkObjects(walk_heap_fn walk) {
     VMPrimitive::WalkObjects(walk);
-    numberOfArguments = walk(numberOfArguments);
     static_cast<EvaluationRoutine*>(routine)->WalkObjects(walk);
 }
 
@@ -103,6 +101,5 @@ void EvaluationRoutine::WalkObjects(walk_heap_fn walk) {
 }
 
 std::string VMEvaluationPrimitive::AsDebugString() const {
-    return "VMEvaluationPrimitive(" + to_string(
-                    INT_VAL(load_ptr(numberOfArguments))) + ")";
+    return "VMEvaluationPrimitive(" + to_string(numberOfArguments) + ")";
 }