[GR-26044] Update singleton class of a class to be frozen if class is frozen

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:

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

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

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

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"
     }

spec/ruby/language/fixtures/module.rb

@@ -17,6 +17,10 @@ module IncludedInObject
     module IncludedModuleSpecs
     end
   end
+
+  module FrozenModule
+  end
+  FrozenModule.freeze
 end
 
 class Object

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

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

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

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