Project

General

Profile

Actions

Feature #8923

closed

Frozen nil/true/false

Added by ko1 (Koichi Sasada) about 11 years ago. Updated about 10 years ago.

Status:
Closed
Target version:
[ruby-core:57273]

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 1 (0 open1 closed)

Related to Ruby master - Feature #8579: Frozen string syntaxClosed06/29/2013Actions

Updated by Hanmac (Hans Mackowiak) about 11 years 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

Updated by headius (Charles Nutter) about 11 years ago

I obviously support this :-)

Updated by hsbt (Hiroshi SHIBATA) almost 11 years ago

  • Target version changed from 2.1.0 to 2.2.0

Updated by ko1 (Koichi Sasada) about 10 years ago

Matz, what do you think about it?

Updated by matz (Yukihiro Matsumoto) about 10 years ago

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

Matz.

Updated by nobu (Nobuyoshi Nakada) about 10 years ago

  • Description updated (diff)

Updated by Hanmac (Hans Mackowiak) about 10 years 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 ...

Updated by ko1 (Koichi Sasada) about 10 years 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?

Updated by ko1 (Koichi Sasada) about 10 years 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.

Updated by fxn (Xavier Noria) about 10 years 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?

Updated by matz (Yukihiro Matsumoto) about 10 years ago

@ko1 (Koichi Sasada) 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.

Updated by nobu (Nobuyoshi Nakada) about 10 years 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].
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0