From b2cbe048423bb3c2c0c06a06b356b2daf28dfb32 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 24 Feb 2025 05:53:48 -0600 Subject: [PATCH 01/23] Use Optional for Env match and catch members --- include/natalie/env.hpp | 10 +++++----- src/env.cpp | 14 +++++++------- src/string_object.cpp | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/include/natalie/env.hpp b/include/natalie/env.hpp index f234517c4..ed3a3d643 100644 --- a/include/natalie/env.hpp +++ b/include/natalie/env.hpp @@ -170,9 +170,9 @@ class Env : public Cell { ModuleObject *module() { return m_module; } void set_module(ModuleObject *module) { m_module = module; } - Value match() { return m_match; } - void set_match(Value match) { m_match = match; } - void clear_match() { m_match = nullptr; } + Optional match() { return m_match; } + void set_match(Optional match) { m_match = match; } + void clear_match() { m_match = {}; } Value exception_object(); ExceptionObject *exception(); @@ -205,8 +205,8 @@ class Env : public Cell { size_t m_line { 0 }; const Method *m_method { nullptr }; ModuleObject *m_module { nullptr }; - Value m_match { nullptr }; + Optional m_match {}; ExceptionObject *m_exception { nullptr }; - Value m_catch { nullptr }; + Optional m_catch {}; }; } diff --git a/src/env.cpp b/src/env.cpp index 0effbfb13..1d59d64f3 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -241,7 +241,7 @@ void Env::raise_type_error2(const Value obj, const char *expected) { } bool Env::has_catch(Value value) const { - return (m_catch && Object::equal(value, m_catch)) || (m_caller && m_caller->has_catch(value)) || (m_outer && m_outer->has_catch(value)); + return (m_catch && Object::equal(value, m_catch.value())) || (m_caller && m_caller->has_catch(value)) || (m_outer && m_outer->has_catch(value)); } void Env::warn(String message) { @@ -300,9 +300,7 @@ void Env::ensure_no_extra_keywords(HashObject *kwargs) { Value Env::last_match() { Env *env = non_block_env(); - if (env->m_match) - return env->m_match; - return Value::nil(); + return env->m_match.value_or(Value::nil()); } bool Env::has_last_match() { @@ -310,7 +308,7 @@ bool Env::has_last_match() { } void Env::set_last_match(MatchDataObject *match) { - non_block_env()->set_match(match); + non_block_env()->set_match(match ? Value(match) : Optional()); } Value Env::exception_object() { @@ -394,8 +392,10 @@ void Env::visit_children(Visitor &visitor) const { visitor.visit(m_caller); visitor.visit(m_method); visitor.visit(m_module); - visitor.visit(m_match); + if (m_match) + visitor.visit(m_match.value()); visitor.visit(m_exception); - visitor.visit(m_catch); + if (m_catch) + visitor.visit(m_catch.value()); } } diff --git a/src/string_object.cpp b/src/string_object.cpp index b697f6c6b..c3f53014c 100644 --- a/src/string_object.cpp +++ b/src/string_object.cpp @@ -1294,7 +1294,7 @@ Value StringObject::scan(Env *env, Value pattern, Block *block) { while (!(match_value = regexp->match_at_byte_offset(env, this, byte_index)).is_nil()) { match_obj = match_value.as_match_data(); - env->set_match(match_obj); + env->set_match(Value(match_obj)); if (match_obj->has_captures()) { auto captures = match_obj->captures(env).as_array_or_raise(env); From 66123937ace1707b37dda04dfe43e6763756d02e Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 24 Feb 2025 06:01:05 -0600 Subject: [PATCH 02/23] Use Optional for Method self member --- include/natalie/method.hpp | 5 +++-- src/method.cpp | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/natalie/method.hpp b/include/natalie/method.hpp index 0aededda8..2b904f312 100644 --- a/include/natalie/method.hpp +++ b/include/natalie/method.hpp @@ -81,7 +81,8 @@ class Method : public Cell { virtual void visit_children(Visitor &visitor) const override final { visitor.visit(m_owner); visitor.visit(m_env); - visitor.visit(m_self); + if (m_self) + visitor.visit(m_self.value()); visitor.visit(m_original_method); } @@ -94,7 +95,7 @@ class Method : public Cell { Method *m_original_method = nullptr; ModuleObject *m_owner; MethodFnPtr m_fn; - Value m_self { nullptr }; + Optional m_self {}; int m_arity { 0 }; Optional m_file {}; Optional m_line {}; diff --git a/src/method.cpp b/src/method.cpp index 2503c9ac0..9d69e0a42 100644 --- a/src/method.cpp +++ b/src/method.cpp @@ -15,9 +15,8 @@ Value Method::call(Env *env, Value self, Args &&args, Block *block) const { e.set_line(env->line()); e.set_block(block); - if (m_self) { - self = m_self; - } + if (m_self) + self = m_self.value(); auto call_fn = [&](Args &&args) { if (block && !block->calling_env()) { From 9cb1574abbc225d12a2cbf5c50c44e9e31a3e663 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 24 Feb 2025 06:14:28 -0600 Subject: [PATCH 03/23] Use Optional for setting a global in Env --- include/natalie/env.hpp | 3 ++- include/natalie/value.hpp | 3 +++ src/env.cpp | 11 +++++++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/include/natalie/env.hpp b/include/natalie/env.hpp index ed3a3d643..87bf7b82d 100644 --- a/include/natalie/env.hpp +++ b/include/natalie/env.hpp @@ -39,7 +39,8 @@ class Env : public Cell { bool global_defined(SymbolObject *); Value global_get(SymbolObject *); - Value global_set(SymbolObject *, Value, bool = false); + Value global_set(SymbolObject *, Object *, bool = false); + Value global_set(SymbolObject *, Optional, bool = false); Value global_alias(SymbolObject *, SymbolObject *); const Method *current_method(); diff --git a/include/natalie/value.hpp b/include/natalie/value.hpp index d24064f75..8255cc36d 100644 --- a/include/natalie/value.hpp +++ b/include/natalie/value.hpp @@ -59,6 +59,9 @@ class Value { bool is_null() const { return m_value == 0x0; } + bool operator==(void *ptr) const { return (void *)m_value == ptr; } + bool operator!=(void *ptr) const { return (void *)m_value != ptr; } + bool operator==(Value other) const { return m_value == other.m_value; } bool operator!=(Value other) const { return m_value != other.m_value; } diff --git a/src/env.cpp b/src/env.cpp index 1d59d64f3..360db7ce7 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -20,8 +20,15 @@ Value Env::global_get(SymbolObject *name) { return GlobalEnv::the()->global_get(this, name); } -Value Env::global_set(SymbolObject *name, Value val, bool readonly) { - NAT_GC_GUARD_VALUE(val); +Value Env::global_set(SymbolObject *name, Object *obj, bool readonly) { + return GlobalEnv::the()->global_set(this, name, obj ? Value(obj) : Optional(), readonly); +} + +Value Env::global_set(SymbolObject *name, Optional val, bool readonly) { +#ifdef NAT_GC_GUARD + if (val) + NAT_GC_GUARD_VALUE(val.value()); +#endif return GlobalEnv::the()->global_set(this, name, val, readonly); } From 9a9101372ba280b992c747a8fc496eaa68ddf42e Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 24 Feb 2025 06:05:22 -0600 Subject: [PATCH 04/23] Use Optional for Thread value and nil for fiber scheduler --- include/natalie/thread_object.hpp | 9 ++++++--- src/fiber_object.cpp | 8 ++------ src/thread_object.cpp | 9 +++------ 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/include/natalie/thread_object.hpp b/include/natalie/thread_object.hpp index cfb3ac321..355077052 100644 --- a/include/natalie/thread_object.hpp +++ b/include/natalie/thread_object.hpp @@ -161,7 +161,10 @@ class ThreadObject : public Object { void unlock_mutexes() const; Value fiber_scheduler() const { return m_fiber_scheduler; } - void set_fiber_scheduler(Value scheduler) { m_fiber_scheduler = scheduler; } + void set_fiber_scheduler(Value scheduler) { + assert(scheduler); + m_fiber_scheduler = scheduler; + } bool abort_on_exception() const { return m_abort_on_exception; } bool set_abort_on_exception(bool abrt) { @@ -282,7 +285,7 @@ class ThreadObject : public Object { thread_t m_mach_thread_port { MACH_PORT_NULL }; #endif std::atomic m_exception { nullptr }; - Value m_value { nullptr }; + Optional m_value {}; HashObject *m_thread_variables { nullptr }; FiberObject *m_main_fiber { nullptr }; FiberObject *m_current_fiber { nullptr }; @@ -307,7 +310,7 @@ class ThreadObject : public Object { TM::Hashmap m_mutexes {}; - Value m_fiber_scheduler { nullptr }; + Value m_fiber_scheduler { Value::nil() }; // This condition variable is used to wake a sleeping thread, // i.e. a thread where Kernel#sleep has been called. diff --git a/src/fiber_object.cpp b/src/fiber_object.cpp index a8fd55bde..efc5026cc 100644 --- a/src/fiber_object.cpp +++ b/src/fiber_object.cpp @@ -193,11 +193,7 @@ NO_SANITIZE_ADDRESS Value FiberObject::resume(Env *env, Args args) { } Value FiberObject::scheduler() { - auto scheduler = ThreadObject::current()->fiber_scheduler(); - if (!scheduler) - return Value::nil(); - - return scheduler; + return ThreadObject::current()->fiber_scheduler(); } bool FiberObject::scheduler_is_relevant() { @@ -210,7 +206,7 @@ bool FiberObject::scheduler_is_relevant() { Value FiberObject::set_scheduler(Env *env, Value scheduler) { if (scheduler.is_nil()) { - ThreadObject::current()->set_fiber_scheduler(nullptr); + ThreadObject::current()->set_fiber_scheduler(Value::nil()); } else { TM::Vector required_methods { "block", "unblock", "kernel_sleep", "io_wait" }; for (const auto &required_method : required_methods) { diff --git a/src/thread_object.cpp b/src/thread_object.cpp index 370776c6c..62e1fafeb 100644 --- a/src/thread_object.cpp +++ b/src/thread_object.cpp @@ -465,11 +465,7 @@ Value ThreadObject::sleep(Env *env, float timeout, Thread::MutexObject *mutex_to Value ThreadObject::value(Env *env) { join(env); - - if (!m_value) - return Value::nil(); - - return m_value; + return m_value.value_or(Value::nil()); } Value ThreadObject::name(Env *env) { @@ -649,7 +645,8 @@ void ThreadObject::visit_children(Visitor &visitor) const { visitor.visit(arg); visitor.visit(m_block); visitor.visit(m_exception); - visitor.visit(m_value); + if (m_value) + visitor.visit(m_value.value()); visitor.visit(m_thread_variables); for (auto pair : m_mutexes) visitor.visit(pair.first); From 7e938472500c9cd7d1f2c2eec847561d0cb028ea Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 24 Feb 2025 06:31:45 -0600 Subject: [PATCH 05/23] Remove specialization for Optional This is causing this error now that I am using Optional earlier in this file: Explicit specialization of 'TM::Optional' after instantiation --- include/natalie/env.hpp | 2 +- include/natalie/value.hpp | 78 --------------------------------------- 2 files changed, 1 insertion(+), 79 deletions(-) diff --git a/include/natalie/env.hpp b/include/natalie/env.hpp index 87bf7b82d..96ae8f443 100644 --- a/include/natalie/env.hpp +++ b/include/natalie/env.hpp @@ -173,7 +173,7 @@ class Env : public Cell { Optional match() { return m_match; } void set_match(Optional match) { m_match = match; } - void clear_match() { m_match = {}; } + void clear_match() { m_match = Optional(); } Value exception_object(); ExceptionObject *exception(); diff --git a/include/natalie/value.hpp b/include/natalie/value.hpp index 8255cc36d..ae21412b3 100644 --- a/include/natalie/value.hpp +++ b/include/natalie/value.hpp @@ -242,81 +242,3 @@ class Value { }; } - -namespace TM { - -// This is a temporary specialization to catch an Optional which appears to be present -// but is actually a nullptr. We won't need this at the point we remove nullptr capabilities -// from Value construction. -template <> -class Optional { -public: - Optional(const Natalie::Value &value) { - assert(value != nullptr); - - m_present = true; - m_value = value; - } - - Optional() - : m_present { false } { } - - Optional(const Optional &other) - : m_present { other.m_present } { - if (m_present) - m_value = other.m_value; - } - - ~Optional() { - clear(); - } - - Optional &operator=(const Optional &other) { - m_present = other.m_present; - if (m_present) - m_value = other.m_value; - return *this; - } - - Natalie::Value &value() { - assert(m_present); - return m_value; - } - - const Natalie::Value &value() const { - assert(m_present); - return m_value; - } - - Natalie::Value const &value_or(const Natalie::Value &fallback) const { - if (present()) - return value(); - else - return fallback; - } - - Natalie::Value value_or(std::function fallback) const { - if (present()) - return value(); - else - return fallback(); - } - - Natalie::Value operator*() const { - assert(m_present); - return m_value; - } - - void clear() { m_present = false; } - - operator bool() const { return m_present; } - - bool present() const { return m_present; } - -private: - bool m_present; - - Natalie::Value m_value {}; -}; - -} From 437281d3b38117407f9daa3291eb721e33f8e7f6 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 24 Feb 2025 06:39:00 -0600 Subject: [PATCH 06/23] Use Optional for all the send() `sent_from` parameters --- include/natalie/object.hpp | 10 +++++----- include/natalie/value.hpp | 10 +++++----- src/object.cpp | 10 +++++----- src/value.cpp | 6 +++--- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/include/natalie/object.hpp b/include/natalie/object.hpp index e7113bbf6..6f55d5d4a 100644 --- a/include/natalie/object.hpp +++ b/include/natalie/object.hpp @@ -148,22 +148,22 @@ class Object : public Cell { return buf; } - Value public_send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Value sent_from = nullptr); + Value public_send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Optional sent_from = {}); static Value public_send(Env *, Value, Args &&, Block *); - Value send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Value sent_from = nullptr); + Value send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Optional sent_from = {}); static Value send(Env *, Value, Args &&, Block *); - Value send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Value sent_from = nullptr) { + Value send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Optional sent_from = {}) { // NOTE: sent_from is unused, but accepting it makes the SendInstruction codegen simpler. :-) return send(env, name, Args(args), block); } - Value send(Env *, SymbolObject *, Args &&, Block *, MethodVisibility, Value = nullptr); + Value send(Env *, SymbolObject *, Args &&, Block *, MethodVisibility, Optional = {}); Value method_missing_send(Env *, SymbolObject *, Args &&, Block *); static Value method_missing(Env *, Value, Args &&, Block *); - Method *find_method(Env *, SymbolObject *, MethodVisibility, Value) const; + Method *find_method(Env *, SymbolObject *, MethodVisibility, Optional) const; Value duplicate(Env *) const; Value clone(Env *env, Optional freeze = {}); diff --git a/include/natalie/value.hpp b/include/natalie/value.hpp index ae21412b3..88ff55412 100644 --- a/include/natalie/value.hpp +++ b/include/natalie/value.hpp @@ -68,19 +68,19 @@ class Value { bool operator!() const { return is_null(); } operator bool() const { return !is_null(); } - Value public_send(Env *, SymbolObject *, Args && = {}, Block * = nullptr, Value sent_from = nullptr); + Value public_send(Env *, SymbolObject *, Args && = {}, Block * = nullptr, Optional sent_from = {}); - Value public_send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Value sent_from = nullptr) { + Value public_send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Optional sent_from = {}) { return public_send(env, name, Args(args), block, sent_from); } - Value send(Env *, SymbolObject *, Args && = {}, Block * = nullptr, Value sent_from = nullptr); + Value send(Env *, SymbolObject *, Args && = {}, Block * = nullptr, Optional sent_from = {}); - Value send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Value sent_from = nullptr) { + Value send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Optional sent_from = {}) { return send(env, name, Args(args), block, sent_from); } - Value immediate_send(Env *env, SymbolObject *name, Args &&args, Block *block, Value sent_from, MethodVisibility visibility); + Value immediate_send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from, MethodVisibility visibility); ClassObject *klass() const; diff --git a/src/object.cpp b/src/object.cpp index 02c83a935..02d4866e0 100644 --- a/src/object.cpp +++ b/src/object.cpp @@ -536,7 +536,7 @@ Value Object::module_function(Env *env, Args &&args) { abort(); } -Value Object::public_send(Env *env, SymbolObject *name, Args &&args, Block *block, Value sent_from) { +Value Object::public_send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { return send(env, name, std::move(args), block, MethodVisibility::Public, sent_from); } @@ -545,7 +545,7 @@ Value Object::public_send(Env *env, Value self, Args &&args, Block *block) { return self.public_send(env->caller(), name, std::move(args), block); } -Value Object::send(Env *env, SymbolObject *name, Args &&args, Block *block, Value sent_from) { +Value Object::send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { return send(env, name, std::move(args), block, MethodVisibility::Private, sent_from); } @@ -554,7 +554,7 @@ Value Object::send(Env *env, Value self, Args &&args, Block *block) { return self.send(env->caller(), name, std::move(args), block); } -Value Object::send(Env *env, SymbolObject *name, Args &&args, Block *block, MethodVisibility visibility_at_least, Value sent_from) { +Value Object::send(Env *env, SymbolObject *name, Args &&args, Block *block, MethodVisibility visibility_at_least, Optional sent_from) { static const auto initialize = SymbolObject::intern("initialize"); Method *method = find_method(env, name, visibility_at_least, sent_from); // TODO: make a copy if has empty keyword hash (unless that's not rare) @@ -591,7 +591,7 @@ Value Object::method_missing(Env *env, Value self, Args &&args, Block *block) { } } -Method *Object::find_method(Env *env, SymbolObject *method_name, MethodVisibility visibility_at_least, Value sent_from) const { +Method *Object::find_method(Env *env, SymbolObject *method_name, MethodVisibility visibility_at_least, Optional sent_from) const { ModuleObject *klass = singleton_class(); if (!klass) klass = m_klass; @@ -609,7 +609,7 @@ Method *Object::find_method(Env *env, SymbolObject *method_name, MethodVisibilit if (visibility >= visibility_at_least) return method_info.method(); - if (visibility == MethodVisibility::Protected && sent_from && sent_from.is_a(env, klass)) + if (visibility == MethodVisibility::Protected && sent_from && sent_from.value().is_a(env, klass)) return method_info.method(); switch (visibility) { diff --git a/src/value.cpp b/src/value.cpp index 27a934f66..7bd575eed 100644 --- a/src/value.cpp +++ b/src/value.cpp @@ -42,7 +42,7 @@ Value Value::integer(TM::String &&str) { }); // FIXME: this doesn't check method visibility, but no tests are failing yet :-) -Value Value::immediate_send(Env *env, SymbolObject *name, Args &&args, Block *block, Value sent_from, MethodVisibility visibility) { +Value Value::immediate_send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from, MethodVisibility visibility) { auto method_info = klass()->find_method(env, name); if (!method_info.is_defined()) { // FIXME: store missing reason on current thread @@ -130,7 +130,7 @@ StringObject *Value::to_str2(Env *env) { result.klass()->inspect_str()); } -Value Value::public_send(Env *env, SymbolObject *name, Args &&args, Block *block, Value sent_from) { +Value Value::public_send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { PROFILED_SEND(NativeProfilerEvent::Type::PUBLIC_SEND); if (is_integer() || is_nil()) @@ -139,7 +139,7 @@ Value Value::public_send(Env *env, SymbolObject *name, Args &&args, Block *block return object()->public_send(env, name, std::move(args), block, sent_from); } -Value Value::send(Env *env, SymbolObject *name, Args &&args, Block *block, Value sent_from) { +Value Value::send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { PROFILED_SEND(NativeProfilerEvent::Type::SEND); if (is_integer() || is_nil()) From c4cb51abfab81fd4c4dfbfe483b44805365d7a63 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Mon, 24 Feb 2025 07:27:17 -0600 Subject: [PATCH 07/23] Use Optional for const_find* returns --- include/natalie.hpp | 2 +- include/natalie/module_object.hpp | 6 +++--- include/natalie/object.hpp | 4 ++-- lib/fiddle.rb | 6 +++--- .../instructions/const_find_instruction.rb | 3 ++- .../instructions/define_class_instruction.rb | 9 ++++++--- .../instructions/define_module_instruction.rb | 16 ++++++++++------ lib/socket.cpp | 8 ++++---- src/encoding_object.cpp | 6 +++--- src/module_object.cpp | 14 +++++++------- src/object.cpp | 6 +++--- src/string_object.cpp | 6 +++--- 12 files changed, 47 insertions(+), 39 deletions(-) diff --git a/include/natalie.hpp b/include/natalie.hpp index 3ce515a22..d06aab82a 100644 --- a/include/natalie.hpp +++ b/include/natalie.hpp @@ -184,7 +184,7 @@ Value super(Env *, Value, Args &&, Block *); void clean_up_and_exit(int status); inline Value find_top_level_const(Env *env, SymbolObject *name) { - return GlobalEnv::the()->Object()->const_find(env, name); + return GlobalEnv::the()->Object()->const_find(env, name).value(); } inline Value fetch_nested_const(std::initializer_list names) { diff --git a/include/natalie/module_object.hpp b/include/natalie/module_object.hpp index eab035d37..3d2e77eb5 100644 --- a/include/natalie/module_object.hpp +++ b/include/natalie/module_object.hpp @@ -53,8 +53,8 @@ class ModuleObject : public Object { Value is_autoload(Env *, Value) const; - Value const_find_with_autoload(Env *, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing); - Value const_find(Env *, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing); + Optional const_find_with_autoload(Env *, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing); + Optional const_find(Env *, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing); Value const_get(SymbolObject *) const; Value const_get(Env *, Value, Optional = {}); @@ -183,7 +183,7 @@ class ModuleObject : public Object { } private: - Value handle_missing_constant(Env *, Value, ConstLookupFailureMode); + Optional handle_missing_constant(Env *, Value, ConstLookupFailureMode); ClassObject *as_class(); diff --git a/include/natalie/object.hpp b/include/natalie/object.hpp index 6f55d5d4a..cb4c30717 100644 --- a/include/natalie/object.hpp +++ b/include/natalie/object.hpp @@ -30,7 +30,7 @@ class Object : public Cell { }; enum class ConstLookupFailureMode { - Null, + None, Raise, ConstMissing, }; @@ -94,7 +94,7 @@ class Object : public Cell { void extend_once(Env *, ModuleObject *); - static Value const_find_with_autoload(Env *, Value, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing); + static Optional const_find_with_autoload(Env *, Value, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing); static Value const_fetch(Value, SymbolObject *); static Value const_set(Env *, Value, SymbolObject *, Value); static Value const_set(Env *, Value, SymbolObject *, MethodFnPtr, StringObject *); diff --git a/lib/fiddle.rb b/lib/fiddle.rb index 2f5862761..47a9b4167 100644 --- a/lib/fiddle.rb +++ b/lib/fiddle.rb @@ -19,7 +19,7 @@ class Handle path.assert_type(env, Object::Type::String, "String"); auto handle = dlopen(path.as_string()->c_str(), RTLD_LAZY); if (!handle) { - auto dl_error = self.klass()->const_find(env, "DLError"_s, Object::ConstLookupSearchMode::NotStrict).as_class(); + auto dl_error = self.klass()->const_find(env, "DLError"_s, Object::ConstLookupSearchMode::NotStrict).value().as_class(); env->raise(dl_error, "{}", dlerror()); } auto handle_ptr = new VoidPObject { handle }; @@ -90,7 +90,7 @@ def call(*args) auto symbol = self->ivar_get(env, "@symbol"_s).integer().to_nat_int_t(); auto fn = (void* (*)())symbol; auto result = fn(); - auto pointer_class = self.klass()->const_find(env, "Pointer"_s, Object::ConstLookupSearchMode::NotStrict).as_class(); + auto pointer_class = self.klass()->const_find(env, "Pointer"_s, Object::ConstLookupSearchMode::NotStrict).value().as_class(); auto pointer_obj = new Object { Object::Type::Object, pointer_class }; auto pointer_ptr = new VoidPObject { result }; pointer_obj->ivar_set(env, "@ptr"_s, pointer_ptr); @@ -101,7 +101,7 @@ def call(*args) auto symbol = self->ivar_get(env, "@symbol"_s).integer().to_nat_int_t(); auto fn = (void* (*)(void*))symbol; void *p1_ptr; - auto pointer_class = self.klass()->const_find(env, "Pointer"_s, Object::ConstLookupSearchMode::NotStrict).as_class(); + auto pointer_class = self.klass()->const_find(env, "Pointer"_s, Object::ConstLookupSearchMode::NotStrict).value().as_class(); if (p1.is_a(env, pointer_class)) p1_ptr = p1->ivar_get(env, "@ptr"_s).as_void_p()->void_ptr(); else if (p1.is_void_p()) diff --git a/lib/natalie/compiler/instructions/const_find_instruction.rb b/lib/natalie/compiler/instructions/const_find_instruction.rb index a8738d601..247116c08 100644 --- a/lib/natalie/compiler/instructions/const_find_instruction.rb +++ b/lib/natalie/compiler/instructions/const_find_instruction.rb @@ -7,6 +7,7 @@ def initialize(name, strict:, failure_mode: 'ConstMissing', file: nil, line: nil @name = name.to_sym @strict = strict @failure_mode = failure_mode + raise 'None not acceptable failure_mode here' if @failure_mode == 'None' # source location info @file = file @@ -37,7 +38,7 @@ def generate(transform) ] transform.exec_and_push( :const, - "Object::const_find_with_autoload(#{args.join(', ')})" + "Object::const_find_with_autoload(#{args.join(', ')}).value()" ) end diff --git a/lib/natalie/compiler/instructions/define_class_instruction.rb b/lib/natalie/compiler/instructions/define_class_instruction.rb index 081eb125a..8241a323c 100644 --- a/lib/natalie/compiler/instructions/define_class_instruction.rb +++ b/lib/natalie/compiler/instructions/define_class_instruction.rb @@ -45,15 +45,18 @@ def generate(transform) end klass = transform.temp('class') + klass_found = transform.temp('class_found') namespace = transform.pop superclass = transform.pop search_mode = private? ? 'StrictPrivate' : 'Strict' code = [] - code << "auto #{klass} = Object::const_find_with_autoload(env, #{namespace}, self, " \ + code << "Value #{klass}" + code << "auto #{klass_found} = Object::const_find_with_autoload(env, #{namespace}, self, " \ "#{transform.intern(@name)}, Object::ConstLookupSearchMode::#{search_mode}, " \ - 'Object::ConstLookupFailureMode::Null)' - code << "if (#{klass}) {" + 'Object::ConstLookupFailureMode::None)' + code << "if (#{klass_found}) {" + code << " #{klass} = #{klass_found}.value()" code << " if (!#{klass}.is_class()) {" code << " env->raise(\"TypeError\", \"#{@name} is not a class\");" code << ' }' diff --git a/lib/natalie/compiler/instructions/define_module_instruction.rb b/lib/natalie/compiler/instructions/define_module_instruction.rb index 5df2fa1f7..e13526144 100644 --- a/lib/natalie/compiler/instructions/define_module_instruction.rb +++ b/lib/natalie/compiler/instructions/define_module_instruction.rb @@ -44,20 +44,24 @@ def generate(transform) end mod = transform.temp('module') + mod_found = transform.temp('module_found') namespace = transform.pop search_mode = private? ? 'StrictPrivate' : 'Strict' code = [] - code << "auto #{mod} = Object::const_find_with_autoload(env, #{namespace}, self, " \ + code << "Value #{mod}" + code << "auto #{mod_found} = Object::const_find_with_autoload(env, #{namespace}, self, " \ "#{transform.intern(@name)}, Object::ConstLookupSearchMode::#{search_mode}, " \ - 'Object::ConstLookupFailureMode::Null)' - code << "if (!#{mod}) {" + 'Object::ConstLookupFailureMode::None)' + code << "if (#{mod_found}) {" + code << " #{mod} = #{mod_found}.value()" + code << " if (!#{mod}.is_module() || #{mod}.is_class()) {" + code << " env->raise(\"TypeError\", \"#{@name} is not a module\");" + code << ' }' + code << '} else {' code << " #{mod} = new ModuleObject(#{@name.to_s.inspect})" code << " Object::const_set(env, #{namespace}, #{transform.intern(@name)}, #{mod})" code << '}' - code << "if (!#{mod}.is_module() || #{mod}.is_class()) {" - code << " env->raise(\"TypeError\", \"#{@name} is not a module\");" - code << '}' code << "#{mod}.as_module()->eval_body(env, #{fn})" transform.exec_and_push(:result_of_define_module, code) diff --git a/lib/socket.cpp b/lib/socket.cpp index 228263e22..918467450 100644 --- a/lib/socket.cpp +++ b/lib/socket.cpp @@ -41,12 +41,12 @@ Value Socket_const_name_to_i(Env *env, Value self, Args &&args, Block *) { if (name.is_string() || name.is_symbol()) { auto sym = name.to_symbol(env, Value::Conversion::Strict); auto Socket = find_top_level_const(env, "Socket"_s).as_module(); - auto value = Socket->const_find(env, sym, Object::ConstLookupSearchMode::Strict, Object::ConstLookupFailureMode::Null); - if (!value) - value = Socket->const_find(env, "SHORT_CONSTANTS"_s).as_hash_or_raise(env)->get(env, sym); + auto value = Socket->const_find(env, sym, Object::ConstLookupSearchMode::Strict, Object::ConstLookupFailureMode::None); if (!value) + value = Socket->const_find(env, "SHORT_CONSTANTS"_s).value().as_hash_or_raise(env)->get(env, sym); + if (!value || !value.value()) env->raise_name_error(sym, "uninitialized constant {}::{}", Socket->inspect_str(), sym->string()); - return value; + return value.value(); } else { if (default_zero) return Value::integer(0); diff --git a/src/encoding_object.cpp b/src/encoding_object.cpp index da8bec356..609c32238 100644 --- a/src/encoding_object.cpp +++ b/src/encoding_object.cpp @@ -103,7 +103,7 @@ Value EncodingObject::encode(Env *env, EncodingObject *orig_encoding, StringObje String::hex(cpt, String::HexFormat::Uppercase), orig_encoding->name(), name()); - env->raise(EncodingClass->const_find(env, "UndefinedConversionError"_s).as_class(), message); + env->raise(EncodingClass->const_find(env, "UndefinedConversionError"_s).value().as_class(), message); } auto result_str = result.to_str(env); @@ -156,7 +156,7 @@ Value EncodingObject::encode(Env *env, EncodingObject *orig_encoding, StringObje String::hex(source_codepoint, String::HexFormat::Uppercase), orig_encoding->name()); } - env->raise(EncodingClass->const_find(env, "UndefinedConversionError"_s).as_class(), message); + env->raise(EncodingClass->const_find(env, "UndefinedConversionError"_s).value().as_class(), message); } auto destination_codepoint = from_unicode_codepoint(unicode_codepoint); @@ -195,7 +195,7 @@ Value EncodingObject::encode(Env *env, EncodingObject *orig_encoding, StringObje hex.append_sprintf("%04X", source_codepoint); message = StringObject::format("U+{} from UTF-8 to {}", hex, name()); } - env->raise(EncodingClass->const_find(env, "UndefinedConversionError"_s).as_class(), message); + env->raise(EncodingClass->const_find(env, "UndefinedConversionError"_s).value().as_class(), message); case EncodeUndefOption::Replace: if (options.replace_option) { temp_string.append(options.replace_option); diff --git a/src/module_object.cpp b/src/module_object.cpp index 902919b17..6cee5f835 100644 --- a/src/module_object.cpp +++ b/src/module_object.cpp @@ -227,7 +227,7 @@ Value ModuleObject::is_autoload(Env *env, Value name) const { return Value::nil(); } -Value ModuleObject::const_find_with_autoload(Env *env, Value self, SymbolObject *name, ConstLookupSearchMode search_mode, ConstLookupFailureMode failure_mode) { +Optional ModuleObject::const_find_with_autoload(Env *env, Value self, SymbolObject *name, ConstLookupSearchMode search_mode, ConstLookupFailureMode failure_mode) { ModuleObject *module = nullptr; auto constant = find_constant(env, name, &module, search_mode); @@ -243,7 +243,7 @@ Value ModuleObject::const_find_with_autoload(Env *env, Value self, SymbolObject return const_find(env, name, search_mode, failure_mode); } -Value ModuleObject::const_find(Env *env, SymbolObject *name, ConstLookupSearchMode search_mode, ConstLookupFailureMode failure_mode) { +Optional ModuleObject::const_find(Env *env, SymbolObject *name, ConstLookupSearchMode search_mode, ConstLookupFailureMode failure_mode) { auto constant = find_constant(env, name, nullptr, search_mode); if (!constant) @@ -252,9 +252,9 @@ Value ModuleObject::const_find(Env *env, SymbolObject *name, ConstLookupSearchMo return constant->value(); } -Value ModuleObject::handle_missing_constant(Env *env, Value name, ConstLookupFailureMode failure_mode) { - if (failure_mode == ConstLookupFailureMode::Null) - return nullptr; +Optional ModuleObject::handle_missing_constant(Env *env, Value name, ConstLookupFailureMode failure_mode) { + if (failure_mode == ConstLookupFailureMode::None) + return Optional(); if (failure_mode == ConstLookupFailureMode::Raise) { auto name_str = name.to_s(env); @@ -341,7 +341,7 @@ Value ModuleObject::constants(Env *env, Optional inherit) const { } Value ModuleObject::const_missing(Env *env, Value name) { - return handle_missing_constant(env, name, ConstLookupFailureMode::Raise); + return handle_missing_constant(env, name, ConstLookupFailureMode::Raise).value(); } void ModuleObject::make_method_alias(Env *env, SymbolObject *new_name, SymbolObject *old_name) { @@ -1074,7 +1074,7 @@ bool ModuleObject::const_defined(Env *env, Value name_value, Optional inh if (inherited && inherited.value().is_falsey()) { return !!m_constants.get(name); } - return !!const_find(env, name, ConstLookupSearchMode::NotStrict, ConstLookupFailureMode::Null); + return const_find(env, name, ConstLookupSearchMode::NotStrict, ConstLookupFailureMode::None).present(); } Value ModuleObject::alias_method(Env *env, Value new_name_value, Value old_name_value) { diff --git a/src/object.cpp b/src/object.cpp index 02d4866e0..081a274af 100644 --- a/src/object.cpp +++ b/src/object.cpp @@ -251,7 +251,7 @@ void Object::extend_once(Env *env, ModuleObject *module) { singleton_class(env, this)->include_once(env, module); } -Value Object::const_find_with_autoload(Env *env, Value ns, Value self, SymbolObject *name, ConstLookupSearchMode search_mode, ConstLookupFailureMode failure_mode) { +Optional Object::const_find_with_autoload(Env *env, Value ns, Value self, SymbolObject *name, ConstLookupSearchMode search_mode, ConstLookupFailureMode failure_mode) { if (GlobalEnv::the()->instance_evaling()) { auto context = GlobalEnv::the()->current_instance_eval_context(); if (context.caller_env->module()) { @@ -733,13 +733,13 @@ void Object::copy_instance_variables(const Value other) { } const char *Object::defined(Env *env, SymbolObject *name, bool strict) { - Value obj = nullptr; + Optional obj; if (name->is_constant_name()) { if (strict) { if (m_type == Type::Module || m_type == Type::Class) obj = static_cast(this)->const_get(name); } else { - obj = m_klass->const_find(env, name, ConstLookupSearchMode::NotStrict, ConstLookupFailureMode::Null); + obj = m_klass->const_find(env, name, ConstLookupSearchMode::NotStrict, ConstLookupFailureMode::None); } if (obj) return "constant"; } else if (name->is_global_name()) { diff --git a/src/string_object.cpp b/src/string_object.cpp index c3f53014c..b6f8f9e3a 100644 --- a/src/string_object.cpp +++ b/src/string_object.cpp @@ -1436,7 +1436,7 @@ Value StringObject::encode_in_place(Env *env, Optional dst_encoding_arg, EncodingObject *dst_encoding_obj = find_encoding(dst_encoding); EncodingObject *src_encoding_obj = find_encoding(src_encoding); if (!dst_encoding_obj || !src_encoding_obj) { - auto klass = m_encoding->klass()->const_find(env, "ConverterNotFoundError"_s).as_class(); + auto klass = m_encoding->klass()->const_find(env, "ConverterNotFoundError"_s).value().as_class(); auto to_name = dst_encoding.to_s(env)->string(); auto from_name = src_encoding.to_s(env)->string(); env->raise(klass, "code converter not found ({} to {})", from_name, to_name); @@ -3338,7 +3338,7 @@ Value StringObject::rstrip(Env *env) const { return new StringObject { "", m_encoding }; if (!valid_encoding()) { - env->raise(m_encoding->klass()->const_find(env, "CompatibilityError"_s).as_class(), "invalid byte sequence in {}", m_encoding->name()->string()); + env->raise(m_encoding->klass()->const_find(env, "CompatibilityError"_s).value().as_class(), "invalid byte sequence in {}", m_encoding->name()->string()); } assert(length() < NAT_INT_MAX); @@ -3363,7 +3363,7 @@ Value StringObject::rstrip_in_place(Env *env) { return Value::nil(); if (!valid_encoding()) - env->raise(m_encoding->klass()->const_find(env, "CompatibilityError"_s).as_class(), "invalid byte sequence in {}", m_encoding->name()->string()); + env->raise(m_encoding->klass()->const_find(env, "CompatibilityError"_s).value().as_class(), "invalid byte sequence in {}", m_encoding->name()->string()); assert(length() < NAT_INT_MAX); nat_int_t last_char; From 5b74a74e70483786b46ccaac3113918e29418a76 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 07:22:24 -0600 Subject: [PATCH 08/23] Move Kernel#send and #public_send to KernelModule --- include/natalie/kernel_module.hpp | 2 ++ include/natalie/object.hpp | 9 --------- lib/natalie/compiler/binding_gen.rb | 6 +++--- src/kernel_module.cpp | 10 ++++++++++ src/object.cpp | 10 ---------- 5 files changed, 15 insertions(+), 22 deletions(-) diff --git a/include/natalie/kernel_module.hpp b/include/natalie/kernel_module.hpp index 8f119d7eb..f9e41bd4d 100644 --- a/include/natalie/kernel_module.hpp +++ b/include/natalie/kernel_module.hpp @@ -92,10 +92,12 @@ class KernelModule { static Value private_methods(Env *env, Value self, Optional recur = {}); static Value protected_methods(Env *env, Value self, Optional recur = {}); static Value public_methods(Env *env, Value self, Optional recur = {}); + static Value public_send(Env *, Value, Args &&, Block *); static Value remove_instance_variable(Env *env, Value self, Value name_val); static bool respond_to_missing(Env *, Value, Value, Value) { return false; } static bool respond_to_method(Env *, Value, Value, Optional); static bool respond_to_method(Env *, Value, Value, bool); + static Value send(Env *, Value, Args &&, Block *); static Value tap(Env *env, Value self, Block *block); }; diff --git a/include/natalie/object.hpp b/include/natalie/object.hpp index cb4c30717..c7dd0bcf7 100644 --- a/include/natalie/object.hpp +++ b/include/natalie/object.hpp @@ -149,16 +149,7 @@ class Object : public Cell { } Value public_send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Optional sent_from = {}); - static Value public_send(Env *, Value, Args &&, Block *); - Value send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Optional sent_from = {}); - static Value send(Env *, Value, Args &&, Block *); - - Value send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Optional sent_from = {}) { - // NOTE: sent_from is unused, but accepting it makes the SendInstruction codegen simpler. :-) - return send(env, name, Args(args), block); - } - Value send(Env *, SymbolObject *, Args &&, Block *, MethodVisibility, Optional = {}); Value method_missing_send(Env *, SymbolObject *, Args &&, Block *); static Value method_missing(Env *, Value, Args &&, Block *); diff --git a/lib/natalie/compiler/binding_gen.rb b/lib/natalie/compiler/binding_gen.rb index 55b24ab94..d7f5da1a1 100644 --- a/lib/natalie/compiler/binding_gen.rb +++ b/lib/natalie/compiler/binding_gen.rb @@ -481,7 +481,7 @@ def generate_name gen.static_binding_as_instance_method('BasicObject', '__id__', 'Object', 'object_id', argc: 0, pass_env: false, pass_block: false, return_type: :int) gen.static_binding_as_instance_method('BasicObject', 'equal?', 'Object', 'equal', argc: 1, pass_env: false, pass_block: false, return_type: :bool) -gen.static_binding_as_instance_method('BasicObject', '__send__', 'Object', 'send', argc: 1.., pass_env: true, pass_block: true, return_type: :Object) +gen.static_binding_as_instance_method('BasicObject', '__send__', 'KernelModule', 'send', argc: 1.., pass_env: true, pass_block: true, return_type: :Object) gen.static_binding_as_instance_method('BasicObject', '!', 'Object', 'not_truthy', argc: 0, pass_env: false, pass_block: false, return_type: :bool) gen.static_binding_as_instance_method('BasicObject', '==', 'Object', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :bool) gen.static_binding_as_instance_method('BasicObject', '!=', 'Object', 'neq', argc: 1, pass_env: true, pass_block: false, return_type: :bool) @@ -1031,8 +1031,8 @@ def generate_name gen.static_binding_as_instance_method('Kernel', 'singleton_class', 'Object', 'singleton_class', argc: 0, pass_env: true, pass_block: false, return_type: :Object) gen.static_binding_as_instance_method('Kernel', 'tap', 'KernelModule', 'tap', argc: 0, pass_env: true, pass_block: true, return_type: :Object) gen.static_binding_as_instance_method('Kernel', 'to_s', 'KernelModule', 'inspect', argc: 0, pass_env: true, pass_block: false, return_type: :Object) -gen.static_binding_as_instance_method('Kernel', 'send', 'Object', 'send', argc: 1.., pass_env: true, pass_block: true, return_type: :Object) -gen.static_binding_as_instance_method('Kernel', 'public_send', 'Object', 'public_send', argc: 1.., pass_env: true, pass_block: true, return_type: :Object) +gen.static_binding_as_instance_method('Kernel', 'send', 'KernelModule', 'send', argc: 1.., pass_env: true, pass_block: true, return_type: :Object) +gen.static_binding_as_instance_method('Kernel', 'public_send', 'KernelModule', 'public_send', argc: 1.., pass_env: true, pass_block: true, return_type: :Object) gen.static_binding_as_instance_method('Kernel', 'clone', 'Object', 'clone_obj', argc: 0, kwargs: [:freeze], pass_env: true, pass_block: false, return_type: :Object) gen.static_binding_as_instance_method('Kernel', 'extend', 'KernelModule', 'extend', argc: 1.., pass_env: true, pass_block: false, return_type: :Object) gen.static_binding_as_instance_method('Kernel', 'frozen?', 'KernelModule', 'is_frozen', argc: 0, pass_env: false, pass_block: false, return_type: :bool) diff --git a/src/kernel_module.cpp b/src/kernel_module.cpp index 37f759902..c525994ab 100644 --- a/src/kernel_module.cpp +++ b/src/kernel_module.cpp @@ -1104,6 +1104,11 @@ Value KernelModule::public_methods(Env *env, Value self, Optional recur) return self.klass()->public_instance_methods(env, recur); } +Value KernelModule::public_send(Env *env, Value self, Args &&args, Block *block) { + auto name = args.shift().to_symbol(env, Value::Conversion::Strict); + return self.public_send(env->caller(), name, std::move(args), block); +} + Value KernelModule::remove_instance_variable(Env *env, Value self, Value name_val) { auto name = Object::to_instance_variable_name(env, name_val); self.assert_not_frozen(env); @@ -1143,6 +1148,11 @@ bool KernelModule::respond_to_method(Env *env, Value self, Value name_val, bool } } +Value KernelModule::send(Env *env, Value self, Args &&args, Block *block) { + auto name = args.shift().to_symbol(env, Value::Conversion::Strict); + return self.send(env->caller(), name, std::move(args), block); +} + Value KernelModule::tap(Env *env, Value self, Block *block) { Value args[] = { self }; if (!block) diff --git a/src/object.cpp b/src/object.cpp index 081a274af..c2b8c63ff 100644 --- a/src/object.cpp +++ b/src/object.cpp @@ -540,20 +540,10 @@ Value Object::public_send(Env *env, SymbolObject *name, Args &&args, Block *bloc return send(env, name, std::move(args), block, MethodVisibility::Public, sent_from); } -Value Object::public_send(Env *env, Value self, Args &&args, Block *block) { - auto name = args.shift().to_symbol(env, Value::Conversion::Strict); - return self.public_send(env->caller(), name, std::move(args), block); -} - Value Object::send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { return send(env, name, std::move(args), block, MethodVisibility::Private, sent_from); } -Value Object::send(Env *env, Value self, Args &&args, Block *block) { - auto name = args.shift().to_symbol(env, Value::Conversion::Strict); - return self.send(env->caller(), name, std::move(args), block); -} - Value Object::send(Env *env, SymbolObject *name, Args &&args, Block *block, MethodVisibility visibility_at_least, Optional sent_from) { static const auto initialize = SymbolObject::intern("initialize"); Method *method = find_method(env, name, visibility_at_least, sent_from); From 4da5b0065ef847914a879f359c2ded391669b45e Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 07:48:05 -0600 Subject: [PATCH 09/23] Attempt to clean up and optimize send/public_send --- include/natalie/object.hpp | 8 ++++--- include/natalie/value.hpp | 15 ++++++------- .../compiler/instructions/send_instruction.rb | 11 +++++----- src/object.cpp | 9 +------- src/proc_object.cpp | 8 +++---- src/value.cpp | 21 +++++++++++++------ 6 files changed, 39 insertions(+), 33 deletions(-) diff --git a/include/natalie/object.hpp b/include/natalie/object.hpp index c7dd0bcf7..92fa2c4ed 100644 --- a/include/natalie/object.hpp +++ b/include/natalie/object.hpp @@ -148,9 +148,11 @@ class Object : public Cell { return buf; } - Value public_send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Optional sent_from = {}); - Value send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Optional sent_from = {}); - Value send(Env *, SymbolObject *, Args &&, Block *, MethodVisibility, Optional = {}); + Value public_send(Env *env, SymbolObject *name, Args &&args = {}, Block *block = nullptr, Optional sent_from = {}) { + return send(env, name, std::move(args), block, MethodVisibility::Public, sent_from); + } + + Value send(Env *, SymbolObject *, Args && = {}, Block * = nullptr, MethodVisibility = MethodVisibility::Private, Optional = {}); Value method_missing_send(Env *, SymbolObject *, Args &&, Block *); static Value method_missing(Env *, Value, Args &&, Block *); diff --git a/include/natalie/value.hpp b/include/natalie/value.hpp index 88ff55412..7831a69c0 100644 --- a/include/natalie/value.hpp +++ b/include/natalie/value.hpp @@ -68,19 +68,20 @@ class Value { bool operator!() const { return is_null(); } operator bool() const { return !is_null(); } - Value public_send(Env *, SymbolObject *, Args && = {}, Block * = nullptr, Optional sent_from = {}); + Value public_send(Env *, SymbolObject *, Args &&, Block *, Value sent_from); + Value public_send(Env *, SymbolObject *, Args && = {}, Block * = nullptr); - Value public_send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Optional sent_from = {}) { - return public_send(env, name, Args(args), block, sent_from); + Value public_send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr) { + return public_send(env, name, Args(args), block); } - Value send(Env *, SymbolObject *, Args && = {}, Block * = nullptr, Optional sent_from = {}); + Value send(Env *, SymbolObject *, Args && = {}, Block * = nullptr); - Value send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr, Optional sent_from = {}) { - return send(env, name, Args(args), block, sent_from); + Value send(Env *env, SymbolObject *name, std::initializer_list args, Block *block = nullptr) { + return send(env, name, Args(args), block); } - Value immediate_send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from, MethodVisibility visibility); + Value immediate_send(Env *env, SymbolObject *name, Args &&args, Block *block, MethodVisibility visibility); ClassObject *klass() const; diff --git a/lib/natalie/compiler/instructions/send_instruction.rb b/lib/natalie/compiler/instructions/send_instruction.rb index 1ef19295d..122a488d8 100644 --- a/lib/natalie/compiler/instructions/send_instruction.rb +++ b/lib/natalie/compiler/instructions/send_instruction.rb @@ -58,17 +58,17 @@ def to_s def generate(transform) if @args_array_on_stack if @forward_args - args_list = "std::move(#{transform.pop})" + args_list = ["std::move(#{transform.pop})"] else args = "#{transform.pop}.as_array()" - args_list = "Args(#{args}, #{@has_keyword_hash ? 'true' : 'false'})" + args_list = ["Args(#{args}, #{@has_keyword_hash ? 'true' : 'false'})"] end else arg_count = transform.pop raise "bad argc #{arg_count.inspect} for SendInstruction #{@message.inspect}" unless arg_count.is_a?(Integer) args = [] arg_count.times { args.unshift transform.pop } - args_list = "Args({ #{args.join(', ')} }, #{@has_keyword_hash ? 'true' : 'false'})" + args_list = ["Args({ #{args.join(', ')} }, #{@has_keyword_hash ? 'true' : 'false'})"] end transform.set_file(@file) @@ -82,9 +82,10 @@ def generate(transform) exit 1 end - block = @with_block ? "to_block(env, #{transform.pop})" : 'nullptr' + args_list << (@with_block ? "to_block(env, #{transform.pop})" : 'nullptr') + args_list << 'self' if method == :public_send - call = "#{receiver}.#{method}(env, #{transform.intern(@message)}, #{args_list}, #{block}, self)" + call = "#{receiver}.#{method}(env, #{transform.intern(@message)}, #{args_list.join(', ')})" if message =~ /\w=$|\[\]=$/ transform.exec(call) diff --git a/src/object.cpp b/src/object.cpp index c2b8c63ff..7def750df 100644 --- a/src/object.cpp +++ b/src/object.cpp @@ -1,5 +1,6 @@ #include "natalie.hpp" #include "natalie/forward.hpp" +#include "natalie/method_visibility.hpp" #include namespace Natalie { @@ -536,14 +537,6 @@ Value Object::module_function(Env *env, Args &&args) { abort(); } -Value Object::public_send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { - return send(env, name, std::move(args), block, MethodVisibility::Public, sent_from); -} - -Value Object::send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { - return send(env, name, std::move(args), block, MethodVisibility::Private, sent_from); -} - Value Object::send(Env *env, SymbolObject *name, Args &&args, Block *block, MethodVisibility visibility_at_least, Optional sent_from) { static const auto initialize = SymbolObject::intern("initialize"); Method *method = find_method(env, name, visibility_at_least, sent_from); diff --git a/src/proc_object.cpp b/src/proc_object.cpp index e4bb2b2de..7aad7eef3 100644 --- a/src/proc_object.cpp +++ b/src/proc_object.cpp @@ -29,15 +29,15 @@ bool ProcObject::equal_value(Value other) const { static Value compose_ltlt(Env *env, Value self, Args &&args, Block *block) { auto block_var = ProcObject::from_block_maybe(block); auto call = "call"_s; - auto other_call_result = env->outer()->var_get("other", 0).send(env, call, std::move(args), to_block(env, block_var), self); - return self.send(env, call, { other_call_result }, nullptr, self); + auto other_call_result = env->outer()->var_get("other", 0).send(env, call, std::move(args), to_block(env, block_var)); + return self.send(env, call, { other_call_result }); } static Value compose_gtgt(Env *env, Value self, Args &&args, Block *block) { auto block_var = ProcObject::from_block_maybe(block); auto call = "call"_s; - auto self_call_result = self.send(env, call, std::move(args), to_block(env, block_var), self); - return env->outer()->var_get("other", 0).send(env, call, { self_call_result }, nullptr, self); + auto self_call_result = self.send(env, call, std::move(args), to_block(env, block_var)); + return env->outer()->var_get("other", 0).send(env, call, { self_call_result }); } Value ProcObject::ltlt(Env *env, Value other) { diff --git a/src/value.cpp b/src/value.cpp index 7bd575eed..b612a918e 100644 --- a/src/value.cpp +++ b/src/value.cpp @@ -42,7 +42,7 @@ Value Value::integer(TM::String &&str) { }); // FIXME: this doesn't check method visibility, but no tests are failing yet :-) -Value Value::immediate_send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from, MethodVisibility visibility) { +Value Value::immediate_send(Env *env, SymbolObject *name, Args &&args, Block *block, MethodVisibility visibility) { auto method_info = klass()->find_method(env, name); if (!method_info.is_defined()) { // FIXME: store missing reason on current thread @@ -130,22 +130,31 @@ StringObject *Value::to_str2(Env *env) { result.klass()->inspect_str()); } -Value Value::public_send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { +Value Value::public_send(Env *env, SymbolObject *name, Args &&args, Block *block, Value sent_from) { PROFILED_SEND(NativeProfilerEvent::Type::PUBLIC_SEND); if (is_integer() || is_nil()) - return immediate_send(env, name, std::move(args), block, sent_from, MethodVisibility::Public); + return immediate_send(env, name, std::move(args), block, MethodVisibility::Public); return object()->public_send(env, name, std::move(args), block, sent_from); } -Value Value::send(Env *env, SymbolObject *name, Args &&args, Block *block, Optional sent_from) { +Value Value::public_send(Env *env, SymbolObject *name, Args &&args, Block *block) { + PROFILED_SEND(NativeProfilerEvent::Type::PUBLIC_SEND); + + if (is_integer() || is_nil()) + return immediate_send(env, name, std::move(args), block, MethodVisibility::Public); + + return object()->public_send(env, name, std::move(args), block); +} + +Value Value::send(Env *env, SymbolObject *name, Args &&args, Block *block) { PROFILED_SEND(NativeProfilerEvent::Type::SEND); if (is_integer() || is_nil()) - return immediate_send(env, name, std::move(args), block, sent_from, MethodVisibility::Private); + return immediate_send(env, name, std::move(args), block, MethodVisibility::Private); - return object()->send(env, name, std::move(args), block, sent_from); + return object()->send(env, name, std::move(args), block); } Integer Value::integer() const { From 32fcc122fd634f8396fec9ef03f12b0ad510ad27 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 08:10:03 -0600 Subject: [PATCH 10/23] Add rake task test_perf_quickly This doesn't run the test on the self-hosted compiler, so it runs much quicker. --- Rakefile | 4 ++++ spec/support/test_perf.rb | 13 +++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Rakefile b/Rakefile index 519826520..2b795d471 100644 --- a/Rakefile +++ b/Rakefile @@ -133,6 +133,10 @@ task test_perf: [:build_release, 'bin/nat'] do sh 'ruby spec/support/test_perf.rb' end +task test_perf_quickly: [:build_release] do + sh 'ruby spec/support/test_perf.rb --quickly' +end + task output_all_ruby_specs: :build do version = RUBY_VERSION.sub(/\.\d+$/, '') sh <<~END diff --git a/spec/support/test_perf.rb b/spec/support/test_perf.rb index 165bd5e20..8bb27f9d0 100644 --- a/spec/support/test_perf.rb +++ b/spec/support/test_perf.rb @@ -5,13 +5,18 @@ require 'net/http' require 'uri' -totals = {} -{ +TESTS = { hello: '/tmp/hello', fib: '/tmp/fib', boardslam: '/tmp/boardslam 3 5 1', - nat: 'bin/nat -c /tmp/bs examples/boardslam.rb', -}.each do |name, command| +} + +unless ARGV.first == '--quickly' + TESTS[:nat] = 'bin/nat -c /tmp/bs examples/boardslam.rb' +end + +totals = {} +TESTS.each do |name, command| unless name == :nat system "bin/natalie -c /tmp/#{name} examples/#{name}.rb" end From b34f4ab2bdeab6bdf9130cc92460eea8667e4f3e Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 19:55:25 -0600 Subject: [PATCH 11/23] Don't store nullptr Value on ExceptionObject --- include/natalie/exception_object.hpp | 14 ++++---------- src/exception_object.cpp | 4 ++-- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/include/natalie/exception_object.hpp b/include/natalie/exception_object.hpp index 48bb64f52..155dc8cc2 100644 --- a/include/natalie/exception_object.hpp +++ b/include/natalie/exception_object.hpp @@ -57,13 +57,7 @@ class ExceptionObject : public Object { virtual void visit_children(Visitor &) const override final; virtual void gc_inspect(char *buf, size_t len) const override { - if (m_message == nullptr) { - snprintf(buf, len, "", this); - // } else if (m_message.type() == Object::Type::String) { - // snprintf(buf, len, "", this, m_message.as_string()->c_str()); - } else { - snprintf(buf, len, "", this); - } + snprintf(buf, len, "", this); } void set_local_jump_error_type(LocalJumpErrorType type) { m_local_jump_error_type = type; } @@ -78,10 +72,10 @@ class ExceptionObject : public Object { private: ArrayObject *generate_backtrace(); - Value m_message { nullptr }; + Value m_message { Value::nil() }; Backtrace *m_backtrace { nullptr }; - Value m_backtrace_value { nullptr }; - Value m_backtrace_locations { nullptr }; + ArrayObject *m_backtrace_value { nullptr }; + ArrayObject *m_backtrace_locations { nullptr }; ExceptionObject *m_cause { nullptr }; nat_int_t m_break_point { 0 }; LocalJumpErrorType m_local_jump_error_type { LocalJumpErrorType::None }; diff --git a/src/exception_object.cpp b/src/exception_object.cpp index 1391a9dbf..a004619e2 100644 --- a/src/exception_object.cpp +++ b/src/exception_object.cpp @@ -112,7 +112,7 @@ Value ExceptionObject::inspect(Env *env) { } StringObject *ExceptionObject::to_s(Env *env) { - if (m_message == nullptr || m_message.is_nil()) { + if (m_message.is_nil()) { return new StringObject { m_klass->inspect_str() }; } else if (m_message.is_string()) { return m_message.as_string(); @@ -172,7 +172,7 @@ Value ExceptionObject::set_backtrace(Env *env, Value backtrace) { if (!element.is_string()) env->raise("TypeError", "backtrace must be Array of String"); } - m_backtrace_value = backtrace; + m_backtrace_value = backtrace.as_array(); } else if (backtrace.is_string()) { m_backtrace_value = new ArrayObject { backtrace }; } else if (backtrace.is_nil()) { From 691965e0e701557848072c532b47131e679781c0 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 19:56:45 -0600 Subject: [PATCH 12/23] Remove nullptr from HashKey Values --- include/natalie/hash_object.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/natalie/hash_object.hpp b/include/natalie/hash_object.hpp index 443340c50..4be8cd6cb 100644 --- a/include/natalie/hash_object.hpp +++ b/include/natalie/hash_object.hpp @@ -16,8 +16,8 @@ namespace Natalie { struct HashKey : public Cell { HashKey *prev { nullptr }; HashKey *next { nullptr }; - Value key { nullptr }; - Value val { nullptr }; + Value key; + Value val; size_t hash { 0 }; bool removed { false }; From db6835ff935460b9e0292ead6af77164eaad6789 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 19:57:19 -0600 Subject: [PATCH 13/23] Remove some nullptrs from local variables in main and fiber --- src/fiber_object.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fiber_object.cpp b/src/fiber_object.cpp index efc5026cc..8fc207532 100644 --- a/src/fiber_object.cpp +++ b/src/fiber_object.cpp @@ -385,7 +385,7 @@ void fiber_wrapper_func(mco_coro *co) { } assert(fiber->block()); - Natalie::Value return_arg = nullptr; + Natalie::Value return_arg; bool reraise = false; try { From 506e5cc3bf64e60bc80d6a6461538ce27191401c Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 21:04:07 -0600 Subject: [PATCH 14/23] Don't return nullptr from EVAL --- lib/natalie/repl.rb | 8 ++++---- src/main.cpp | 13 ++++++------- test/natalie/libnat_test.rb | 7 ++++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/natalie/repl.rb b/lib/natalie/repl.rb index 3f9396447..fa95ba79e 100644 --- a/lib/natalie/repl.rb +++ b/lib/natalie/repl.rb @@ -129,14 +129,14 @@ def self.init library = Module.new do extend FFI::Library ffi_lib temp.path - attach_function :EVAL, [:pointer, :pointer], :pointer + attach_function :EVAL, [:pointer, :pointer], :int end @env ||= FFI::Pointer.new_env - result_ptr = library.EVAL(@env, @result_memory) + status = library.EVAL(@env, @result_memory) - unless result_ptr.null? - p result_ptr.to_obj + if status.zero? + p @result_memory.to_obj end File.unlink(temp.path) diff --git a/src/main.cpp b/src/main.cpp index c9f39e10a..5ac5215fe 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -15,7 +15,7 @@ extern "C" Env *build_top_env() { return env; } -extern "C" Value *EVAL(Env *env, Value *result_memory) { +extern "C" int EVAL(Env *env, Value *result_memory) { /*NAT_EVAL_INIT*/ [[maybe_unused]] Value self = GlobalEnv::the()->main_obj(); @@ -25,7 +25,7 @@ extern "C" Value *EVAL(Env *env, Value *result_memory) { Args args; Block *block = nullptr; - Value result = nullptr; + Value result = Value::nil(); try { // FIXME: top-level `return` in a Ruby script should probably be changed to `exit`. result = [&]() -> Value { @@ -36,10 +36,10 @@ extern "C" Value *EVAL(Env *env, Value *result_memory) { run_at_exit_handlers(env); } catch (ExceptionObject *exception) { handle_top_level_exception(env, exception, run_exit_handlers); - result = nullptr; + return 1; } memcpy(result_memory, &result, sizeof(Value)); - return result_memory; + return 0; } int main(int argc, char *argv[]) { @@ -77,9 +77,8 @@ int main(int argc, char *argv[]) { ARGV->push(new StringObject { argv[i] }); } - Value result = nullptr; - EVAL(env, &result); - auto return_code = result ? 0 : 1; + Value result = Value::nil(); + auto return_code = EVAL(env, &result); #ifdef NAT_NATIVE_PROFILER NativeProfiler::the()->dump(); diff --git a/test/natalie/libnat_test.rb b/test/natalie/libnat_test.rb index 75641d4cd..753e22d54 100644 --- a/test/natalie/libnat_test.rb +++ b/test/natalie/libnat_test.rb @@ -55,13 +55,14 @@ def self.compile(ast, path, encoding) library = Module.new do extend FFI::Library ffi_lib(temp_path) - attach_function :EVAL, [:pointer, :pointer], :pointer + attach_function :EVAL, [:pointer, :pointer], :int end env = FFI::Pointer.from_env result_memory = FFI::Pointer.new_value - result = library.EVAL(env, result_memory).to_obj - result.should == 3 + status = library.EVAL(env, result_memory) + status.should == 0 + result_memory.to_obj.should == 3 ensure File.unlink(temp_path) end From 45017140900454421ce126d70bce03da1dbef173 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 22:15:47 -0600 Subject: [PATCH 15/23] Return Optional from HashObject::remove() --- include/natalie/hash_object.hpp | 2 +- lib/natalie/compiler/binding_gen.rb | 6 +-- .../instructions/hash_delete_instruction.rb | 2 +- lib/openssl.cpp | 26 ++++++------- lib/socket.cpp | 20 +++++----- src/exception_object.cpp | 7 ++-- src/hash_object.cpp | 8 ++-- src/ioutil.cpp | 37 +++++++++++-------- src/string_object.cpp | 22 +++++------ 9 files changed, 65 insertions(+), 65 deletions(-) diff --git a/include/natalie/hash_object.hpp b/include/natalie/hash_object.hpp index 4be8cd6cb..8ac25b323 100644 --- a/include/natalie/hash_object.hpp +++ b/include/natalie/hash_object.hpp @@ -114,7 +114,7 @@ class HashObject : public Object { Value set_default(Env *, Value); void put(Env *, Value, Value); - Value remove(Env *, Value); + Optional remove(Env *, Value); Value clear(Env *); Value default_proc(Env *); diff --git a/lib/natalie/compiler/binding_gen.rb b/lib/natalie/compiler/binding_gen.rb index d7f5da1a1..4f5a266ba 100644 --- a/lib/natalie/compiler/binding_gen.rb +++ b/lib/natalie/compiler/binding_gen.rb @@ -281,11 +281,7 @@ def pop_kwargs "auto kwargs = args.pop_keyword_hash();", ].tap do |lines| @kwargs.each do |kw| - lines << "Optional kwarg_#{kw};" - lines << 'if (kwargs) {' - lines << " auto value_or_null = kwargs->remove(env, #{kw.to_s.inspect}_s);" - lines << " if (value_or_null != nullptr) kwarg_#{kw} = value_or_null;" - lines << '}' + lines << "auto kwarg_#{kw} = kwargs ? kwargs->remove(env, #{kw.to_s.inspect}_s) : Optional();" end end.join("\n") when true diff --git a/lib/natalie/compiler/instructions/hash_delete_instruction.rb b/lib/natalie/compiler/instructions/hash_delete_instruction.rb index e32a0d300..6c4dfd908 100644 --- a/lib/natalie/compiler/instructions/hash_delete_instruction.rb +++ b/lib/natalie/compiler/instructions/hash_delete_instruction.rb @@ -13,7 +13,7 @@ def to_s def generate(transform) hash = transform.peek - transform.exec_and_push(@name, "#{hash}.as_hash()->remove(env, #{transform.intern(@name)})") + transform.exec_and_push(@name, "#{hash}.as_hash()->remove(env, #{transform.intern(@name)}).value()") end def execute(vm) diff --git a/lib/openssl.cpp b/lib/openssl.cpp index 6b265418e..51fd4f8b6 100644 --- a/lib/openssl.cpp +++ b/lib/openssl.cpp @@ -964,10 +964,10 @@ Value OpenSSL_KDF_pbkdf2_hmac(Env *env, Value self, Args &&args, Block *) { auto pass = args.at(0).to_str(env); if (!kwargs) kwargs = new HashObject {}; env->ensure_no_missing_keywords(kwargs, { "salt", "iterations", "length", "hash" }); - auto salt = kwargs->remove(env, "salt"_s).to_str(env); - auto iterations = kwargs->remove(env, "iterations"_s).to_int(env); - auto length = kwargs->remove(env, "length"_s).to_int(env); - auto hash = kwargs->remove(env, "hash"_s); + auto salt = kwargs->remove(env, "salt"_s).value().to_str(env); + auto iterations = kwargs->remove(env, "iterations"_s).value().to_int(env); + auto length = kwargs->remove(env, "length"_s).value().to_int(env); + auto hash = kwargs->remove(env, "hash"_s).value(); auto digest_klass = GlobalEnv::the()->Object()->const_get("OpenSSL"_s).as_module()->const_get("Digest"_s).as_module(); if (!hash.is_a(env, digest_klass)) hash = Object::_new(env, digest_klass, { hash }, nullptr); @@ -999,11 +999,11 @@ Value OpenSSL_KDF_scrypt(Env *env, Value self, Args &&args, Block *) { args.ensure_argc_is(env, 1); auto pass = args.at(0).to_str(env); env->ensure_no_missing_keywords(kwargs, { "salt", "N", "r", "p", "length" }); - auto salt = kwargs->remove(env, "salt"_s).to_str(env); - auto N = kwargs->remove(env, "N"_s).to_int(env); - auto r = kwargs->remove(env, "r"_s).to_int(env); - auto p = kwargs->remove(env, "p"_s).to_int(env); - auto length = kwargs->remove(env, "length"_s).to_int(env); + auto salt = kwargs->remove(env, "salt"_s).value().to_str(env); + auto N = kwargs->remove(env, "N"_s).value().to_int(env); + auto r = kwargs->remove(env, "r"_s).value().to_int(env); + auto p = kwargs->remove(env, "p"_s).value().to_int(env); + auto length = kwargs->remove(env, "length"_s).value().to_int(env); if (length.is_negative() || length.is_bignum()) env->raise("ArgumentError", "negative string size (or size too big)"); env->ensure_no_extra_keywords(kwargs); @@ -1183,8 +1183,8 @@ Value OpenSSL_Random_random_bytes(Env *env, Value self, Args &&args, Block *) { Value OpenSSL_X509_Name_add_entry(Env *env, Value self, Args &&args, Block *) { auto kwargs = args.pop_keyword_hash(); - auto kwarg_loc = kwargs ? kwargs->remove(env, "loc"_s) : nullptr; - auto kwarg_set = kwargs ? kwargs->remove(env, "set"_s) : nullptr; + auto kwarg_loc = kwargs ? kwargs->remove(env, "loc"_s) : Optional(); + auto kwarg_set = kwargs ? kwargs->remove(env, "set"_s) : Optional(); env->ensure_no_extra_keywords(kwargs); args.ensure_argc_between(env, 2, 3); auto oid = args.at(0).to_str(env); @@ -1197,8 +1197,8 @@ Value OpenSSL_X509_Name_add_entry(Env *env, Value self, Args &&args, Block *) { type = OBJECT_TYPE_TEMPLATE->ref(env, oid); } auto name = static_cast(self->ivar_get(env, "@name"_s).as_void_p()->void_ptr()); - int loc = kwarg_loc && !kwarg_loc.is_nil() ? IntegerMethods::convert_to_nat_int_t(env, kwarg_loc) : -1; - int set = kwarg_set && !kwarg_set.is_nil() ? IntegerMethods::convert_to_nat_int_t(env, kwarg_set) : 0; + int loc = kwarg_loc && !kwarg_loc.value().is_nil() ? IntegerMethods::convert_to_nat_int_t(env, kwarg_loc.value()) : -1; + int set = kwarg_set && !kwarg_set.value().is_nil() ? IntegerMethods::convert_to_nat_int_t(env, kwarg_set.value()) : 0; if (!X509_NAME_add_entry_by_txt(name, oid->c_str(), IntegerMethods::convert_to_nat_int_t(env, type), reinterpret_cast(value->c_str()), value->bytesize(), loc, set)) OpenSSL_X509_Name_raise_error(env, "X509_NAME_add_entry_by_txt"); return self; diff --git a/lib/socket.cpp b/lib/socket.cpp index 918467450..dfc1653a3 100644 --- a/lib/socket.cpp +++ b/lib/socket.cpp @@ -554,7 +554,7 @@ Value BasicSocket_recv_nonblock(Env *env, Value self, Args &&args, Block *) { const auto maxlen = IntegerMethods::convert_to_nat_int_t(env, args[0]); const auto flags = IntegerMethods::convert_to_nat_int_t(env, args.at(1, Value::integer(0))); auto buffer = args.at(2, new StringObject { "", Encoding::ASCII_8BIT }).to_str(env); - auto exception = kwargs ? kwargs->remove(env, "exception"_s) : TrueObject::the(); + auto exception = kwargs ? kwargs->remove(env, "exception"_s) : Value(TrueObject::the()); env->ensure_no_extra_keywords(kwargs); #ifdef __APPLE__ @@ -568,7 +568,7 @@ Value BasicSocket_recv_nonblock(Env *env, Value self, Args &&args, Block *) { nullptr, nullptr); if (recvfrom_result < 0) { if (errno == EWOULDBLOCK || errno == EAGAIN) { - if (exception.is_falsey()) + if (exception && exception.value().is_falsey()) return "wait_readable"_s; auto SystemCallError = find_top_level_const(env, "SystemCallError"_s); ExceptionObject *error = SystemCallError.send(env, "exception"_s, { Value::integer(errno) }).as_exception(); @@ -996,10 +996,10 @@ Value Socket_accept(Env *env, Value self, Args &&args, Block *block) { Value Socket_accept_nonblock(Env *env, Value self, Args &&args, Block *block) { auto kwargs = args.pop_keyword_hash(); - auto exception = kwargs ? kwargs->remove(env, "exception"_s) : TrueObject::the(); + auto exception = kwargs ? kwargs->remove(env, "exception"_s) : Value(TrueObject::the()); env->ensure_no_extra_keywords(kwargs); args.ensure_argc_is(env, 0); - return Socket_accept(env, self, false, exception.is_truthy()); + return Socket_accept(env, self, false, exception.value().is_truthy()); } Value Socket_bind(Env *env, Value self, Args &&args, Block *block) { @@ -1674,7 +1674,7 @@ Value TCPServer_accept(Env *env, Value self, Args &&args, Block *) { Value TCPServer_accept_nonblock(Env *env, Value self, Args &&args, Block *) { auto kwargs = args.pop_keyword_hash(); - auto exception = kwargs ? kwargs->remove(env, "exception"_s) : TrueObject::the(); + auto exception = kwargs ? kwargs->remove(env, "exception"_s) : Value(TrueObject::the()); args.ensure_argc_is(env, 0); env->ensure_no_extra_keywords(kwargs); @@ -1683,7 +1683,7 @@ Value TCPServer_accept_nonblock(Env *env, Value self, Args &&args, Block *) { sockaddr_storage addr; socklen_t len = sizeof(addr); - return Server_accept(env, self, "TCPSocket"_s, addr, len, false, exception.is_truthy()); + return Server_accept(env, self, "TCPSocket"_s, addr, len, false, exception.value().is_truthy()); } Value TCPServer_listen(Env *env, Value self, Args &&args, Block *) { @@ -1733,7 +1733,7 @@ Value UDPSocket_connect(Env *env, Value self, Args &&args, Block *block) { Value UDPSocket_recvfrom_nonblock(Env *env, Value self, Args &&args, Block *) { auto kwargs = args.pop_keyword_hash(); - auto exception = kwargs ? kwargs->remove(env, "exception"_s) : TrueObject::the(); + auto exception = kwargs ? kwargs->remove(env, "exception"_s) : Value(TrueObject::the()); args.ensure_argc_between(env, 1, 3); env->ensure_no_extra_keywords(kwargs); @@ -1751,7 +1751,7 @@ Value UDPSocket_recvfrom_nonblock(Env *env, Value self, Args &&args, Block *) { reinterpret_cast(&addr), &addr_len); if (recvfrom_result < 0) { if (errno == EWOULDBLOCK || errno == EAGAIN) { - if (exception.is_falsey()) + if (exception.value().is_falsey()) return "wait_readable"_s; auto SystemCallError = find_top_level_const(env, "SystemCallError"_s); ExceptionObject *error = SystemCallError.send(env, "exception"_s, { Value::integer(errno) }).as_exception(); @@ -1869,12 +1869,12 @@ Value UNIXServer_accept(Env *env, Value self, Args &&args, Block *) { Value UNIXServer_accept_nonblock(Env *env, Value self, Args &&args, Block *) { auto kwargs = args.pop_keyword_hash(); - auto exception = kwargs ? kwargs->remove(env, "exception"_s) : TrueObject::the(); + auto exception = kwargs ? kwargs->remove(env, "exception"_s) : Value(TrueObject::the()); args.ensure_argc_is(env, 0); env->ensure_no_extra_keywords(kwargs); sockaddr_storage addr; socklen_t len = sizeof(addr); - return Server_accept(env, self, "UNIXSocket"_s, addr, len, false, exception.is_truthy()); + return Server_accept(env, self, "UNIXSocket"_s, addr, len, false, exception.value().is_truthy()); } Value UNIXServer_listen(Env *env, Value self, Args &&args, Block *) { diff --git a/src/exception_object.cpp b/src/exception_object.cpp index a004619e2..3e77bd501 100644 --- a/src/exception_object.cpp +++ b/src/exception_object.cpp @@ -4,7 +4,7 @@ namespace Natalie { ExceptionObject *ExceptionObject::create_for_raise(Env *env, Args &&args, ExceptionObject *current_exception, bool accept_cause) { auto kwargs = args.pop_keyword_hash(); - auto cause = accept_cause && kwargs ? kwargs->remove(env, "cause"_s) : nullptr; + auto cause = accept_cause && kwargs ? kwargs->remove(env, "cause"_s) : Optional(); args.ensure_argc_between(env, 0, 3); auto klass = args.at(0, nullptr); auto message = args.at(1, nullptr); @@ -59,9 +59,8 @@ ExceptionObject *ExceptionObject::create_for_raise(Env *env, Args &&args, Except else env->raise("TypeError", "exception klass/object expected"); - if (accept_cause && cause && cause.is_exception()) { - exception->set_cause(cause.as_exception()); - } + if (accept_cause && cause && cause.value().is_exception()) + exception->set_cause(cause.value().as_exception()); if (backtrace) exception->set_backtrace(env, backtrace); diff --git a/src/hash_object.cpp b/src/hash_object.cpp index 00242e66e..6b15a5c18 100644 --- a/src/hash_object.cpp +++ b/src/hash_object.cpp @@ -110,7 +110,7 @@ void HashObject::put(Env *env, Value key, Value val) { } } -Value HashObject::remove(Env *env, Value key) { +Optional HashObject::remove(Env *env, Value key) { std::lock_guard lock(g_gc_recursive_mutex); HashKey key_container; @@ -124,7 +124,7 @@ Value HashObject::remove(Env *env, Value key) { m_hashmap.remove(&key_container, env); return val; } else { - return nullptr; + return {}; } } @@ -399,9 +399,9 @@ Value HashObject::delete_if(Env *env, Block *block) { Value HashObject::delete_key(Env *env, Value key, Block *block) { assert_not_frozen(env); - Value val = remove(env, key); + auto val = remove(env, key); if (val) - return val; + return val.value(); else if (block) return block->run(env, {}, nullptr); else diff --git a/src/ioutil.cpp b/src/ioutil.cpp index 0cd3eb2c1..c285c92c0 100644 --- a/src/ioutil.cpp +++ b/src/ioutil.cpp @@ -104,23 +104,24 @@ namespace ioutil { void flags_struct::parse_mode(Env *env) { if (!m_kwargs) return; auto mode = m_kwargs->remove(env, "mode"_s); - if (!mode || mode.is_nil()) return; + if (!mode || mode.value().is_nil()) return; if (has_mode()) env->raise("ArgumentError", "mode specified twice"); - parse_flags_obj(env, mode); + parse_flags_obj(env, mode.value()); } void flags_struct::parse_flags(Env *env) { if (!m_kwargs) return; auto flags = m_kwargs->remove(env, "flags"_s); - if (!flags || flags.is_nil()) return; - m_flags |= static_cast(flags.to_int(env).to_nat_int_t()); + if (!flags || flags.value().is_nil()) return; + m_flags |= static_cast(flags.value().to_int(env).to_nat_int_t()); } void flags_struct::parse_encoding(Env *env) { if (!m_kwargs) return; - auto encoding = m_kwargs->remove(env, "encoding"_s); - if (!encoding || encoding.is_nil()) return; + auto encoding_kwarg = m_kwargs->remove(env, "encoding"_s); + if (!encoding_kwarg || encoding_kwarg.value().is_nil()) return; + auto encoding = encoding_kwarg.value(); if (m_external_encoding) { env->raise("ArgumentError", "encoding specified twice"); } else if (m_kwargs->has_key(env, "external_encoding"_s)) { @@ -144,8 +145,9 @@ namespace ioutil { void flags_struct::parse_external_encoding(Env *env) { if (!m_kwargs) return; - auto external_encoding = m_kwargs->remove(env, "external_encoding"_s); - if (!external_encoding || external_encoding.is_nil()) return; + auto external_encoding_kwarg = m_kwargs->remove(env, "external_encoding"_s); + if (!external_encoding_kwarg || external_encoding_kwarg.value().is_nil()) return; + auto external_encoding = external_encoding_kwarg.value(); if (m_external_encoding) env->raise("ArgumentError", "encoding specified twice"); if (external_encoding.is_encoding()) { @@ -157,8 +159,9 @@ namespace ioutil { void flags_struct::parse_internal_encoding(Env *env) { if (!m_kwargs) return; - auto internal_encoding = m_kwargs->remove(env, "internal_encoding"_s); - if (!internal_encoding || internal_encoding.is_nil()) return; + auto internal_encoding_kwarg = m_kwargs->remove(env, "internal_encoding"_s); + if (!internal_encoding_kwarg || internal_encoding_kwarg.value().is_nil()) return; + auto internal_encoding = internal_encoding_kwarg.value(); if (m_internal_encoding) env->raise("ArgumentError", "encoding specified twice"); if (internal_encoding.is_encoding()) { @@ -175,8 +178,9 @@ namespace ioutil { void flags_struct::parse_textmode(Env *env) { if (!m_kwargs) return; - auto textmode = m_kwargs->remove(env, "textmode"_s); - if (!textmode || textmode.is_nil()) return; + auto textmode_kwarg = m_kwargs->remove(env, "textmode"_s); + if (!textmode_kwarg || textmode_kwarg.value().is_nil()) return; + auto textmode = textmode_kwarg.value(); if (binmode()) { env->raise("ArgumentError", "both textmode and binmode specified"); } else if (this->textmode()) { @@ -188,8 +192,9 @@ namespace ioutil { void flags_struct::parse_binmode(Env *env) { if (!m_kwargs) return; - auto binmode = m_kwargs->remove(env, "binmode"_s); - if (!binmode || binmode.is_nil()) return; + auto binmode_kwarg = m_kwargs->remove(env, "binmode"_s); + if (!binmode_kwarg || binmode_kwarg.value().is_nil()) return; + auto binmode = binmode_kwarg.value(); if (this->binmode()) { env->raise("ArgumentError", "binmode specified twice"); } else if (textmode()) { @@ -211,14 +216,14 @@ namespace ioutil { return; } - m_autoclose = autoclose.is_truthy(); + m_autoclose = autoclose.value().is_truthy(); } void flags_struct::parse_path(Env *env) { if (!m_kwargs) return; auto path = m_kwargs->remove(env, "path"_s); if (!path) return; - m_path = convert_using_to_path(env, path); + m_path = convert_using_to_path(env, path.value()); } flags_struct::flags_struct(Env *env, Value flags_obj, HashObject *kwargs) diff --git a/src/string_object.cpp b/src/string_object.cpp index b6f8f9e3a..e67705606 100644 --- a/src/string_object.cpp +++ b/src/string_object.cpp @@ -1391,36 +1391,36 @@ Value StringObject::encode_in_place(Env *env, Optional dst_encoding_arg, auto invalid = kwargs->remove(env, "invalid"_s); if (invalid) { - if (invalid.is_nil()) + if (invalid.value().is_nil()) options.invalid_option = EncodeInvalidOption::Raise; - else if (invalid == "replace"_s) + else if (invalid.value() == "replace"_s) options.invalid_option = EncodeInvalidOption::Replace; } auto undef = kwargs->remove(env, "undef"_s); if (undef) { - if (undef.is_nil()) + if (!undef || undef.value().is_nil()) options.undef_option = EncodeUndefOption::Raise; - else if (undef == "replace"_s) + else if (undef.value() == "replace"_s) options.undef_option = EncodeUndefOption::Replace; } auto replace = kwargs->remove(env, "replace"_s); - if (replace && !replace.is_nil()) - options.replace_option = replace.as_string_or_raise(env)->encode(env, dst_encoding).as_string_or_raise(env); + if (replace && !replace.value().is_nil()) + options.replace_option = replace.value().as_string_or_raise(env)->encode(env, dst_encoding).as_string_or_raise(env); auto fallback = kwargs->remove(env, "fallback"_s); - if (fallback && !fallback.is_nil()) - options.fallback_option = fallback; + if (fallback && !fallback.value().is_nil()) + options.fallback_option = fallback.value(); auto xml = kwargs->remove(env, "xml"_s); if (xml) { - if (xml == "attr"_s) + if (xml.value() == "attr"_s) options.xml_option = EncodeXmlOption::Attr; - else if (xml == "text"_s) + else if (xml.value() == "text"_s) options.xml_option = EncodeXmlOption::Text; else - env->raise("ArgumentError", "unexpected value for xml option: {}", xml.inspect_str(env)); + env->raise("ArgumentError", "unexpected value for xml option: {}", xml.value().inspect_str(env)); } } From ff988e464ae62e2ed06f1b4098fc23fab481df58 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 22:16:30 -0600 Subject: [PATCH 16/23] Add Args::maybe_at() that returns Optional The name isn't my favorite... subject to change. :-) --- include/natalie/args.hpp | 2 ++ src/args.cpp | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/include/natalie/args.hpp b/include/natalie/args.hpp index f67e05685..d23439227 100644 --- a/include/natalie/args.hpp +++ b/include/natalie/args.hpp @@ -1,6 +1,7 @@ #pragma once #include "tm/macros.hpp" +#include "tm/optional.hpp" #include "tm/string.hpp" #include "tm/vector.hpp" #include @@ -54,6 +55,7 @@ class Args { Value at(size_t index) const; Value at(size_t index, Value default_value) const; + Optional maybe_at(size_t index) const; ArrayObject *to_array() const; ArrayObject *to_array_for_block(Env *env, ssize_t min_count, ssize_t max_count, bool spread) const; diff --git a/src/args.cpp b/src/args.cpp index 037007780..6adce25a4 100644 --- a/src/args.cpp +++ b/src/args.cpp @@ -11,9 +11,16 @@ Value Args::at(size_t index) const { } Value Args::at(size_t index, Value default_value) const { + auto result = maybe_at(index); + if (result) + return result.value(); + return default_value; +} + +Optional Args::maybe_at(size_t index) const { auto offset = m_args_start_index + index; if (offset >= tl_current_arg_stack->size() || offset >= m_args_start_index + m_args_size) - return default_value; + return Optional(); return (*tl_current_arg_stack)[offset]; } From d18ea918c00f25023f956c55f084fd40d6e04c29 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 22:16:46 -0600 Subject: [PATCH 17/23] Use less nullptr in ExceptionObject::create_for_raise() --- src/exception_object.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/exception_object.cpp b/src/exception_object.cpp index 3e77bd501..2cddb2a0b 100644 --- a/src/exception_object.cpp +++ b/src/exception_object.cpp @@ -6,9 +6,9 @@ ExceptionObject *ExceptionObject::create_for_raise(Env *env, Args &&args, Except auto kwargs = args.pop_keyword_hash(); auto cause = accept_cause && kwargs ? kwargs->remove(env, "cause"_s) : Optional(); args.ensure_argc_between(env, 0, 3); - auto klass = args.at(0, nullptr); - auto message = args.at(1, nullptr); - auto backtrace = args.at(2, nullptr); + auto klass = args.maybe_at(0); + auto message = args.maybe_at(1); + auto backtrace = args.maybe_at(2); if (!message && kwargs && !kwargs->is_empty()) message = kwargs; @@ -18,13 +18,13 @@ ExceptionObject *ExceptionObject::create_for_raise(Env *env, Args &&args, Except if (!klass && !message && cause) env->raise("ArgumentError", "only cause is given with no arguments"); - if (klass && klass.is_class() && !message) - return _new(env, klass.as_class(), {}, nullptr).as_exception_or_raise(env); + if (klass && klass.value().is_class() && !message) + return _new(env, klass.value().as_class(), {}, nullptr).as_exception_or_raise(env); - if (klass && !klass.is_class() && klass.respond_to(env, "exception"_s)) { + if (klass && !klass.value().is_class() && klass.value().respond_to(env, "exception"_s)) { Vector args; - if (message) args.push(message); - klass = klass.send(env, "exception"_s, std::move(args)); + if (message) args.push(message.value()); + klass = klass.value().send(env, "exception"_s, std::move(args)); } if (!klass && current_exception) @@ -36,7 +36,7 @@ ExceptionObject *ExceptionObject::create_for_raise(Env *env, Args &&args, Except } if (!message) { - Value arg = klass; + Value arg = klass.value(); if (arg.is_string()) { klass = find_top_level_const(env, "RuntimeError"_s).as_class(); message = arg; @@ -49,13 +49,13 @@ ExceptionObject *ExceptionObject::create_for_raise(Env *env, Args &&args, Except Vector exception_args; if (message) - exception_args.push(message); + exception_args.push(message.value()); ExceptionObject *exception; - if (klass.is_class()) - exception = _new(env, klass.as_class(), { std::move(exception_args), false }, nullptr).as_exception(); - else if (klass.is_exception()) - exception = klass.as_exception(); + if (klass.value().is_class()) + exception = _new(env, klass.value().as_class(), { std::move(exception_args), false }, nullptr).as_exception(); + else if (klass.value().is_exception()) + exception = klass.value().as_exception(); else env->raise("TypeError", "exception klass/object expected"); @@ -63,7 +63,7 @@ ExceptionObject *ExceptionObject::create_for_raise(Env *env, Args &&args, Except exception->set_cause(cause.value().as_exception()); if (backtrace) - exception->set_backtrace(env, backtrace); + exception->set_backtrace(env, backtrace.value()); return exception; } From 0a147f9f44d591dad11f8b6594c84fda7412308d Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Tue, 25 Feb 2025 22:20:49 -0600 Subject: [PATCH 18/23] Use Optional for various RangeObject iteration functions --- include/natalie/range_object.hpp | 8 ++++---- src/range_object.cpp | 31 +++++++++++++++---------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/include/natalie/range_object.hpp b/include/natalie/range_object.hpp index 0db3149f4..e48c9691b 100644 --- a/include/natalie/range_object.hpp +++ b/include/natalie/range_object.hpp @@ -69,16 +69,16 @@ class RangeObject : public Object { bool m_exclude_end { false }; template - Value iterate_over_range(Env *env, Function &&f); + Optional iterate_over_range(Env *env, Function &&f); template - Value iterate_over_integer_range(Env *env, Function &&f); + Optional iterate_over_integer_range(Env *env, Function &&f); template - Value iterate_over_string_range(Env *env, Function &&f); + Optional iterate_over_string_range(Env *env, Function &&f); template - Value iterate_over_symbol_range(Env *env, Function &&f); + Optional iterate_over_symbol_range(Env *env, Function &&f); }; } diff --git a/src/range_object.cpp b/src/range_object.cpp index 4234bff71..8f868b0a9 100644 --- a/src/range_object.cpp +++ b/src/range_object.cpp @@ -30,7 +30,7 @@ Value RangeObject::initialize(Env *env, Value begin, Value end, Optional } template -Value RangeObject::iterate_over_range(Env *env, Function &&func) { +Optional RangeObject::iterate_over_range(Env *env, Function &&func) { if (m_begin.is_integer()) return iterate_over_integer_range(env, func); @@ -72,11 +72,11 @@ Value RangeObject::iterate_over_range(Env *env, Function &&func) { } } - return nullptr; + return {}; } template -Value RangeObject::iterate_over_integer_range(Env *env, Function &&func) { +Optional RangeObject::iterate_over_integer_range(Env *env, Function &&func) { auto end = m_end; if (end.is_float()) { if (end.as_float()->is_infinity()) @@ -107,13 +107,13 @@ Value RangeObject::iterate_over_integer_range(Env *env, Function &&func) { } } } - return nullptr; + return {}; } template -Value RangeObject::iterate_over_string_range(Env *env, Function &&func) { +Optional RangeObject::iterate_over_string_range(Env *env, Function &&func) { if (Object::equal(m_begin.send(env, "<=>"_s, { m_end }), Value::integer(1))) - return nullptr; + return {}; TM::Optional current; auto end = m_end.as_string()->string(); @@ -128,13 +128,13 @@ Value RangeObject::iterate_over_string_range(Env *env, Function &&func) { } } - return nullptr; + return {}; } template -Value RangeObject::iterate_over_symbol_range(Env *env, Function &&func) { +Optional RangeObject::iterate_over_symbol_range(Env *env, Function &&func) { if (Object::equal(m_begin.send(env, "<=>"_s, { m_end }), Value::integer(1))) - return nullptr; + return {}; TM::Optional current; auto end = m_end.as_symbol()->string(); @@ -149,7 +149,7 @@ Value RangeObject::iterate_over_symbol_range(Env *env, Function &&func) { } } - return nullptr; + return {}; } Value RangeObject::to_a(Env *env) { @@ -169,14 +169,13 @@ Value RangeObject::each(Env *env, Block *block) { return send(env, "enum_for"_s, { "each"_s }, size_block); } - Value break_value = iterate_over_range(env, [&](Value item) -> Value { + auto break_value = iterate_over_range(env, [&](Value item) -> Value { Value args[] = { item }; block->run(env, Args(1, args), nullptr); return nullptr; }); - if (break_value) { - return break_value; - } + if (break_value) + return break_value.value(); return this; } @@ -331,14 +330,14 @@ bool RangeObject::include(Env *env, Value arg) { auto eqeq = "=="_s; // NATFIXME: Break out of iteration if current arg is smaller than item. // This means we have to implement a way to break out of `iterate_over_range` - Value found_item = iterate_over_range(env, [&](Value item) -> Value { + auto found_item = iterate_over_range(env, [&](Value item) -> Value { if (arg.send(env, eqeq, { item }).is_truthy()) return item; return nullptr; }); - return !!found_item; + return found_item.present(); } Value RangeObject::bsearch(Env *env, Block *block) { From d629a26dd9d2cc1ae52d3c56d64443e836c44a08 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 26 Feb 2025 18:35:45 -0600 Subject: [PATCH 19/23] Use less nullptr for string encoding selection --- include/natalie/encoding_object.hpp | 2 +- src/string_object.cpp | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/natalie/encoding_object.hpp b/include/natalie/encoding_object.hpp index a617a2549..95d9957f1 100644 --- a/include/natalie/encoding_object.hpp +++ b/include/natalie/encoding_object.hpp @@ -112,7 +112,7 @@ class EncodingObject : public Object { EncodeNewlineOption newline_option = EncodeNewlineOption::None; EncodeXmlOption xml_option = EncodeXmlOption::None; StringObject *replace_option = nullptr; - Value fallback_option = nullptr; + Value fallback_option; }; virtual Value encode(Env *, EncodingObject *, StringObject *, EncodeOptions) const; diff --git a/src/string_object.cpp b/src/string_object.cpp index e67705606..834327116 100644 --- a/src/string_object.cpp +++ b/src/string_object.cpp @@ -1372,10 +1372,12 @@ Value StringObject::encode(Env *env, Optional dst_encoding, Optional dst_encoding_arg, Optional src_encoding_arg, HashObject *kwargs) { assert_not_frozen(env); - Value dst_encoding = EncodingObject::default_internal(); + Value dst_encoding; if (dst_encoding_arg) dst_encoding = dst_encoding_arg.value(); - if (dst_encoding == nullptr) // default_internal can return null + else if (EncodingObject::default_internal()) + dst_encoding = EncodingObject::default_internal(); + else dst_encoding = EncodingObject::get(Encoding::UTF_8); auto src_encoding = src_encoding_arg.value_or(m_encoding.ptr()); From af8feedfb641f4ade74bb1e5de2ea2ecf95626b8 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 26 Feb 2025 18:38:08 -0600 Subject: [PATCH 20/23] Use Optional for StringUnpacker unpacked value --- include/natalie/string_unpacker.hpp | 9 +++++---- src/string_unpacker.cpp | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/include/natalie/string_unpacker.hpp b/include/natalie/string_unpacker.hpp index 7d1643530..a58d438a7 100644 --- a/include/natalie/string_unpacker.hpp +++ b/include/natalie/string_unpacker.hpp @@ -23,7 +23,8 @@ class StringUnpacker : public Cell { virtual void visit_children(Visitor &visitor) const override { visitor.visit(m_source); - visitor.visit(m_unpacked_value); + if (m_unpacked_value) + visitor.visit(m_unpacked_value.value()); visitor.visit(m_unpacked_array); } @@ -197,7 +198,7 @@ class StringUnpacker : public Cell { } else { if (!m_unpacked_array) { m_unpacked_array = new ArrayObject {}; - m_unpacked_array->push(m_unpacked_value); + m_unpacked_array->push(m_unpacked_value.value()); } m_unpacked_array->push(value); } @@ -207,13 +208,13 @@ class StringUnpacker : public Cell { if (m_unpacked_array) return m_unpacked_array; else if (m_unpacked_value) - return new ArrayObject { m_unpacked_value }; + return new ArrayObject { m_unpacked_value.value() }; return new ArrayObject {}; } const StringObject *m_source; TM::Vector *m_directives; - Value m_unpacked_value { nullptr }; + Optional m_unpacked_value {}; ArrayObject *m_unpacked_array { nullptr }; size_t m_index { 0 }; }; diff --git a/src/string_unpacker.cpp b/src/string_unpacker.cpp index d93ac3917..dd3e30acc 100644 --- a/src/string_unpacker.cpp +++ b/src/string_unpacker.cpp @@ -52,7 +52,7 @@ Value StringUnpacker::unpack1(Env *env) { for (auto token : *m_directives) { unpack_token(env, token); if (m_unpacked_value) - return m_unpacked_value; + return m_unpacked_value.value(); } return Value::nil(); } From c4f31e44ea019c20af13c488670564d12398e565 Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 26 Feb 2025 19:12:30 -0600 Subject: [PATCH 21/23] Use less nullptr in Tempfile#initialize --- lib/tempfile.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/tempfile.cpp b/lib/tempfile.cpp index 877e0e437..c8cc53789 100644 --- a/lib/tempfile.cpp +++ b/lib/tempfile.cpp @@ -8,12 +8,14 @@ Value init_tempfile(Env *env, Value self) { Value Tempfile_initialize(Env *env, Value self, Args &&args, Block *) { auto kwargs = args.pop_keyword_hash(); + if (!kwargs) + kwargs = new HashObject {}; args.ensure_argc_between(env, 0, 2); - auto basename = args.at(0, nullptr); - auto tmpdir = args.at(1, nullptr); + auto basename = args.at(0, Value::nil()); + auto tmpdir = args.at(1, Value::nil()); auto suffix = new StringObject { "" }; - if (!basename) { + if (basename.is_nil()) { basename = new StringObject { "" }; } else if (!basename.is_string() && basename.respond_to(env, "to_str"_s)) { basename = basename.to_str(env); @@ -26,7 +28,7 @@ Value Tempfile_initialize(Env *env, Value self, Args &&args, Block *) { if (!basename.is_string()) { env->raise("ArgumentError", "unexpected prefix: {}", basename.inspect_str(env)); } - if (tmpdir && !tmpdir.is_nil()) { + if (!tmpdir.is_nil()) { tmpdir.assert_type(env, Object::Type::String, "String"); } else { tmpdir = GlobalEnv::the()->Object()->const_fetch("Dir"_s).send(env, "tmpdir"_s); From 94857fadfa4ff46aa1e59eae495681c216f19a3c Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 26 Feb 2025 19:12:38 -0600 Subject: [PATCH 22/23] Use less nullptr in File#initialize --- src/file_object.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/file_object.cpp b/src/file_object.cpp index 4cc166e35..84c1e4e83 100644 --- a/src/file_object.cpp +++ b/src/file_object.cpp @@ -40,15 +40,15 @@ Value FileObject::initialize(Env *env, Args &&args, Block *block) { auto kwargs = args.pop_keyword_hash(); args.ensure_argc_between(env, 1, 3); auto filename = args.at(0); - auto flags_obj = args.at(1, nullptr); - auto perm = args.at(2, nullptr); + auto flags_obj = args.at(1, Value::nil()); + auto perm = args.at(2, Value::nil()); const ioutil::flags_struct flags { env, flags_obj, kwargs }; const auto modenum = ioutil::perm_to_mode(env, perm); if (filename.is_integer()) { // passing in a number uses fd number int fileno = IntegerMethods::convert_to_int(env, filename); String flags_str = String('r'); - if (flags_obj) { + if (!flags_obj.is_nil()) { flags_obj.assert_type(env, Object::Type::String, "String"); flags_str = flags_obj.as_string()->string(); } From eaa9423c096d62a8e780b620426b48e59012263f Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Wed, 26 Feb 2025 19:14:04 -0600 Subject: [PATCH 23/23] Use less nullptr in IO#read_file --- src/io_object.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/io_object.cpp b/src/io_object.cpp index 21f09d900..fe4ba9280 100644 --- a/src/io_object.cpp +++ b/src/io_object.cpp @@ -281,7 +281,7 @@ Value IoObject::read_file(Env *env, Args &&args) { auto filename = args.at(0); auto length = args.at(1, Value::nil()); auto offset = args.at(2, Value::nil()); - const ioutil::flags_struct flags { env, nullptr, kwargs }; + const ioutil::flags_struct flags { env, Value::nil(), kwargs }; if (!flags_is_readable(flags.flags())) env->raise("IOError", "not opened for reading"); if (filename.is_string() && filename.as_string()->string()[0] == '|')