Feature #8923

Frozen nil/true/false

Added by Koichi Sasada over 1 year ago. Updated 9 months ago.

[ruby-core:57273]
Status:Closed
Priority:Normal
Assignee:Yukihiro Matsumoto

Description

Related to [Feature #8906]

We already froze Integer, Float. Symbols soon (now working).

How about to freeze nil, true and false, too?

# frozen ruby? "Ruby"f?


Related issues

Related to Ruby trunk - Feature #8579: Frozen string syntax Closed 06/29/2013

Associated revisions

Revision 47525
Added by Nobuyoshi Nakada 9 months ago

test_object.rb: add assertions

  • test/ruby/test_object.rb (test_freeze_immediate): assertions for [Feature #8923].

Revision 47525
Added by Nobuyoshi Nakada 9 months ago

test_object.rb: add assertions

  • test/ruby/test_object.rb (test_freeze_immediate): assertions for [Feature #8923].

History

#1 Updated by Hans Mackowiak over 1 year ago

they are a bit different with methods, because defining singleton methods on them redirects to the curresponding Class

but yes, instance variables on them doesnt make much sense

#2 Updated by Charles Nutter over 1 year ago

I obviously support this :-)

#3 Updated by Hiroshi SHIBATA over 1 year ago

  • Target version changed from 2.1.0 to current: 2.2.0

#4 Updated by Koichi Sasada 9 months ago

Matz, what do you think about it?

#5 Updated by Yukihiro Matsumoto 9 months ago

I agreed with making those values, if no significant comparability problem happens.
Please experiment.

Matz.

#6 Updated by Nobuyoshi Nakada 9 months ago

  • Description updated (diff)

#7 Updated by Hans Mackowiak 9 months ago

i am unsure about toally freeze them ... some might extend/include some "Boolean" module into true/false or the TrueClass/FalseClass to specially check if a value is only true or false

i dont know if freezing this objects would break such a thing ...

#8 Updated by Koichi Sasada 9 months ago

The following patch makes test-all green.

Index: gc.c
===================================================================
--- gc.c    (revision 47513)
+++ gc.c    (working copy)
@@ -2286,11 +2286,11 @@ should_be_callable(VALUE block)
 static void
 should_be_finalizable(VALUE obj)
 {
-    rb_check_frozen(obj);
     if (!FL_ABLE(obj)) {
    rb_raise(rb_eArgError, "cannot define finalizer for %s",
         rb_obj_classname(obj));
     }
+    rb_check_frozen(obj);
 }

 /*
Index: include/ruby/ruby.h
===================================================================
--- include/ruby/ruby.h (revision 47513)
+++ include/ruby/ruby.h (working copy)
@@ -1126,7 +1126,7 @@ struct RStruct {
     (OBJ_TAINTABLE(x) && FL_ABLE(s)) ? \
     RBASIC(x)->flags |= RBASIC(s)->flags & FL_TAINT : 0)

-#define OBJ_FROZEN(x) (!!(FL_ABLE(x)?(RBASIC(x)->flags&(FL_FREEZE)):(FIXNUM_P(x)||FLONUM_P(x)||STATIC_SYM_P(x))))
+#define OBJ_FROZEN(x) (!!(FL_ABLE(x)?(RBASIC(x)->flags&(FL_FREEZE)):1))
 #define OBJ_FREEZE(x) FL_SET((x), FL_FREEZE)

 #if USE_RGENGC
Index: object.c
===================================================================
--- object.c    (revision 47513)
+++ object.c    (working copy)
@@ -980,7 +980,7 @@ rb_obj_tainted(VALUE obj)
 VALUE
 rb_obj_taint(VALUE obj)
 {
-    if (!OBJ_TAINTED(obj)) {
+    if (!OBJ_TAINTED(obj) && OBJ_TAINTABLE(obj)) {
    rb_check_frozen(obj);
    OBJ_TAINT(obj);
     }
@@ -1057,8 +1057,6 @@ rb_obj_infect(VALUE obj1, VALUE obj2)
     OBJ_INFECT(obj1, obj2);
 }

-static st_table *immediate_frozen_tbl = 0;
-
 /*
  *  call-seq:
  *     obj.freeze    -> obj
@@ -1089,10 +1087,7 @@ rb_obj_freeze(VALUE obj)
     if (!OBJ_FROZEN(obj)) {
    OBJ_FREEZE(obj);
    if (SPECIAL_CONST_P(obj)) {
-       if (!immediate_frozen_tbl) {
-       immediate_frozen_tbl = st_init_numtable();
-       }
-       st_insert(immediate_frozen_tbl, obj, (st_data_t)Qtrue);
+       rb_bug("special consts should be frozen.");
    }
     }
     return obj;
@@ -1112,12 +1107,7 @@ rb_obj_freeze(VALUE obj)
 VALUE
 rb_obj_frozen_p(VALUE obj)
 {
-    if (OBJ_FROZEN(obj)) return Qtrue;
-    if (SPECIAL_CONST_P(obj)) {
-   if (!immediate_frozen_tbl) return Qfalse;
-   if (st_lookup(immediate_frozen_tbl, obj, 0)) return Qtrue;
-    }
-    return Qfalse;
+    return OBJ_FROZEN(obj) ? Qtrue : Qfalse;
 }


Index: test/ruby/test_eval.rb
===================================================================
--- test/ruby/test_eval.rb  (revision 47513)
+++ test/ruby/test_eval.rb  (working copy)
@@ -130,7 +130,7 @@ class TestEval < Test::Unit::TestCase
   def forall_TYPE
     objects = [Object.new, [], nil, true, false] # TODO: check
     objects.each do |obj|
-      obj.instance_variable_set :@ivar, 12
+      obj.instance_variable_set :@ivar, 12 unless obj.frozen?
       yield obj
     end
   end
@@ -145,7 +145,7 @@ class TestEval < Test::Unit::TestCase
       assert_equal :sym,  o.instance_eval(":sym")

       assert_equal 11,    o.instance_eval("11")
-      assert_equal 12,    o.instance_eval("@ivar")
+      assert_equal 12,    o.instance_eval("@ivar") unless o.frozen?
       assert_equal 13,    o.instance_eval("@@cvar")
       assert_equal 14,    o.instance_eval("$gvar__eval")
       assert_equal 15,    o.instance_eval("Const")
@@ -155,7 +155,7 @@ class TestEval < Test::Unit::TestCase
       assert_equal "19",  o.instance_eval(%q("1#{9}"))

       1.times {
-        assert_equal 12,  o.instance_eval("@ivar")
+        assert_equal 12,  o.instance_eval("@ivar") unless o.frozen?
         assert_equal 13,  o.instance_eval("@@cvar")
         assert_equal 14,  o.instance_eval("$gvar__eval")
         assert_equal 15,  o.instance_eval("Const")
@@ -173,7 +173,7 @@ class TestEval < Test::Unit::TestCase
       assert_equal :sym,  o.instance_eval { :sym }

       assert_equal 11,    o.instance_eval { 11 }
-      assert_equal 12,    o.instance_eval { @ivar }
+      assert_equal 12,    o.instance_eval { @ivar } unless o.frozen?
       assert_equal 13,    o.instance_eval { @@cvar }
       assert_equal 14,    o.instance_eval { $gvar__eval }
       assert_equal 15,    o.instance_eval { Const }
@@ -183,7 +183,7 @@ class TestEval < Test::Unit::TestCase
       assert_equal "19",  o.instance_eval { "1#{9}" }

       1.times {
-        assert_equal 12,  o.instance_eval { @ivar }
+        assert_equal 12,  o.instance_eval { @ivar } unless o.frozen?
         assert_equal 13,  o.instance_eval { @@cvar }
         assert_equal 14,  o.instance_eval { $gvar__eval }
         assert_equal 15,  o.instance_eval { Const }
Index: test/ruby/test_weakmap.rb
===================================================================
--- test/ruby/test_weakmap.rb   (revision 47513)
+++ test/ruby/test_weakmap.rb   (working copy)
@@ -19,13 +19,13 @@ class TestWeakMap < Test::Unit::TestCase
     assert_raise(ArgumentError) {@wm[true] = x}
     assert_raise(ArgumentError) {@wm[false] = x}
     assert_raise(ArgumentError) {@wm[nil] = x}
-    assert_raise(RuntimeError) {@wm[42] = x}
-    assert_raise(RuntimeError) {@wm[:foo] = x}
+    assert_raise(ArgumentError) {@wm[42] = x}
+    assert_raise(ArgumentError) {@wm[:foo] = x}
     assert_raise(ArgumentError) {@wm[x] = true}
     assert_raise(ArgumentError) {@wm[x] = false}
     assert_raise(ArgumentError) {@wm[x] = nil}
-    assert_raise(RuntimeError) {@wm[x] = 42}
-    assert_raise(RuntimeError) {@wm[x] = :foo}
+    assert_raise(ArgumentError) {@wm[x] = 42}
+    assert_raise(ArgumentError) {@wm[x] = :foo}
   end

   def test_include?

#9 Updated by Koichi Sasada 9 months ago

Hans Mackowiak wrote:

i am unsure about toally freeze them ... some might extend/include some "Boolean" module into true/false or the TrueClass/FalseClass to specially check if a value is only true or false

i dont know if freezing this objects would break such a thing ...

p true.frozen? #=> true
module Boolean; end
class TrueClass
  include Boolean
end
p true.kind_of?(Boolean) #=> true

Not freeze TrueClass, but freeze true object.

#10 Updated by Xavier Noria 9 months ago

Hans Mackowiak wrote:

i am unsure about toally freeze them ... some might extend/include some "Boolean" module into true/false or the TrueClass/FalseClass to specially check if a value is only true or false

Can you explain that use case a little further?

#11 Updated by Yukihiro Matsumoto 9 months ago

@ko1 you have proven there's no significant issue. Go ahead.
Of course, we might have to revert it if something happens outside of test suites.

Matz.

#12 Updated by Nobuyoshi Nakada 9 months ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

Applied in changeset r47525.


test_object.rb: add assertions

  • test/ruby/test_object.rb (test_freeze_immediate): assertions for [Feature #8923].

Also available in: Atom PDF