Skip to content

Commit

Permalink
Merge pull request #376 from FnControlOption/bool
Browse files Browse the repository at this point in the history
Make boolean methods return bool instead of Value
  • Loading branch information
seven1m authored Jan 7, 2022
2 parents 2765989 + 93ffbca commit a4ec813
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 66 deletions.
6 changes: 3 additions & 3 deletions include/natalie/array_object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,15 @@ class ArrayObject : public Object {
Value drop_while(Env *, Block *);
Value each(Env *, Block *);
Value each_index(Env *, Block *);
Value eq(Env *, Value);
Value eql(Env *, Value);
bool eq(Env *, Value);
bool eql(Env *, Value);
Value fetch(Env *, Value, Value, Block *);
Value fill(Env *, Value, Value, Value, Block *);
Value first(Env *, Value);
Value flatten(Env *, Value);
Value flatten_in_place(Env *, Value);
Value hash(Env *);
Value include(Env *, Value);
bool include(Env *, Value);
Value index(Env *, Value, Block *);
Value initialize_copy(Env *, Value);
Value inspect(Env *);
Expand Down
6 changes: 3 additions & 3 deletions include/natalie/hash_object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class HashObject : public Object {
}

Value compare_by_identity(Env *);
Value is_comparing_by_identity() const;
bool is_comparing_by_identity() const;
Value delete_if(Env *, Block *);
Value delete_key(Env *, Value, Block *);
Value dig(Env *, size_t, Value *);
Expand All @@ -153,8 +153,8 @@ class HashObject : public Object {
Value fetch(Env *, Value, Value, Block *);
Value fetch_values(Env *, size_t, Value *, Block *);
Value hash(Env *);
Value has_key(Env *, Value);
Value has_value(Env *, Value);
bool has_key(Env *, Value);
bool has_value(Env *, Value);
Value initialize(Env *, Value, Block *);
Value inspect(Env *);
Value keep_if(Env *, Block *);
Expand Down
2 changes: 1 addition & 1 deletion include/natalie/kernel_module.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class KernelModule : public Object {
Value inspect(Env *env);
static Value inspect(Env *env, Value value);
Value main_obj_inspect(Env *);
Value instance_variable_defined(Env *env, Value name_val);
bool instance_variable_defined(Env *env, Value name_val);
Value instance_variable_get(Env *env, Value name_val);
Value instance_variable_set(Env *env, Value name_val, Value value);
Value lambda(Env *env, Block *block);
Expand Down
2 changes: 1 addition & 1 deletion include/natalie/object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class Object : public Cell {
virtual Value const_fetch(SymbolObject *);
virtual Value const_set(SymbolObject *, Value);

Value ivar_defined(Env *, SymbolObject *);
bool ivar_defined(Env *, SymbolObject *);
Value ivar_get(Env *, SymbolObject *);
Value ivar_remove(Env *, SymbolObject *);
Value ivar_set(Env *, SymbolObject *, Value);
Expand Down
6 changes: 3 additions & 3 deletions include/natalie/range_object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ class RangeObject : public Object {
Value each(Env *, Block *);
Value first(Env *, Value);
Value inspect(Env *);
Value eq(Env *, Value);
Value eqeqeq(Env *, Value);
Value include(Env *, Value);
bool eq(Env *, Value);
bool eqeqeq(Env *, Value);
bool include(Env *, Value);

virtual void visit_children(Visitor &visitor) override {
Object::visit_children(visitor);
Expand Down
30 changes: 15 additions & 15 deletions lib/natalie/compiler/binding_gen.rb
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,9 @@ def generate_name
gen.binding('Array', '&', 'ArrayObject', 'intersection', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', '<<', 'ArrayObject', 'ltlt', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', '<=>', 'ArrayObject', 'cmp', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', 'eql?', 'ArrayObject', 'eql', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', '==', 'ArrayObject', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', '===', 'ArrayObject', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', 'eql?', 'ArrayObject', 'eql', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Array', '==', 'ArrayObject', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Array', '===', 'ArrayObject', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Array', '[]', 'ArrayObject', 'ref', argc: 1..2, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', '[]=', 'ArrayObject', 'refeq', argc: 2..3, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', 'any?', 'ArrayObject', 'any', argc: :any, pass_env: true, pass_block: true, return_type: :Object)
Expand Down Expand Up @@ -343,7 +343,7 @@ def generate_name
gen.binding('Array', 'flatten', 'ArrayObject', 'flatten', argc: 0..1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', 'flatten!', 'ArrayObject', 'flatten_in_place', argc: 0..1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', 'hash', 'ArrayObject', 'hash', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', 'include?', 'ArrayObject', 'include', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', 'include?', 'ArrayObject', 'include', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Array', 'index', 'ArrayObject', 'index', argc: 0..1, pass_env: true, pass_block: true, return_type: :Object)
gen.binding('Array', 'intersection', 'ArrayObject', 'intersection', argc: :any, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Array', 'initialize', 'ArrayObject', 'initialize', argc: 0..2, pass_env: true, pass_block: true, return_type: :Object, visibility: :private)
Expand Down Expand Up @@ -503,7 +503,7 @@ def generate_name
gen.binding('Hash', 'compact', 'HashObject', 'compact', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'compact!', 'HashObject', 'compact_in_place', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'compare_by_identity', 'HashObject', 'compare_by_identity', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'compare_by_identity?', 'HashObject', 'is_comparing_by_identity', argc: 0, pass_env: false, pass_block: false, return_type: :Object)
gen.binding('Hash', 'compare_by_identity?', 'HashObject', 'is_comparing_by_identity', argc: 0, pass_env: false, pass_block: false, return_type: :bool)
gen.binding('Hash', 'default', 'HashObject', 'get_default', argc: 0..1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'default=', 'HashObject', 'set_default', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'default_proc', 'HashObject', 'default_proc', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
Expand All @@ -519,17 +519,17 @@ def generate_name
gen.binding('Hash', 'fetch', 'HashObject', 'fetch', argc: 1..2, pass_env: true, pass_block: true, return_type: :Object)
gen.binding('Hash', 'fetch_values', 'HashObject', 'fetch_values', argc: :any, pass_env: true, pass_block: true, return_type: :Object)
gen.binding('Hash', 'hash', 'HashObject', 'hash', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'has_key?', 'HashObject', 'has_key', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'has_value?', 'HashObject', 'has_value', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'has_key?', 'HashObject', 'has_key', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Hash', 'has_value?', 'HashObject', 'has_value', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Hash', 'initialize', 'HashObject', 'initialize', argc: 0..1, pass_env: true, pass_block: true, return_type: :Object, visibility: :private)
gen.binding('Hash', 'initialize_copy', 'HashObject', 'replace', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'inspect', 'HashObject', 'inspect', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'include?', 'HashObject', 'has_key', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'include?', 'HashObject', 'has_key', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Hash', 'keep_if', 'HashObject', 'keep_if', argc: 0, pass_env: true, pass_block: true, return_type: :Object)
gen.binding('Hash', 'key?', 'HashObject', 'has_key', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'key?', 'HashObject', 'has_key', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Hash', 'keys', 'HashObject', 'keys', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'length', 'HashObject', 'size', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'member?', 'HashObject', 'has_key', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'member?', 'HashObject', 'has_key', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Hash', 'merge', 'HashObject', 'merge', argc: :any, pass_env: true, pass_block: true, return_type: :Object)
gen.binding('Hash', 'merge!', 'HashObject', 'merge_in_place', argc: :any, pass_env: true, pass_block: true, return_type: :Object)
gen.binding('Hash', 'replace', 'HashObject', 'replace', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
Expand All @@ -541,7 +541,7 @@ def generate_name
gen.binding('Hash', 'to_hash', 'HashObject', 'to_hash', argc: 0, pass_env: false, pass_block: false, return_type: :Object)
gen.binding('Hash', 'to_s', 'HashObject', 'inspect', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'update', 'HashObject', 'merge_in_place', argc: :any, pass_env: true, pass_block: true, return_type: :Object)
gen.binding('Hash', 'value?', 'HashObject', 'has_value', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Hash', 'value?', 'HashObject', 'has_value', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Hash', 'values', 'HashObject', 'values', argc: 0, pass_env: true, pass_block: false, return_type: :Object)

gen.undefine_singleton_method('Integer', 'new')
Expand Down Expand Up @@ -621,7 +621,7 @@ def generate_name
gen.binding('Kernel', 'Hash', 'KernelModule', 'Hash', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Kernel', 'hash', 'KernelModule', 'hash', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Kernel', 'inspect', 'KernelModule', 'inspect', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Kernel', 'instance_variable_defined?', 'KernelModule', 'instance_variable_defined', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Kernel', 'instance_variable_defined?', 'KernelModule', 'instance_variable_defined', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Kernel', 'instance_variable_get', 'KernelModule', 'instance_variable_get', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Kernel', 'instance_variable_set', 'KernelModule', 'instance_variable_set', argc: 2, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Kernel', 'instance_variables', 'KernelModule', 'instance_variables', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
Expand Down Expand Up @@ -744,9 +744,9 @@ def generate_name
gen.binding('Range', 'each', 'RangeObject', 'each', argc: 0, pass_env: true, pass_block: true, return_type: :Object)
gen.binding('Range', 'first', 'RangeObject', 'first', argc: 0..1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Range', 'inspect', 'RangeObject', 'inspect', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Range', '==', 'RangeObject', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Range', '===', 'RangeObject', 'eqeqeq', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Range', 'include?', 'RangeObject', 'include', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('Range', '==', 'RangeObject', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Range', '===', 'RangeObject', 'eqeqeq', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
gen.binding('Range', 'include?', 'RangeObject', 'include', argc: 1, pass_env: true, pass_block: false, return_type: :bool)

gen.static_binding('Regexp', 'compile', 'RegexpObject', 'compile', argc: 1..2, pass_env: true, pass_block: false, pass_klass: true, return_type: :Object)
gen.static_binding('Regexp', 'last_match', 'RegexpObject', 'last_match', argc: 0..1, pass_env: true, pass_block: false, pass_klass: false, return_type: :Object)
Expand Down
26 changes: 15 additions & 11 deletions src/array_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,10 @@ Value ArrayObject::any(Env *env, size_t argc, Value *args, Block *block) {
return any_method->call(env, this, argc, args, block);
}

Value ArrayObject::eq(Env *env, Value other) {
bool ArrayObject::eq(Env *env, Value other) {
TM::PairedRecursionGuard guard { this, other.object() };

return guard.run([&](bool is_recursive) -> Value {
Value result = guard.run([&](bool is_recursive) -> Value { // don't return bool in recursion guard
if (other == this)
return TrueObject::the();

Expand Down Expand Up @@ -362,12 +362,14 @@ Value ArrayObject::eq(Env *env, Value other) {

return TrueObject::the();
});

return result->is_true();
}

Value ArrayObject::eql(Env *env, Value other) {
bool ArrayObject::eql(Env *env, Value other) {
TM::PairedRecursionGuard guard { this, other.object() };

return guard.run([&](bool is_recursive) -> Value {
Value result = guard.run([&](bool is_recursive) -> Value { // don't return bool in recursion guard
if (other == this)
return TrueObject::the();
if (!other->is_array())
Expand All @@ -394,6 +396,8 @@ Value ArrayObject::eql(Env *env, Value other) {

return TrueObject::the();
});

return result->is_true();
}

Value ArrayObject::each(Env *env, Block *block) {
Expand Down Expand Up @@ -837,16 +841,16 @@ Value ArrayObject::last(Env *env, Value n) {
return array;
}

Value ArrayObject::include(Env *env, Value item) {
bool ArrayObject::include(Env *env, Value item) {
if (size() == 0) {
return FalseObject::the();
return false;
} else {
for (auto &compare_item : *this) {
if (compare_item.send(env, "=="_s, { item })->is_truthy()) {
return TrueObject::the();
return true;
}
}
return FalseObject::the();
return false;
}
}

Expand Down Expand Up @@ -1423,7 +1427,7 @@ Value ArrayObject::uniq_in_place(Env *env, Block *block) {
if (block) {
key = NAT_RUN_BLOCK_WITHOUT_BREAK(env, block, 1, &item, nullptr);
}
if (hash->has_key(env, key)->is_false()) {
if (!hash->has_key(env, key)) {
hash->put(env, key, item);
}
}
Expand Down Expand Up @@ -1560,7 +1564,7 @@ Value ArrayObject::hash(Env *env) {
// this allows us to return the same hash for recursive arrays:
// a = []; a << a; a.hash == [a].hash # => true
// a = []; a << a << a; a.hash == [a, a].hash # => true
if (item->is_array() && size() == item->as_array()->size() && eql(env, item)->is_truthy())
if (item->is_array() && size() == item->as_array()->size() && eql(env, item))
continue;

if (!item_hash->is_integer() && item_hash->respond_to(env, to_int))
Expand Down Expand Up @@ -1671,7 +1675,7 @@ Value ArrayObject::union_of(Env *env, Value arg) {

auto *result = new ArrayObject();
auto add_value = [&result, &env](Value &val) {
if (result->include(env, val)->is_falsey()) {
if (!result->include(env, val)) {
result->push(val);
}
};
Expand Down
18 changes: 9 additions & 9 deletions src/hash_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ Value HashObject::compare_by_identity(Env *env) {
return this;
}

Value HashObject::is_comparing_by_identity() const {
bool HashObject::is_comparing_by_identity() const {
if (m_is_comparing_by_identity) {
return TrueObject::the();
return true;
} else {
return FalseObject::the();
return false;
}
}

Expand Down Expand Up @@ -624,22 +624,22 @@ Value HashObject::hash(Env *env) {
});
}

Value HashObject::has_key(Env *env, Value key) {
bool HashObject::has_key(Env *env, Value key) {
Value val = get(env, key);
if (val) {
return TrueObject::the();
return true;
} else {
return FalseObject::the();
return false;
}
}

Value HashObject::has_value(Env *env, Value value) {
bool HashObject::has_value(Env *env, Value value) {
for (auto &node : *this) {
if (node.val.send(env, "=="_s, { value })->is_true()) {
return TrueObject::the();
return true;
}
}
return FalseObject::the();
return false;
}

Value HashObject::merge(Env *env, size_t argc, Value *args, Block *block) {
Expand Down
4 changes: 2 additions & 2 deletions src/kernel_module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,9 @@ Value KernelModule::main_obj_inspect(Env *env) {
return new StringObject { "main" };
}

Value KernelModule::instance_variable_defined(Env *env, Value name_val) {
bool KernelModule::instance_variable_defined(Env *env, Value name_val) {
if (is_nil() || is_boolean() || is_integer() || is_float() || is_symbol()) {
return FalseObject::the();
return false;
}
auto name = name_val->to_instance_variable_name(env);
return ivar_defined(env, name);
Expand Down
6 changes: 3 additions & 3 deletions src/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -382,15 +382,15 @@ Value Object::const_set(SymbolObject *name, Value val) {
return m_klass->const_set(name, val);
}

Value Object::ivar_defined(Env *env, SymbolObject *name) {
bool Object::ivar_defined(Env *env, SymbolObject *name) {
if (!name->is_ivar_name())
env->raise_name_error(name, "`{}' is not allowed as an instance variable name", name->c_str());

auto val = m_ivars.get(name, env);
if (val)
return TrueObject::the();
return true;
else
return FalseObject::the();
return false;
}

Value Object::ivar_get(Env *env, SymbolObject *name) {
Expand Down
Loading

0 comments on commit a4ec813

Please sign in to comment.