Skip to content

Commit

Permalink
Remove GC-related warning pragmas (#26)
Browse files Browse the repository at this point in the history
I convinced myself that the current implementation should be ok.
And I changed things a little in VMMethod, which seems to be fine, too.
  • Loading branch information
smarr authored Jul 20, 2024
2 parents 5d54adb + 431f159 commit bed4c1a
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 25 deletions.
1 change: 0 additions & 1 deletion src/compiler/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,6 @@ void Parser::unaryMessage(MethodGenerationContext* mgenc, bool super) {
void Parser::binaryMessage(MethodGenerationContext* mgenc, bool super) {
VMSymbol* msg = binarySelector();

bool tmp_bool = false;
binaryOperand(mgenc);

if (super)
Expand Down
3 changes: 0 additions & 3 deletions src/interpreter/Interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,14 +411,11 @@ void Interpreter::doJump(long bytecodeIndex) {
}

void Interpreter::WalkGlobals(walk_heap_fn walk) {
#warning method and frame are stored as VMptrs, is that acceptable? Is the solution here with _store_ptr and load_ptr robust?

method = load_ptr(static_cast<GCMethod*>(walk(_store_ptr(method))));

// Get the current frame and mark it.
// Since marking is done recursively, this automatically
// marks the whole stack
# warning Do I need a null check here?
frame = load_ptr(static_cast<GCFrame*>(walk(_store_ptr(frame))));
}

Expand Down
1 change: 0 additions & 1 deletion src/memory/GenerationalCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ static gc_oop_t copy_if_necessary(gc_oop_t oop) {
// walk recursively
newObj->WalkObjects(copy_if_necessary);

#warning not sure about the use of _store_ptr here, or whether it should be a plain cast
return _store_ptr(newObj);
}

Expand Down
8 changes: 0 additions & 8 deletions src/vm/Universe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,6 @@ Universe::~Universe() {
VMObject* Universe::InitializeGlobals() {
set_vt_to_null();

# warning is _store_ptr sufficient?

//
//allocate nil object
//
Expand Down Expand Up @@ -499,14 +497,12 @@ VMClass* Universe::GetBlockClassWithArgs(long numberOfArguments) {
result->AddInstancePrimitive(new (GetHeap<HEAP_CLS>()) VMEvaluationPrimitive(numberOfArguments) );

SetGlobal(name, result);
# warning is _store_ptr sufficient here?
blockClassesByNoOfArgs[numberOfArguments] = _store_ptr(result);

return result;
}

vm_oop_t Universe::GetGlobal(VMSymbol* name) {
# warning is _store_ptr correct here? it relies on _store_ptr not to be really changed...
auto it = globals.find(_store_ptr(name));
if (it == globals.end()) {
return nullptr;
Expand All @@ -516,7 +512,6 @@ vm_oop_t Universe::GetGlobal(VMSymbol* name) {
}

bool Universe::HasGlobal(VMSymbol* name) {
# warning is _store_ptr correct here? it relies on _store_ptr not to be really changed...
auto it = globals.find(_store_ptr(name));
if (it == globals.end()) {
return false;
Expand Down Expand Up @@ -719,7 +714,6 @@ VMFrame* Universe::NewFrame(VMFrame* previousFrame, VMMethod* method) const {
long additionalBytes = length * sizeof(VMObject*);
result = new (GetHeap<HEAP_CLS>(), additionalBytes) VMFrame(length);
result->clazz = nullptr;
# warning I think _store_ptr is sufficient here, but...
result->method = _store_ptr(method);
result->previousFrame = _store_ptr(previousFrame);
result->ResetStackPointer();
Expand Down Expand Up @@ -879,7 +873,6 @@ VMSymbol* Universe::NewSymbol(const StdString& str) {

VMSymbol* Universe::NewSymbol(const size_t length, const char* str) {
VMSymbol* result = new (GetHeap<HEAP_CLS>(), PADDED_SIZE(length)) VMSymbol(length, str);
# warning is _store_ptr sufficient here?
symbolsMap[StdString(str, length)] = _store_ptr(result);

LOG_ALLOCATION("VMSymbol", result->GetObjectSize());
Expand All @@ -904,6 +897,5 @@ VMSymbol* Universe::SymbolFor(const StdString& str) {
}

void Universe::SetGlobal(VMSymbol* name, vm_oop_t val) {
# warning is _store_ptr correct here? it relies on _store_ptr not to be really changed...
globals[_store_ptr(name)] = _store_ptr(val);
}
1 change: 0 additions & 1 deletion src/vmobjects/VMFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ VMFrame::VMFrame(long size, long nof) :
gc_oop_t* end = (gc_oop_t*) SHIFTED_PTR(this, objectSize);
long i = 0;
while (arguments + i < end) {
# warning is the direct use of gc_oop_t here safe for all GCs?
arguments[i] = nilObject;
i++;
}
Expand Down
18 changes: 8 additions & 10 deletions src/vmobjects/VMMethod.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,11 @@ VMMethod::VMMethod(long bcCount, long numberOfConstants) :
#ifdef UNSAFE_FRAME_OPTIMIZATION
cachedFrame = nullptr;
#endif
# warning not sure whether the use of _store_ptr is ok here

bcLength = _store_ptr(NEW_INT(bcCount));
numberOfLocals = _store_ptr(NEW_INT(0));
maximumNumberOfStackElements = _store_ptr(NEW_INT(0));
numberOfArguments = _store_ptr(NEW_INT(0));
this->numberOfConstants = _store_ptr(NEW_INT(numberOfConstants));
store_ptr(bcLength, NEW_INT(bcCount));
store_ptr(numberOfLocals, NEW_INT(0));
store_ptr(maximumNumberOfStackElements, NEW_INT(0));
store_ptr(numberOfArguments, NEW_INT(0));
store_ptr(this->numberOfConstants, NEW_INT(numberOfConstants));

indexableFields = (gc_oop_t*)(&indexableFields + 2);
for (long i = 0; i < numberOfConstants; ++i) {
Expand Down Expand Up @@ -97,9 +95,9 @@ void VMMethod::WalkObjects(walk_heap_fn walk) {

long numIndexableFields = GetNumberOfIndexableFields();
for (long i = 0; i < numIndexableFields; ++i) {
if (GetIndexableField(i) != nullptr)
# warning not sure _store_ptr is the best way, perhaps we should access the array content directly
indexableFields[i] = walk(_store_ptr(GetIndexableField(i)));
if (indexableFields[i] != nullptr) {
indexableFields[i] = walk(indexableFields[i]);
}
}
}

Expand Down
1 change: 0 additions & 1 deletion src/vmobjects/VMObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ void VMObject::WalkObjects(walk_heap_fn walk) {

long numFields = GetNumberOfFields();
for (long i = 0; i < numFields; ++i) {
# warning not sure whether the use of _store_ptr is correct and robust here
FIELDS[i] = walk(_store_ptr(GetField(i)));
}
}
Expand Down

0 comments on commit bed4c1a

Please sign in to comment.