From a5c5f83b24a1b7024d4e7fe3bbce091634da53b2 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 16 Jan 2024 09:33:16 +0100 Subject: [PATCH] Make `const_source_location` return the real constant as soon as defined [Bug #20188] Ref: https://github.com/fxn/zeitwerk/issues/281#issuecomment-1893228355 Previously, it would only return the real constant location once the autoload was fully completed. --- spec/ruby/core/module/autoload_spec.rb | 11 +++++++++++ spec/ruby/core/module/const_source_location_spec.rb | 12 ++++++++++++ .../fixtures/autoload_const_source_location.rb | 6 ++++++ variable.c | 13 +++++++++++++ 4 files changed, 42 insertions(+) create mode 100644 spec/ruby/core/module/fixtures/autoload_const_source_location.rb diff --git a/spec/ruby/core/module/autoload_spec.rb b/spec/ruby/core/module/autoload_spec.rb index 271c55ebf0..45d18b8608 100644 --- a/spec/ruby/core/module/autoload_spec.rb +++ b/spec/ruby/core/module/autoload_spec.rb @@ -406,6 +406,8 @@ describe "Module#autoload" do before :each do @path = fixture(__FILE__, "autoload_during_autoload_after_define.rb") ModuleSpecs::Autoload.autoload :DuringAutoloadAfterDefine, @path + @autoload_location = [__FILE__, __LINE__ - 1] + @const_location = [@path, 2] @remove << :DuringAutoloadAfterDefine raise unless ModuleSpecs::Autoload.autoload?(:DuringAutoloadAfterDefine) == @path end @@ -437,6 +439,15 @@ describe "Module#autoload" do } results.should == [@path, nil, @path, nil] end + + ruby_bug("#20188", ""..."3.4") do + it "returns the real constant location in autoload thread and returns the autoload location in other threads for Module#const_source_location" do + results = check_before_during_thread_after(:DuringAutoloadAfterDefine) { + ModuleSpecs::Autoload.const_source_location(:DuringAutoloadAfterDefine) + } + results.should == [@autoload_location, @const_location, @autoload_location, @const_location] + end + end end it "does not remove the constant from Module#constants if load fails and keeps it as an autoload" do diff --git a/spec/ruby/core/module/const_source_location_spec.rb b/spec/ruby/core/module/const_source_location_spec.rb index ded2aa51d7..c194c9113f 100644 --- a/spec/ruby/core/module/const_source_location_spec.rb +++ b/spec/ruby/core/module/const_source_location_spec.rb @@ -233,5 +233,17 @@ describe "Module#const_source_location" do line = ConstantSpecs::CONST_LOCATION ConstantSpecs.const_source_location('CONST_LOCATION').should == [file, line] end + + ruby_bug("#20188", ""..."3.4") do + it 'returns the real constant location as soon as it is defined' do + file = fixture(__FILE__, 'autoload_const_source_location.rb') + ConstantSpecs.autoload :ConstSource, file + autoload_location = [__FILE__, __LINE__ - 1] + + ConstantSpecs.const_source_location(:ConstSource).should == autoload_location + ConstantSpecs::ConstSource::LOCATION.should == ConstantSpecs.const_source_location(:ConstSource) + ConstantSpecs::BEFORE_DEFINE_LOCATION.should == autoload_location + end + end end end diff --git a/spec/ruby/core/module/fixtures/autoload_const_source_location.rb b/spec/ruby/core/module/fixtures/autoload_const_source_location.rb new file mode 100644 index 0000000000..ee0e5a689f --- /dev/null +++ b/spec/ruby/core/module/fixtures/autoload_const_source_location.rb @@ -0,0 +1,6 @@ +module ConstantSpecs + BEFORE_DEFINE_LOCATION = const_source_location(:ConstSource) + module ConstSource + LOCATION = Object.const_source_location(name) + end +end diff --git a/variable.c b/variable.c index b484d65d19..99332d89a0 100644 --- a/variable.c +++ b/variable.c @@ -3189,6 +3189,19 @@ rb_const_location_from(VALUE klass, ID id, int exclude, int recurse, int visibil if (exclude && klass == rb_cObject) { goto not_found; } + + if (UNDEF_P(ce->value)) { // autoload + VALUE autoload_const_value = autoload_data(klass, id); + if (RTEST(autoload_const_value)) { + struct autoload_const *autoload_const; + struct autoload_data *autoload_data = get_autoload_data(autoload_const_value, &autoload_const); + + if (!UNDEF_P(autoload_const->value) && RTEST(rb_mutex_owned_p(autoload_data->mutex))) { + return rb_assoc_new(autoload_const->file, INT2NUM(autoload_const->line)); + } + } + } + if (NIL_P(ce->file)) return rb_ary_new(); return rb_assoc_new(ce->file, INT2NUM(ce->line)); }