ruby-changes:68314
From: Jeremy <ko1@a...>
Date: Sat, 9 Oct 2021 06:54:43 +0900 (JST)
Subject: [ruby-changes:68314] 08759edea8 (master): Remove autoload for constant if the autoload fails
https://git.ruby-lang.org/ruby.git/commit/?id=08759edea8 From 08759edea8fb75d46c3e75217e6613465426a0d2 Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Fri, 8 Oct 2021 12:54:26 -0900 Subject: Remove autoload for constant if the autoload fails Previously, if an autoload failed (the file was loaded, but the constant was not defined by the autoloaded file). Ruby will try to autoload again if you delete the autoloaded file from $LOADED_FEATURES. With this change, the autoload and the constant itself are removed as soon as it fails. To handle cases where multiple threads are autoloading, when deleting an autoload, handle the case where another thread already deleted it. Fixes [Bug #15790] --- spec/ruby/core/module/autoload_spec.rb | 100 +++++++++++++++++++++++--------- spec/ruby/core/module/const_set_spec.rb | 3 +- test/ruby/test_autoload.rb | 27 ++++++++- variable.c | 38 +++++++----- 4 files changed, 121 insertions(+), 47 deletions(-) diff --git a/spec/ruby/core/module/autoload_spec.rb b/spec/ruby/core/module/autoload_spec.rb index f17675846b..1d61646db5 100644 --- a/spec/ruby/core/module/autoload_spec.rb +++ b/spec/ruby/core/module/autoload_spec.rb @@ -441,21 +441,42 @@ describe "Module#autoload" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/module/autoload_spec.rb#L441 ScratchPad.recorded.should == [:raise, :raise] end - it "does not remove the constant from Module#constants if the loaded file does not define it, but leaves it as 'undefined'" do - path = fixture(__FILE__, "autoload_o.rb") - ScratchPad.record [] - ModuleSpecs::Autoload.autoload :O, path + ruby_version_is "3.1" do + it "removes the constant from Module#constants if the loaded file does not define it" do + path = fixture(__FILE__, "autoload_o.rb") + ScratchPad.record [] + ModuleSpecs::Autoload.autoload :O, path - ModuleSpecs::Autoload.const_defined?(:O).should == true - ModuleSpecs::Autoload.should have_constant(:O) - ModuleSpecs::Autoload.autoload?(:O).should == path + ModuleSpecs::Autoload.const_defined?(:O).should == true + ModuleSpecs::Autoload.should have_constant(:O) + ModuleSpecs::Autoload.autoload?(:O).should == path - -> { ModuleSpecs::Autoload::O }.should raise_error(NameError) + -> { ModuleSpecs::Autoload::O }.should raise_error(NameError) - ModuleSpecs::Autoload.should have_constant(:O) - ModuleSpecs::Autoload.const_defined?(:O).should == false - ModuleSpecs::Autoload.autoload?(:O).should == nil - -> { ModuleSpecs::Autoload.const_get(:O) }.should raise_error(NameError) + ModuleSpecs::Autoload.const_defined?(:O).should == false + ModuleSpecs::Autoload.should_not have_constant(:O) + ModuleSpecs::Autoload.autoload?(:O).should == nil + -> { ModuleSpecs::Autoload.const_get(:O) }.should raise_error(NameError) + end + end + + ruby_version_is ""..."3.1" do + it "does not remove the constant from Module#constants if the loaded file does not define it, but leaves it as 'undefined'" do + path = fixture(__FILE__, "autoload_o.rb") + ScratchPad.record [] + ModuleSpecs::Autoload.autoload :O, path + + ModuleSpecs::Autoload.const_defined?(:O).should == true + ModuleSpecs::Autoload.should have_constant(:O) + ModuleSpecs::Autoload.autoload?(:O).should == path + + -> { ModuleSpecs::Autoload::O }.should raise_error(NameError) + + ModuleSpecs::Autoload.const_defined?(:O).should == false + ModuleSpecs::Autoload.should have_constant(:O) + ModuleSpecs::Autoload.autoload?(:O).should == nil + -> { ModuleSpecs::Autoload.const_get(:O) }.should raise_error(NameError) + end end it "does not try to load the file again if the loaded file did not define the constant" do @@ -554,31 +575,54 @@ describe "Module#autoload" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/module/autoload_spec.rb#L575 # Basically, the parent autoload constant remains in a "undefined" state self.autoload?(:DeclaredInParentDefinedInCurrent).should == nil const_defined?(:DeclaredInParentDefinedInCurrent).should == false - self.should have_constant(:DeclaredInParentDefinedInCurrent) -> { DeclaredInParentDefinedInCurrent }.should raise_error(NameError) ModuleSpecs::Autoload::LexicalScope.send(:remove_const, :DeclaredInParentDefinedInCurrent) end end - it "and fails when finding the undefined autoload constant in the current scope when declared in current and defined in parent" do - @remove << :DeclaredInCurrentDefinedInParent - module ModuleSpecs::Autoload - ScratchPad.record -> { - DeclaredInCurrentDefinedInParent = :declared_in_current_defined_in_parent - } + ruby_version_is "3.1" do + it "looks up in parent scope after failed autoload" do + @remove << :DeclaredInCurrentDefinedInParent + module ModuleSpecs::Autoload + ScratchPad.record -> { + DeclaredInCurrentDefinedInParent = :declared_in_current_defined_in_parent + } - class LexicalScope - autoload :DeclaredInCurrentDefinedInParent, fixture(__FILE__, "autoload_callback.rb") - -> { DeclaredInCurrentDefinedInParent }.should raise_error(NameError) - # Basically, the autoload constant remains in a "undefined" state - self.autoload?(:DeclaredInCurrentDefinedInParent).should == nil - const_defined?(:DeclaredInCurrentDefinedInParent).should == false - self.should have_constant(:DeclaredInCurrentDefinedInParent) - -> { const_get(:DeclaredInCurrentDefinedInParent) }.should raise_error(NameError) + class LexicalScope + autoload :DeclaredInCurrentDefinedInParent, fixture(__FILE__, "autoload_callback.rb") + -> { DeclaredInCurrentDefinedInParent }.should_not raise_error(NameError) + # Basically, the autoload constant remains in a "undefined" state + self.autoload?(:DeclaredInCurrentDefinedInParent).should == nil + const_defined?(:DeclaredInCurrentDefinedInParent).should == false + -> { const_get(:DeclaredInCurrentDefinedInParent) }.should raise_error(NameError) + end + + DeclaredInCurrentDefinedInParent.should == :declared_in_current_defined_in_parent end + end + end + + ruby_version_is ""..."3.1" do + it "and fails when finding the undefined autoload constant in the current scope when declared in current and defined in parent" do + @remove << :DeclaredInCurrentDefinedInParent + module ModuleSpecs::Autoload + ScratchPad.record -> { + DeclaredInCurrentDefinedInParent = :declared_in_current_defined_in_parent + } + + class LexicalScope + autoload :DeclaredInCurrentDefinedInParent, fixture(__FILE__, "autoload_callback.rb") + -> { DeclaredInCurrentDefinedInParent }.should raise_error(NameError) + # Basically, the autoload constant remains in a "undefined" state + self.autoload?(:DeclaredInCurrentDefinedInParent).should == nil + const_defined?(:DeclaredInCurrentDefinedInParent).should == false + self.should have_constant(:DeclaredInCurrentDefinedInParent) + -> { const_get(:DeclaredInCurrentDefinedInParent) }.should raise_error(NameError) + end - DeclaredInCurrentDefinedInParent.should == :declared_in_current_defined_in_parent + DeclaredInCurrentDefinedInParent.should == :declared_in_current_defined_in_parent + end end end diff --git a/spec/ruby/core/module/const_set_spec.rb b/spec/ruby/core/module/const_set_spec.rb index b537d3f133..ba7810d17b 100644 --- a/spec/ruby/core/module/const_set_spec.rb +++ b/spec/ruby/core/module/const_set_spec.rb @@ -101,7 +101,7 @@ describe "Module#const_set" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/module/const_set_spec.rb#L101 mod.const_get(:Foo).should == 1 end - it "does not warn if the previous value was undefined" do + it "does not warn after a failed autoload" do path = fixture(__FILE__, "autoload_o.rb") ScratchPad.record [] mod = Module.new @@ -109,7 +109,6 @@ describe "Module#const_set" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/module/const_set_spec.rb#L109 mod.autoload :Foo, path -> { mod::Foo }.should raise_error(NameError) - mod.should have_constant(:Foo) mod.const_defined?(:Foo).should == false mod.autoload?(:Foo).should == nil diff --git a/test/ruby/test_autoload.rb b/test/ruby/test_autoload.rb index 98d513da87..7709760d19 100644 --- a/test/ruby/test_autoload.rb +++ b/test/ruby/test_autoload.rb @@ -456,6 +456,31 @@ p Foo::Bar https://github.com/ruby/ruby/blob/trunk/test/ruby/test_autoload.rb#L456 end; end + def test_autoload_after_failed_and_removed_from_loaded_features + Dir.mktmpdir('autoload') do |tmpdir| + autoload_path = File.join(tmpdir, "test-bug-15790.rb") + File.write(autoload_path, '') + + assert_separately(%W[-I #{tmpdir}], <<-RUBY) + path = #{File.realpath(autoload_path).inspect} + autoload :X, path + assert_equal(path, Object.autoload?(:X)) + + assert_raise(NameError){X} + assert_nil(Object.autoload?(:X)) + assert_equal(false, Object.const_defined?(:X)) + + $LOADED_FEATURES.delete(path) + assert_equal(false, Object.const_defined?(:X)) + assert_nil(Object.autoload?(:X)) + + assert_raise(NameError){X} + assert_equal(false, Object.const_defined?(:X)) + assert_nil(Object.autoload?(:X)) + RUBY + end + end + def add_autoload(path) (@autoload_paths ||= []) << path ::Object.class_eval {autoload(:AutoloadTest, path)} @@ -463,7 +488,7 @@ p Foo::Bar https://github.com/ruby/ruby/blob/trunk/test/ruby/test_autoload.rb#L488 def remove_autol (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/