Merge pull request #2622 from natalie-lang/less-null-3

Use Optional<Value> in place of nullptr (part 3)

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

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<SymbolObject *> names) {

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 <iterator>
@@ -54,6 +55,7 @@ class Args {
 
     Value at(size_t index) const;
     Value at(size_t index, Value default_value) const;
+    Optional<Value> 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;

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;

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<Value>, bool = false);
     Value global_alias(SymbolObject *, SymbolObject *);
 
     const Method *current_method();
@@ -170,9 +171,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<Value> match() { return m_match; }
+    void set_match(Optional<Value> match) { m_match = match; }
+    void clear_match() { m_match = Optional<Value>(); }
 
     Value exception_object();
     ExceptionObject *exception();
@@ -205,8 +206,8 @@ class Env : public Cell {
     size_t m_line { 0 };
     const Method *m_method { nullptr };
     ModuleObject *m_module { nullptr };
-    Value m_match { nullptr };
+    Optional<Value> m_match {};
     ExceptionObject *m_exception { nullptr };
-    Value m_catch { nullptr };
+    Optional<Value> m_catch {};
 };
 }

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, "<ExceptionObject %p message=(null)>", this);
-            // } else if (m_message.type() == Object::Type::String) {
-            // snprintf(buf, len, "<ExceptionObject %p message='%s'>", this, m_message.as_string()->c_str());
-        } else {
-            snprintf(buf, len, "<ExceptionObject %p message=?>", this);
-        }
+        snprintf(buf, len, "<ExceptionObject %p message=?>", 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 };

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 };
 
@@ -114,7 +114,7 @@ class HashObject : public Object {
     Value set_default(Env *, Value);
 
     void put(Env *, Value, Value);
-    Value remove(Env *, Value);
+    Optional<Value> remove(Env *, Value);
     Value clear(Env *);
 
     Value default_proc(Env *);

include/natalie/kernel_module.hpp

@@ -92,10 +92,12 @@ class KernelModule {
     static Value private_methods(Env *env, Value self, Optional<Value> recur = {});
     static Value protected_methods(Env *env, Value self, Optional<Value> recur = {});
     static Value public_methods(Env *env, Value self, Optional<Value> 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<Value>);
     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);
 };
 

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<Value> m_self {};
     int m_arity { 0 };
     Optional<String> m_file {};
     Optional<size_t> m_line {};

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<Value> const_find_with_autoload(Env *, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing);
+    Optional<Value> const_find(Env *, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing);
 
     Value const_get(SymbolObject *) const;
     Value const_get(Env *, Value, Optional<Value> = {});
@@ -183,7 +183,7 @@ class ModuleObject : public Object {
     }
 
 private:
-    Value handle_missing_constant(Env *, Value, ConstLookupFailureMode);
+    Optional<Value> handle_missing_constant(Env *, Value, ConstLookupFailureMode);
 
     ClassObject *as_class();
 

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<Value> 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 *);
@@ -148,22 +148,15 @@ class Object : public Cell {
         return buf;
     }
 
-    Value public_send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Value sent_from = nullptr);
-    static Value public_send(Env *, Value, Args &&, Block *);
-
-    Value send(Env *, SymbolObject *, Args && = Args(), Block * = nullptr, Value sent_from = nullptr);
-    static Value send(Env *, Value, Args &&, Block *);
-
-    Value send(Env *env, SymbolObject *name, std::initializer_list<Value> args, Block *block = nullptr, Value sent_from = nullptr) {
-        // NOTE: sent_from is unused, but accepting it makes the SendInstruction codegen simpler. :-)
-        return send(env, name, Args(args), block);
+    Value public_send(Env *env, SymbolObject *name, Args &&args = {}, Block *block = nullptr, Optional<Value> sent_from = {}) {
+        return send(env, name, std::move(args), block, MethodVisibility::Public, sent_from);
     }
 
-    Value send(Env *, SymbolObject *, Args &&, Block *, MethodVisibility, Value = nullptr);
+    Value send(Env *, SymbolObject *, Args && = {}, Block * = nullptr, MethodVisibility = MethodVisibility::Private, Optional<Value> = {});
     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<Value>) const;
 
     Value duplicate(Env *) const;
     Value clone(Env *env, Optional<Value> freeze = {});

include/natalie/range_object.hpp

@@ -69,16 +69,16 @@ class RangeObject : public Object {
     bool m_exclude_end { false };
 
     template <typename Function>
-    Value iterate_over_range(Env *env, Function &&f);
+    Optional<Value> iterate_over_range(Env *env, Function &&f);
 
     template <typename Function>
-    Value iterate_over_integer_range(Env *env, Function &&f);
+    Optional<Value> iterate_over_integer_range(Env *env, Function &&f);
 
     template <typename Function>
-    Value iterate_over_string_range(Env *env, Function &&f);
+    Optional<Value> iterate_over_string_range(Env *env, Function &&f);
 
     template <typename Function>
-    Value iterate_over_symbol_range(Env *env, Function &&f);
+    Optional<Value> iterate_over_symbol_range(Env *env, Function &&f);
 };
 
 }

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<Token> *m_directives;
-    Value m_unpacked_value { nullptr };
+    Optional<Value> m_unpacked_value {};
     ArrayObject *m_unpacked_array { nullptr };
     size_t m_index { 0 };
 };

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<ExceptionObject *> m_exception { nullptr };
-    Value m_value { nullptr };
+    Optional<Value> 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<Thread::MutexObject *> 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.