diff --git a/CHANGELOG.md b/CHANGELOG.md index 44ea03ca0cd5..2a69570313af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ New features: Bug fixes: * Add missing thread-safe objects write barriers for `TruffleRuby::ConcurrentMap` (#3179, @eregon). +* Fix singleton class of class should be frozen if class is frozen (@bjfish). Compatibility: @@ -37,7 +38,6 @@ Bug fixes: * Fix using the `--cpusampler` profiler when there are custom unblock functions for `rb_thread_call_without_gvl()` (#3013, @eregon). * Fix recursive raising `FrozenError` exception when redefined `#inspect` modifies an object (#3388, @andrykonchin). * Fix `Integer#div` returning the wrong object type when the divisor is a `Rational` (@simonlevasseur, @nirvdrum). -* Remove constant `Random::DEFAULT` (#3039, @patricklinpl) Compatibility: @@ -74,6 +74,7 @@ Compatibility: * Remove deprecated `Fixnum` and `Bignum` constants (#3039, @andrykonchin). * Add `rb_enc_interned_str_cstr` function (#3408, @goyox86, @thomasmarshall). * Add `rb_str_to_interned_str` function (#3408, @thomasmarshall). +* Remove constant `Random::DEFAULT` (#3039, @patricklinpl) Performance: @@ -84,10 +85,8 @@ Performance: Changes: - Memory Footprint: - # 23.1.0 New features: @@ -152,7 +151,6 @@ Memory Footprint: * Address many truffle-sharing warnings (@horakivo). * Address many truffle-inlining warnings (@horakivo). - # 23.0.0 New features: diff --git a/spec/ruby/fixtures/class.rb b/spec/ruby/fixtures/class.rb index 98cb6c82a2b3..78d0a40b0cc1 100644 --- a/spec/ruby/fixtures/class.rb +++ b/spec/ruby/fixtures/class.rb @@ -126,6 +126,13 @@ def self.get_class_name DEFINE_CLASS = -> do class ::A; end end + + class FrozenClass + end + FrozenClass.freeze + + module ModuleToReopen + end end class Class diff --git a/spec/ruby/language/class_spec.rb b/spec/ruby/language/class_spec.rb index eab3cd0651fe..30de3bd7bab1 100644 --- a/spec/ruby/language/class_spec.rb +++ b/spec/ruby/language/class_spec.rb @@ -46,7 +46,14 @@ class ClassSpecsKeywordWithSemicolon; end -> { class ClassSpecsNumber end - }.should raise_error(TypeError) + }.should raise_error(TypeError, /ClassSpecsNumber is not a class/) + end + + it "raises TypeError if constant given as class name exists and is Module but not a Class" do + -> { + class ClassSpecs::ModuleToReopen + end + }.should raise_error(TypeError, /ModuleToReopen is not a class/) end # test case known to be detecting bugs (JRuby, MRI) @@ -344,6 +351,14 @@ def self.m ClassSpecs::M.m.should == 1 ClassSpecs::L.singleton_class.send(:remove_method, :m) end + + it "does not raise FrozenError if class is frozen and is not modified" do + ClassSpecs::FrozenClass.should.frozen? + + class ClassSpecs::FrozenClass + :foo + end.should == :foo + end end describe "class provides hooks" do diff --git a/spec/ruby/language/constants_spec.rb b/spec/ruby/language/constants_spec.rb index 08c534487ec3..64d5e87167eb 100644 --- a/spec/ruby/language/constants_spec.rb +++ b/spec/ruby/language/constants_spec.rb @@ -734,7 +734,7 @@ class PrivateClass end describe 'Assignment' do - context 'dynamic assignment' do + describe 'dynamic assignment' do it 'raises SyntaxError' do -> do eval <<-CODE @@ -745,4 +745,22 @@ def test end.should raise_error(SyntaxError, /dynamic constant assignment/) end end + + context 'when module is frozen' do + it 'raises FrozenError' do + m = Module.new + m.freeze + + -> { m::A = 1 }.should raise_error(FrozenError, /can't modify frozen Module:/) + end + end + + context 'when class is frozen' do + it 'raises FrozenError' do + c = Class.new + c.freeze + + -> { c::A = 1 }.should raise_error(FrozenError, /can't modify frozen/) + end + end end diff --git a/spec/ruby/language/def_spec.rb b/spec/ruby/language/def_spec.rb index c8531343c06d..4567d0050948 100644 --- a/spec/ruby/language/def_spec.rb +++ b/spec/ruby/language/def_spec.rb @@ -90,7 +90,7 @@ def foo(a); end -> { foo 1, 2 }.should raise_error(ArgumentError, 'wrong number of arguments (given 2, expected 1)') end - it "raises FrozenError with the correct class name" do + it "raises FrozenError with the correct class name if frozen" do -> { Module.new do self.freeze @@ -270,14 +270,15 @@ def obj.==(other) -> { def obj.foo; end }.should raise_error(FrozenError) end - it "raises FrozenError with the correct class name" do + it "raises FrozenError with the correct class name if frozen" do obj = Object.new obj.freeze -> { def obj.foo; end }.should raise_error(FrozenError){ |e| e.message.should.start_with? "can't modify frozen object" } - c = obj.singleton_class + c = Class.new + c.freeze -> { def c.foo; end }.should raise_error(FrozenError){ |e| e.message.should.start_with? "can't modify frozen Class" } diff --git a/spec/ruby/language/fixtures/module.rb b/spec/ruby/language/fixtures/module.rb index 33d323846e2d..82fede2c8ec7 100644 --- a/spec/ruby/language/fixtures/module.rb +++ b/spec/ruby/language/fixtures/module.rb @@ -17,6 +17,10 @@ module IncludedInObject module IncludedModuleSpecs end end + + module FrozenModule + end + FrozenModule.freeze end class Object diff --git a/spec/ruby/language/metaclass_spec.rb b/spec/ruby/language/metaclass_spec.rb index fc8306797759..cf6897c9a0b7 100644 --- a/spec/ruby/language/metaclass_spec.rb +++ b/spec/ruby/language/metaclass_spec.rb @@ -141,3 +141,29 @@ class << @object d_meta.ham.should == 'iberico' end end + +context 'when object is frozen' do + it 'does not raise FrozenError if object is not modified' do + o = Object.new + o.freeze + class << o + :foo + end.should == :foo + end + + it 'does not raise FrozenError if it is a class and it is not modified' do + o = Class.new + o.freeze + class << o + :foo + end.should == :foo + end + + it 'does not raise FrozenError if it is a module and it is not modified' do + o = Module.new + o.freeze + class << o + :foo + end.should == :foo + end +end \ No newline at end of file diff --git a/spec/ruby/language/module_spec.rb b/spec/ruby/language/module_spec.rb index fffcf9c90db9..aff6b60b8f4b 100644 --- a/spec/ruby/language/module_spec.rb +++ b/spec/ruby/language/module_spec.rb @@ -42,6 +42,14 @@ module IncludedModuleSpecs; Reopened = true; end end end + it "does not raise FrozenError when frozen module is reopen and is not modified" do + ModuleSpecs::FrozenModule.should.frozen? + + module ModuleSpecs::FrozenModule + :foo + end.should == :foo + end + it "raises a TypeError if the constant is a Class" do -> do module ModuleSpecs::Modules::Klass; end diff --git a/spec/ruby/language/singleton_class_spec.rb b/spec/ruby/language/singleton_class_spec.rb index 9d037717b24c..c7d1342e0b40 100644 --- a/spec/ruby/language/singleton_class_spec.rb +++ b/spec/ruby/language/singleton_class_spec.rb @@ -282,13 +282,27 @@ def singleton_method; 1 end it "raises a TypeError when new is called" do -> { Object.new.singleton_class.new - }.should raise_error(TypeError) + }.should raise_error(TypeError, /can't create instance of singleton class/) end it "raises a TypeError when allocate is called" do -> { Object.new.singleton_class.allocate - }.should raise_error(TypeError) + }.should raise_error(TypeError, /can't create instance of singleton class/) + end +end + +describe "Copying a singleton class" do + it "raises TypeError when #clone is called" do + -> { + Object.new.singleton_class.clone + }.should raise_error(TypeError, /can't copy singleton class/) + end + + it "raises TypeError when #dup is called" do + -> { + Object.new.singleton_class.dup + }.should raise_error(TypeError, /can't copy singleton class/) end end @@ -314,4 +328,18 @@ def singleton_method; 1 end o2 = o.clone(freeze: false) o2.singleton_class.frozen?.should == false end + + it "has its own singleton class frozen if the object it is created from is frozen" do + o = Object.new + o.freeze + klass = o.singleton_class.singleton_class + klass.frozen?.should == true + end + + it "has its own singleton class frozen if the object it is created from becomes frozen" do + o = Object.new + o.singleton_class.singleton_class.frozen?.should == false + o.freeze + o.singleton_class.singleton_class.frozen?.should == true + end end diff --git a/spec/tags/language/def_tags.txt b/spec/tags/language/def_tags.txt deleted file mode 100644 index 7461260ad73a..000000000000 --- a/spec/tags/language/def_tags.txt +++ /dev/null @@ -1 +0,0 @@ -fails:A singleton method definition raises FrozenError with the correct class name diff --git a/src/main/java/org/truffleruby/language/objects/SingletonClassNode.java b/src/main/java/org/truffleruby/language/objects/SingletonClassNode.java index 4cbb7af9490e..0970eb7c2402 100644 --- a/src/main/java/org/truffleruby/language/objects/SingletonClassNode.java +++ b/src/main/java/org/truffleruby/language/objects/SingletonClassNode.java @@ -9,7 +9,9 @@ */ package org.truffleruby.language.objects; +import com.oracle.truffle.api.dsl.Bind; import com.oracle.truffle.api.dsl.GenerateUncached; +import com.oracle.truffle.api.nodes.Node; import org.truffleruby.RubyContext; import org.truffleruby.core.klass.ClassNodes; import org.truffleruby.core.klass.RubyClass; @@ -42,15 +44,23 @@ public static SingletonClassNode getUncached() { // no need to guard on the context, the rubyClass is context-specific guards = { "isSingleContext()", "rubyClass == cachedClass", "cachedSingletonClass != null" }, limit = "1") - RubyClass singletonClassClassCached(RubyClass rubyClass, + static RubyClass singletonClassClassCached(RubyClass rubyClass, @Cached("rubyClass") RubyClass cachedClass, - @Cached("getSingletonClassOfClassOrNull(getContext(), cachedClass)") RubyClass cachedSingletonClass) { + @Cached IsFrozenNode isFrozenNode, + @Cached FreezeNode freezeNode, + @Bind("this") Node node, + @Cached("getSingletonClassOfClassOrNull(getContext(node), node, cachedClass, isFrozenNode, freezeNode)") RubyClass cachedSingletonClass) { return cachedSingletonClass; } @Specialization(replaces = "singletonClassClassCached") RubyClass singletonClassClassUncached(RubyClass rubyClass) { - return ClassNodes.getSingletonClassOfClass(getContext(), rubyClass); + final RubyClass result = ClassNodes.getSingletonClassOfClass(getContext(), rubyClass); + + if (IsFrozenNodeGen.getUncached().execute(rubyClass)) { + FreezeNode.executeUncached(result); + } + return result; } @Specialization( @@ -111,8 +121,16 @@ private RubyClass noSingletonClass() { throw new RaiseException(getContext(), coreExceptions().typeErrorCantDefineSingleton(this)); } - protected RubyClass getSingletonClassOfClassOrNull(RubyContext context, RubyClass rubyClass) { - return ClassNodes.getSingletonClassOfClassOrNull(context, rubyClass); + protected RubyClass getSingletonClassOfClassOrNull(RubyContext context, Node node, RubyClass rubyClass, + IsFrozenNode isFrozenNode, + FreezeNode freezeNode) { + final RubyClass result = ClassNodes.getSingletonClassOfClassOrNull(context, rubyClass); + + if (result != null && isFrozenNode.execute(rubyClass)) { + freezeNode.execute(node, result); + } + + return result; } @TruffleBoundary