Skip to content

Commit

Permalink
Standardized use of accessors.
Browse files Browse the repository at this point in the history
There are two types of instance variables used in the C++ code:

1. Variables that reference managed objects and which the GC needs to know
about to properly trace the object graph. We call these "slots".
2. Variables that reference native types (eg int, double, etc) that the GC
*cannot* trace (the GC would misunderstand numbers for memory addresses, for
example).

Previously, we treated these to types quite differently. We used the '// slot'
notation when declaring the first type so that we could automatically generate
code to process the objects. The second type was usually somewhat consistent
but also ad hoc in many places.

Now we mostly use two macros (attr_accessor for the first type, and attr_field
for the second type) that both define the variables and define setters and
getters for them. There are still some places where this needs to be cleaned
up.

One reason that we need to be very careful about the setters for the first
type of variable (the managed object references) is that when the concurrent
GC is running, we need to know when an object reference is stored into an
object that may have already been processed by the marker. If we don't see
this, that object may be considered unreachable by the GC *even though it's
being referenced*. This results in the equivalent to a use-after-free bug in a
language with manually managed memory.

Consistently using the managed object reference accessors means that we can
layer other behavior on the access functions. For example, if we tag objects
with an identifier for the thread that created them, we can log or prevent
mutation access from a different thread.
brixen committed Mar 30, 2016
1 parent 46bd531 commit e57cf32
Showing 127 changed files with 1,600 additions and 1,771 deletions.
6 changes: 3 additions & 3 deletions core/constant_cache.rb
Original file line number Diff line number Diff line change
@@ -2,8 +2,8 @@ module Rubinius
class ConstantCache
attr_reader :name
attr_reader :value
attr_reader :under
attr_reader :scope
attr_reader :module
attr_reader :constant_scope
attr_reader :executable

def ip
@@ -21,7 +21,7 @@ def location
end

