diff --git a/include/natalie/module_object.hpp b/include/natalie/module_object.hpp index 65ff594ba..c2fecbbc7 100644 --- a/include/natalie/module_object.hpp +++ b/include/natalie/module_object.hpp @@ -48,8 +48,8 @@ class ModuleObject : public Object { Value is_autoload(Env *, Value) const; - virtual Value const_find(Env *, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::Raise) override; - virtual Value const_find_with_autoload(Env *, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::Raise) override; + virtual Value const_find(Env *, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing) override; + virtual Value const_find_with_autoload(Env *, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing) override; virtual Value const_get(SymbolObject *) const override; virtual Value const_fetch(SymbolObject *) override; virtual Value const_set(SymbolObject *, Value) override; @@ -176,6 +176,9 @@ class ModuleObject : public Object { snprintf(buf, len, "", this); } +private: + Value handle_missing_constant(Env *, Value, ConstLookupFailureMode); + protected: Constant *find_constant(Env *, SymbolObject *, ModuleObject **, ConstLookupSearchMode = ConstLookupSearchMode::Strict); diff --git a/include/natalie/object.hpp b/include/natalie/object.hpp index d00e85b76..c5a205714 100644 --- a/include/natalie/object.hpp +++ b/include/natalie/object.hpp @@ -56,6 +56,7 @@ class Object : public Cell { enum class ConstLookupFailureMode { Null, Raise, + ConstMissing, }; Object() @@ -248,8 +249,8 @@ class Object : public Cell { Value extend(Env *, Args); void extend_once(Env *, ModuleObject *); - virtual Value const_find(Env *, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::Raise); - virtual Value const_find_with_autoload(Env *, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::Raise); + virtual Value const_find(Env *, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing); + virtual Value const_find_with_autoload(Env *, Value, SymbolObject *, ConstLookupSearchMode = ConstLookupSearchMode::Strict, ConstLookupFailureMode = ConstLookupFailureMode::ConstMissing); virtual Value const_get(SymbolObject *) const; virtual Value const_fetch(SymbolObject *); virtual Value const_set(SymbolObject *, Value); diff --git a/lib/natalie/compiler/instructions/const_find_instruction.rb b/lib/natalie/compiler/instructions/const_find_instruction.rb index 8a0189ad2..b4142b0b0 100644 --- a/lib/natalie/compiler/instructions/const_find_instruction.rb +++ b/lib/natalie/compiler/instructions/const_find_instruction.rb @@ -3,9 +3,10 @@ module Natalie class Compiler class ConstFindInstruction < BaseInstruction - def initialize(name, strict:, file: nil, line: nil) + def initialize(name, strict:, failure_mode: 'ConstMissing', file: nil, line: nil) @name = name.to_sym @strict = strict + @failure_mode = failure_mode # source location info @file = file @@ -26,7 +27,16 @@ def generate(transform) namespace = transform.pop search_mode = @strict ? 'Strict' : 'NotStrict' - transform.exec_and_push(:const, "#{namespace}->const_find_with_autoload(env, self, #{transform.intern(name)}, Object::ConstLookupSearchMode::#{search_mode})") + transform.exec_and_push( + :const, + "#{namespace}->const_find_with_autoload(" \ + 'env, ' \ + 'self, ' \ + "#{transform.intern(name)}, " \ + "Object::ConstLookupSearchMode::#{search_mode}, " \ + "Object::ConstLookupFailureMode::#{@failure_mode}" \ + ')' + ) end def execute(vm) @@ -52,6 +62,10 @@ def find_object_with_constant(obj) end while (obj = parent(obj)) end + def raise_if_missing! + @failure_mode = 'Raise' + end + def serialize(rodata) position = rodata.add(@name.to_s) diff --git a/lib/natalie/compiler/pass1.rb b/lib/natalie/compiler/pass1.rb index b209b39e3..9d55bebd2 100644 --- a/lib/natalie/compiler/pass1.rb +++ b/lib/natalie/compiler/pass1.rb @@ -952,7 +952,7 @@ def transform_constant_or_write_node(node, used:) # defined?(CONST) IsDefinedInstruction.new(type: 'constant'), PushSelfInstruction.new, - ConstFindInstruction.new(node.name, strict: false), + ConstFindInstruction.new(node.name, strict: false, failure_mode: 'Raise'), EndInstruction.new(:is_defined), DupInstruction.new, @@ -1072,35 +1072,35 @@ def transform_constant_path_or_write_node(node, used:) name, _is_private, prep_instruction = constant_name(node.target) # FIXME: is_private shouldn't be ignored I think # - # This describes the stack for the three distinct paths - instructions = [ # if !defined?(tmp::CONST) if defined?(tmp::CONST) && !tmp::CONST if defined(tmp::CONST) && tmp::CONST - prep_instruction, # [tmp] [tmp] [tmp] - DupInstruction.new, # [tmp, tmp] [tmp, tmp] [tmp, tmp] - IsDefinedInstruction.new(type: 'constant'), # [tmp, tmp, is_defined] [tmp, tmp, is_defined] [tmp, tmp, is_defined] - SwapInstruction.new, # [tmp, is_defined, tmp] [tmp, is_defined, tmp] [tmp, is_defined, tmp] - ConstFindInstruction.new(name, strict: true), # [tmp, is_defined, tmp, CONST] [tmp, is_defined, tmp, CONST] [tmp, is_defined, tmp, CONST] - EndInstruction.new(:is_defined), # [tmp, false] [tmp, true] [tmp, true] - IfInstruction.new, # [tmp] [tmp] [tmp] - DupInstruction.new, # [tmp, tmp] [tmp, tmp] - ConstFindInstruction.new(name, strict: true), # [tmp, false] [tmp, tmp::CONST] + # This describes the stack for the three distinct paths + instructions = [ # if !defined?(tmp::CONST) if defined?(tmp::CONST) && !tmp::CONST if defined(tmp::CONST) && tmp::CONST + prep_instruction, # [tmp] [tmp] [tmp] + DupInstruction.new, # [tmp, tmp] [tmp, tmp] [tmp, tmp] + IsDefinedInstruction.new(type: 'constant'), # [tmp, tmp, is_defined] [tmp, tmp, is_defined] [tmp, tmp, is_defined] + SwapInstruction.new, # [tmp, is_defined, tmp] [tmp, is_defined, tmp] [tmp, is_defined, tmp] + ConstFindInstruction.new(name, strict: true, failure_mode: 'Raise'), # [tmp, is_defined, tmp, CONST] [tmp, is_defined, tmp, CONST] [tmp, is_defined, tmp, CONST] + EndInstruction.new(:is_defined), # [tmp, false] [tmp, true] [tmp, true] + IfInstruction.new, # [tmp] [tmp] [tmp] + DupInstruction.new, # [tmp, tmp] [tmp, tmp] + ConstFindInstruction.new(name, strict: true), # [tmp, false] [tmp, tmp::CONST] ElseInstruction.new(:if), - PushFalseInstruction.new, # [tmp, false] + PushFalseInstruction.new, # [tmp, false] EndInstruction.new(:if), - IfInstruction.new, # [tmp] [tmp] [tmp] + IfInstruction.new, # [tmp] [tmp] [tmp] ] if used - instructions << ConstFindInstruction.new(name, strict: true) # [tmp::Const] + instructions << ConstFindInstruction.new(name, strict: true) # [tmp::Const] else - instructions << PopInstruction.new # [] + instructions << PopInstruction.new # [] end instructions << ElseInstruction.new(:if) - instructions << DupInstruction.new if used # [tmp, tmp] [tmp, tmp] + instructions << DupInstruction.new if used # [tmp, tmp] [tmp, tmp] instructions.append( - transform_expression(node.value, used: true), # [tmp, tmp, value] [tmp, tmp, value] - SwapInstruction.new, # [tmp, value, tmp] [tmp, value, tmp] - ConstSetInstruction.new(name), # [tmp] [tmp] + transform_expression(node.value, used: true), # [tmp, tmp, value] [tmp, tmp, value] + SwapInstruction.new, # [tmp, value, tmp] [tmp, value, tmp] + ConstSetInstruction.new(name), # [tmp] [tmp] ) - instructions << ConstFindInstruction.new(name, strict: true) if used # [value] [value] + instructions << ConstFindInstruction.new(name, strict: true) if used # [value] [value] instructions << EndInstruction.new(:if) instructions end @@ -1258,6 +1258,8 @@ def transform_defined_node(node, used:) body[index] = [GlobalVariableDefinedInstruction.new(instruction.name), instruction] when InstanceVariableGetInstruction body[index] = [InstanceVariableDefinedInstruction.new(instruction.name), instruction] + when ConstFindInstruction + body[index].raise_if_missing! when SendInstruction if index == body.length - 1 body[index] = instruction.to_method_defined_instruction diff --git a/spec/language/optional_assignments_spec.rb b/spec/language/optional_assignments_spec.rb index 148cf4aee..64bdf6b5a 100644 --- a/spec/language/optional_assignments_spec.rb +++ b/spec/language/optional_assignments_spec.rb @@ -667,9 +667,7 @@ module ConstantSpecs it 'correctly defines non-existing constants' do ConstantSpecs::ClassA::OR_ASSIGNED_CONSTANT1 ||= :assigned - NATFIXME "Don't use const_missing in defined?(const)", exception: SpecFailedException do - ConstantSpecs::ClassA::OR_ASSIGNED_CONSTANT1.should == :assigned - end + ConstantSpecs::ClassA::OR_ASSIGNED_CONSTANT1.should == :assigned end it 'correctly overwrites nil constants' do @@ -684,9 +682,7 @@ module ConstantSpecs x = 0 (x += 1; ConstantSpecs::ClassA)::OR_ASSIGNED_CONSTANT2 ||= :assigned x.should == 1 - NATFIXME "Don't use const_missing in defined?(const)", exception: SpecFailedException do - ConstantSpecs::ClassA::OR_ASSIGNED_CONSTANT2.should == :assigned - end + ConstantSpecs::ClassA::OR_ASSIGNED_CONSTANT2.should == :assigned end it 'causes side-effects of the module part to be applied only once (for nil constant)' do @@ -708,10 +704,8 @@ module ConstantSpecs }.should raise_error(Exception) x.should == 1 - NATFIXME 'it does not evaluate the right-hand side if the module part raises an exception (for undefined constant)', exception: SpecFailedException do - y.should == 0 - defined?(ConstantSpecs::ClassA::OR_ASSIGNED_CONSTANT3).should == nil - end + y.should == 0 + defined?(ConstantSpecs::ClassA::OR_ASSIGNED_CONSTANT3).should == nil end it 'does not evaluate the right-hand side if the module part raises an exception (for nil constant)' do diff --git a/src/module_object.cpp b/src/module_object.cpp index cbbb58aee..011ff91e2 100644 --- a/src/module_object.cpp +++ b/src/module_object.cpp @@ -227,11 +227,8 @@ Value ModuleObject::const_find_with_autoload(Env *env, Value self, SymbolObject ModuleObject *module = nullptr; auto constant = find_constant(env, name, &module, search_mode); - if (!constant) { - if (failure_mode == ConstLookupFailureMode::Null) - return nullptr; - return send(env, "const_missing"_s, { name }); - } + if (!constant) + return handle_missing_constant(env, name, failure_mode); if (constant->needs_load()) { assert(module); @@ -245,15 +242,26 @@ Value ModuleObject::const_find_with_autoload(Env *env, Value self, SymbolObject Value ModuleObject::const_find(Env *env, SymbolObject *name, ConstLookupSearchMode search_mode, ConstLookupFailureMode failure_mode) { auto constant = find_constant(env, name, nullptr, search_mode); - if (!constant) { - if (failure_mode == ConstLookupFailureMode::Null) - return nullptr; - return send(env, "const_missing"_s, { name }); - } + if (!constant) + return handle_missing_constant(env, name, failure_mode); return constant->value(); } +Value ModuleObject::handle_missing_constant(Env *env, Value name, ConstLookupFailureMode failure_mode) { + if (failure_mode == ConstLookupFailureMode::Null) + return nullptr; + + if (failure_mode == ConstLookupFailureMode::Raise) { + auto name_str = name->to_s(env); + if (this == GlobalEnv::the()->Object()) + env->raise_name_error(name_str, "uninitialized constant {}", name_str->string()); + env->raise_name_error(name_str, "uninitialized constant {}::{}", inspect_str(), name_str->string()); + } + + return send(env, "const_missing"_s, { name }); +} + Value ModuleObject::const_set(SymbolObject *name, Value val) { std::lock_guard lock(g_gc_recursive_mutex); @@ -325,10 +333,7 @@ Value ModuleObject::constants(Env *env, Value inherit) const { } Value ModuleObject::const_missing(Env *env, Value name) { - auto name_str = name->to_s(env); - if (this == GlobalEnv::the()->Object()) - env->raise_name_error(name_str, "uninitialized constant {}", name_str->string()); - env->raise_name_error(name_str, "uninitialized constant {}::{}", inspect_str(), name_str->string()); + return handle_missing_constant(env, name, ConstLookupFailureMode::Raise); } void ModuleObject::make_method_alias(Env *env, SymbolObject *new_name, SymbolObject *old_name) { diff --git a/test/natalie/defined_test.rb b/test/natalie/defined_test.rb index 4add712a8..9f2a54c18 100644 --- a/test/natalie/defined_test.rb +++ b/test/natalie/defined_test.rb @@ -9,6 +9,12 @@ class Bar end end +class ConstMissingConst + def self.const_missing(const) + const + end +end + def foo; end describe 'defined?' do @@ -19,6 +25,10 @@ def foo; end defined?(NonExistent).should == nil end + it 'does not call const_missing' do + defined?(ConstMissingConst::Bar).should == nil + end + it 'recognizes namespaced constants' do defined?(Foo::Bar).should == 'constant' defined?(Food::Bar).should == nil