def inspect
"#<#{self.class.name}:0x#{self.object_id.to_s(16)} #{location}##{@name} constant=#{@value} under=#{@under}>"
"#<#{self.class.name}:0x#{self.object_id.to_s(16)} #{location}##{@name} constant=#{@value} module=#{@module} constant_scope=#{@constant_scope}>"
end
end
end
4 changes: 2 additions & 2 deletions machine/builtin/access_variable.hpp
Original file line number Diff line number Diff line change
@@ -20,8 +20,8 @@ namespace rubinius {
static void initialize(STATE, AccessVariable* av) {
Executable::initialize(state, av, AccessVariable::access_execute);

av->name_ = nil<Symbol>();
av->write_ = nil<Object>();
av->name(nil<Symbol>());
av->write(nil<Object>());
}

// Rubinius.primitive :accessvariable_allocate
6 changes: 3 additions & 3 deletions machine/builtin/alias.hpp
Original file line number Diff line number Diff line change
@@ -21,9 +21,9 @@ namespace rubinius {
static void initialize(STATE, Alias* alias) {
Executable::initialize(state, alias, Alias::executor);

alias->original_name_ = nil<Symbol>();
alias->original_module_ = nil<Module>();
alias->original_exec_ = nil<Executable>();
alias->original_name(nil<Symbol>());
alias->original_module(nil<Module>());
alias->original_exec(nil<Executable>());
}

static Alias* create(STATE, Symbol* name, Module* mod, Executable* exec);
100 changes: 50 additions & 50 deletions machine/builtin/array.cpp
Original file line number Diff line number Diff line change
@@ -22,23 +22,23 @@ namespace rubinius {
}

native_int Array::size() {
return total_->to_native();
return total()->to_native();
}

void Array::set_size(native_int size) {
total_ = Fixnum::from(size);
total(Fixnum::from(size));
}

native_int Array::offset() {
return start_->to_native();
return start()->to_native();
}

Array* Array::create(STATE, native_int size) {
Array* ary = state->memory()->new_object<Array>(state, G(array));
Tuple* tup = Tuple::create(state, size);

if(ary->young_object_p()) {
ary->tuple_ = tup;
ary->tuple(tup);
} else {
ary->tuple(state, tup);
}
@@ -51,7 +51,7 @@ namespace rubinius {
Tuple* tup = state->memory()->new_fields<Tuple>(state, G(tuple), size);

if(ary->young_object_p()) {
ary->tuple_ = tup;
ary->tuple(tup);
} else {
ary->tuple(state, tup);
}
@@ -95,16 +95,16 @@ namespace rubinius {

if(tup->young_object_p()) {
for(native_int i = 0, j = start->to_native(); i < total; i++, j++) {
tup->field[i] = tuple_->field[j];
tup->field[i] = tuple()->field[j];
}
} else {
for(native_int i = 0, j = start->to_native(); i < total; i++, j++) {
tup->put(state, i, tuple_->field[j]);
tup->put(state, i, tuple()->field[j]);
}
}

if(ary->young_object_p()) {
ary->tuple_ = tup;
ary->tuple(tup);
} else {
ary->tuple(state, tup);
}
@@ -126,7 +126,7 @@ namespace rubinius {
Array* Array::from_tuple(STATE, Tuple* tup) {
native_int length = tup->num_fields();
Array* ary = Array::create(state, length);
ary->tuple_->copy_from(state, tup,
ary->tuple()->copy_from(state, tup,
Fixnum::from(0), Fixnum::from(length),
Fixnum::from(0));

@@ -165,20 +165,20 @@ namespace rubinius {
// fully handle.
Object* Array::aref(STATE, Fixnum* idx) {
native_int index = idx->to_native();
const native_int start = start_->to_native();
const native_int total = start + total_->to_native();
const native_int s = start()->to_native();
const native_int t = s + total()->to_native();

// Handle negative indexes
if(index < 0) {
index += total;
index += t;
} else {
index += start;
index += s;
}

// Off either end, return nil
if(index >= total || index < start) return cNil;
if(index >= t || index < s) return cNil;

return tuple_->at(state, index);
return tuple()->at(state, index);
}

Object* Array::aset(STATE, Fixnum* idx, Object* val) {
@@ -187,7 +187,7 @@ namespace rubinius {
native_int index = idx->to_native();

if(index < 0) {
index += total_->to_native();
index += total()->to_native();
if(index < 0) return Primitives::failure();
}

@@ -208,14 +208,14 @@ namespace rubinius {
}

native_int new_size = size + osize;
if(new_size <= tuple_->num_fields()) {
if(new_size <= tuple()->num_fields()) {
// We have enough space, but may need to shift elements.
if(start_->to_native() + new_size <= tuple_->num_fields()) {
tuple_->copy_from(state, other->tuple(), other->start(), other->total(),
Fixnum::from(start_->to_native() + total_->to_native()));
if(start()->to_native() + new_size <= tuple()->num_fields()) {
tuple()->copy_from(state, other->tuple(), other->start(), other->total(),
Fixnum::from(start()->to_native() + total()->to_native()));
} else {
tuple_->copy_from(state, tuple_, start_, total_, Fixnum::from(0));
tuple_->copy_from(state, other->tuple(), other->start(), other->total(), total_);
tuple()->copy_from(state, tuple(), start(), total(), Fixnum::from(0));
tuple()->copy_from(state, other->tuple(), other->start(), other->total(), total());
start(state, Fixnum::from(0));
}
} else {
@@ -224,46 +224,46 @@ namespace rubinius {
while(size <= new_size) size *= 2;

Tuple* nt = state->memory()->new_fields<Tuple>(state, G(tuple), size);
nt->copy_from(state, tuple_, start_, total_, Fixnum::from(0));
nt->copy_from(state, other->tuple(), other->start(), other->total(), total_);
nt->copy_from(state, tuple(), start(), total(), Fixnum::from(0));
nt->copy_from(state, other->tuple(), other->start(), other->total(), total());

for(native_int i = new_size; i < size; i++) {
nt->field[i] = cNil;
}

if(young_object_p()) {
tuple_ = nt;
tuple(nt);
} else {
tuple(state, nt);
}

start_ = Fixnum::from(0);
start(Fixnum::from(0));
}

total_ = Fixnum::from(new_size);
total(Fixnum::from(new_size));

return this;
}

Object* Array::get(STATE, native_int idx) {
if(idx >= total_->to_native()) {
if(idx >= total()->to_native()) {
return cNil;
}

idx += start_->to_native();
idx += start()->to_native();

return tuple_->at(state, idx);
return tuple()->at(state, idx);
}

Object* Array::set(STATE, native_int idx, Object* val) {
native_int tuple_size = tuple_->num_fields();
native_int tuple_size = tuple()->num_fields();
native_int oidx = idx;
idx += start_->to_native();
idx += start()->to_native();

if(idx >= tuple_size) {
if(oidx < tuple_size) {
// There is enough space in the tuple for this element
tuple_->lshift_inplace(state, start_);
tuple()->lshift_inplace(state, start());
} else {
// Uses the same algo as 1.8 to resize the tuple
native_int new_size = tuple_size / 2;
@@ -272,61 +272,61 @@ namespace rubinius {
}

Tuple* nt = Tuple::create(state, new_size+idx);
nt->copy_from(state, tuple_, start_, total_, Fixnum::from(0));
nt->copy_from(state, tuple(), start(), total(), Fixnum::from(0));
tuple(state, nt);
}
start(state, Fixnum::from(0));
idx = oidx;
}

tuple_->put(state, idx, val);
if(total_->to_native() <= oidx) {
tuple()->put(state, idx, val);
if(total()->to_native() <= oidx) {
total(state, Fixnum::from(oidx+1));
}
return val;
}

void Array::unshift(STATE, Object* val) {
native_int new_size = total_->to_native() + 1;
native_int lend = start_->to_native();
native_int new_size = total()->to_native() + 1;
native_int lend = start()->to_native();

if(lend > 0) {
tuple_->put(state, lend-1, val);
start_ = Fixnum::from(lend-1);
total_ = Fixnum::from(new_size);
tuple()->put(state, lend-1, val);
start(Fixnum::from(lend-1));
total(Fixnum::from(new_size));
} else {
Tuple* nt = state->memory()->new_fields<Tuple>(state, G(tuple), new_size);

nt->copy_from(state, tuple_, start_, total_, Fixnum::from(1));
nt->copy_from(state, tuple(), start(), total(), Fixnum::from(1));
nt->put(state, 0, val);

total_ = Fixnum::from(new_size);
start_ = Fixnum::from(0);
total(Fixnum::from(new_size));
start(Fixnum::from(0));

tuple(state, nt);
}
}

Object* Array::shift(STATE) {
native_int cnt = total_->to_native();
native_int cnt = total()->to_native();

if(cnt == 0) return cNil;

Object* obj = get(state, 0);
set(state, 0, cNil);
start_ = Fixnum::from(start_->to_native() + 1);
total_ = Fixnum::from(cnt - 1);
start(Fixnum::from(start()->to_native() + 1));
total(Fixnum::from(cnt - 1));

return obj;
}

Object* Array::append(STATE, Object* val) {
set(state, total_->to_native(), val);
set(state, total()->to_native(), val);
return val;
}

bool Array::includes_p(STATE, Object* val) {
native_int cnt = total_->to_native();
native_int cnt = total()->to_native();

for(native_int i = 0; i < cnt; i++) {
if(get(state, i) == val) return true;
@@ -336,7 +336,7 @@ namespace rubinius {
}

Object* Array::pop(STATE) {
native_int cnt = total_->to_native();
native_int cnt = total()->to_native();

if(cnt == 0) return cNil;
Object *obj = get(state, cnt - 1);
6 changes: 3 additions & 3 deletions machine/builtin/array.hpp
Original file line number Diff line number Diff line change
@@ -25,9 +25,9 @@ namespace rubinius {

static void bootstrap(STATE);
static void initialize(STATE, Array* array) {
array->total_ = Fixnum::from(0);
array->tuple_ = nil<Tuple>();
array->start_ = Fixnum::from(0);
array->total(Fixnum::from(0));
array->tuple(nil<Tuple>());
array->start(Fixnum::from(0));
}

static Array* create(STATE, native_int size);
2 changes: 1 addition & 1 deletion machine/builtin/atomic.cpp
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ namespace rubinius {
}

Object* AtomicReference::compare_and_set(STATE, Object* old, Object* new_) {
Object** pp = &value_;
Object** pp = &_value_;

if(atomic::compare_and_swap((void**)pp, old, new_)) {
state->memory()->write_barrier(this, new_);
4 changes: 2 additions & 2 deletions machine/builtin/atomic.hpp
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@ namespace rubinius {

static void bootstrap(STATE);
static void initialize(STATE, AtomicReference* ref) {
ref->value_ = nil<Object>();
ref->value(nil<Object>());
}

static AtomicReference* allocate(STATE);
@@ -26,7 +26,7 @@ namespace rubinius {
// Rubinius.primitive+ :atomic_get
Object* get(STATE) {
atomic::memory_barrier();
return value_;
return value();
}

// Rubinius.primitive+ :atomic_set
12 changes: 6 additions & 6 deletions machine/builtin/autoload.hpp
Original file line number Diff line number Diff line change
@@ -26,12 +26,12 @@ namespace rubinius {
/** Register class with the VM. */
static void bootstrap(STATE);
static void initialize(STATE, Autoload* obj) {
obj->name_ = nil<Symbol>();
obj->scope_ = nil<Module>();
obj->path_ = nil<Object>();
obj->constant_ = nil<Object>();
obj->thread_ = nil<Thread>();
obj->loaded_ = nil<Object>();
obj->name(nil<Symbol>());
obj->scope(nil<Module>());
obj->path(nil<Object>());
obj->constant(nil<Object>());
obj->thread(nil<Thread>());
obj->loaded(nil<Object>());
}

// Rubinius.primitive :autoload_allocate
6 changes: 3 additions & 3 deletions machine/builtin/bignum.cpp
Original file line number Diff line number Diff line change
@@ -786,8 +786,8 @@ namespace rubinius {
}

Object* Bignum::compare(STATE, Float* b) {
if(isinf(b->val)) {
if(b->val > 0) {
if(isinf(b->value())) {
if(b->value() > 0) {
return Fixnum::from(-1);
} else {
return Fixnum::from(1);
@@ -1064,7 +1064,7 @@ namespace rubinius {
}

Integer* Bignum::from_float(STATE, Float* f) {
return Bignum::from_double(state, f->val);
return Bignum::from_double(state, f->value());
}

Integer* Bignum::from_double(STATE, double d) {
Loading

0 comments on commit e57cf32

Please sign in to comment.