ruby-changes:36452
From: nobu <ko1@a...>
Date: Sat, 22 Nov 2014 01:12:02 +0900 (JST)
Subject: [ruby-changes:36452] nobu:r48533 (trunk): get rid of inadvertent ID creation
nobu 2014-11-22 01:11:55 +0900 (Sat, 22 Nov 2014) New Revision: 48533 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=48533 Log: get rid of inadvertent ID creation * object.c (rb_mod_const_get, rb_mod_const_defined): ditto. * variable.c (rb_const_missing, rb_mod_const_missing): call const_missing without new ID to get rid of inadvertent ID creation. Modified files: trunk/ChangeLog trunk/ext/-test-/symbol/init.c trunk/internal.h trunk/object.c trunk/test/-ext-/symbol/test_inadvertent_creation.rb trunk/variable.c Index: ChangeLog =================================================================== --- ChangeLog (revision 48532) +++ ChangeLog (revision 48533) @@ -1,3 +1,11 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Sat Nov 22 01:11:53 2014 Nobuyoshi Nakada <nobu@r...> + + * object.c (rb_mod_const_get, rb_mod_const_defined): ditto. + + * variable.c (rb_const_missing, rb_mod_const_missing): call + const_missing without new ID to get rid of inadvertent ID + creation. + Fri Nov 21 19:32:57 2014 NARUSE, Yui <naruse@r...> * common.mk (ext/ripper/ripper.c): revert about srcdir and top_srcdir. Index: variable.c =================================================================== --- variable.c (revision 48532) +++ variable.c (revision 48533) @@ -1475,23 +1475,24 @@ rb_obj_remove_instance_variable(VALUE ob https://github.com/ruby/ruby/blob/trunk/variable.c#L1475 UNREACHABLE; } -NORETURN(static void uninitialized_constant(VALUE, ID)); +NORETURN(static void uninitialized_constant(VALUE, VALUE)); static void -uninitialized_constant(VALUE klass, ID id) +uninitialized_constant(VALUE klass, VALUE name) { if (klass && rb_class_real(klass) != rb_cObject) - rb_name_error(id, "uninitialized constant %"PRIsVALUE"::%"PRIsVALUE"", - rb_class_name(klass), - QUOTE_ID(id)); + rb_name_error_str(name, "uninitialized constant %"PRIsVALUE"::% "PRIsVALUE"", + rb_class_name(klass), name); else { - rb_name_error(id, "uninitialized constant %"PRIsVALUE"", QUOTE_ID(id)); + rb_name_error_str(name, "uninitialized constant % "PRIsVALUE"", name); } } -static VALUE -const_missing(VALUE klass, ID id) +VALUE +rb_const_missing(VALUE klass, VALUE name) { - return rb_funcall(klass, rb_intern("const_missing"), 1, ID2SYM(id)); + VALUE value = rb_funcallv(klass, rb_intern("const_missing"), 1, &name); + rb_vm_inc_const_missing_count(); + return value; } @@ -1535,7 +1536,7 @@ VALUE https://github.com/ruby/ruby/blob/trunk/variable.c#L1536 rb_mod_const_missing(VALUE klass, VALUE name) { rb_vm_pop_cfunc_frame(); - uninitialized_constant(klass, rb_to_id(name)); + uninitialized_constant(klass, name); UNREACHABLE; } @@ -1876,9 +1877,7 @@ rb_const_get_0(VALUE klass, ID id, int e https://github.com/ruby/ruby/blob/trunk/variable.c#L1877 goto retry; } - value = const_missing(klass, id); - rb_vm_inc_const_missing_count(); - return value; + return rb_const_missing(klass, ID2SYM(id)); } VALUE Index: object.c =================================================================== --- object.c (revision 48532) +++ object.c (revision 48533) @@ -1912,17 +1912,17 @@ rb_class_get_superclass(VALUE klass) https://github.com/ruby/ruby/blob/trunk/object.c#L1912 } #define id_for_setter(name, type, message) \ - check_setter_id(name, rb_is_##type##_id, rb_is_##type##_name, message) + check_setter_id(name, rb_is_##type##_sym, rb_is_##type##_name, message) static ID -check_setter_id(VALUE name, int (*valid_id_p)(ID), int (*valid_name_p)(VALUE), +check_setter_id(VALUE name, int (*valid_sym_p)(VALUE), int (*valid_name_p)(VALUE), const char *message) { ID id; if (SYMBOL_P(name)) { - id = SYM2ID(name); - if (!valid_id_p(id)) { - rb_name_error(id, message, QUOTE_ID(id)); + if (!valid_sym_p(name)) { + rb_name_error_str(name, message, QUOTE(rb_sym2str(name))); } + id = SYM2ID(name); } else { VALUE str = rb_check_string_type(name); @@ -1933,7 +1933,7 @@ check_setter_id(VALUE name, int (*valid_ https://github.com/ruby/ruby/blob/trunk/object.c#L1933 if (!valid_name_p(str)) { rb_name_error_str(str, message, QUOTE(str)); } - id = rb_to_id(str); + id = rb_intern_str(str); } return id; } @@ -1950,6 +1950,12 @@ rb_is_attr_name(VALUE name) https://github.com/ruby/ruby/blob/trunk/object.c#L1950 return rb_is_local_name(name) || rb_is_const_name(name); } +static int +rb_is_attr_sym(VALUE sym) +{ + return rb_is_local_sym(sym) || rb_is_const_sym(sym); +} + static const char invalid_attribute_name[] = "invalid attribute name `%"PRIsVALUE"'"; static ID @@ -2099,17 +2105,14 @@ rb_mod_const_get(int argc, VALUE *argv, https://github.com/ruby/ruby/blob/trunk/object.c#L2105 const char *pbeg, *p, *path, *pend; ID id; - if (argc == 1) { - name = argv[0]; - recur = Qtrue; - } - else { - rb_scan_args(argc, argv, "11", &name, &recur); - } + rb_check_arity(argc, 1, 2); + name = argv[0]; + recur = (argc == 1) ? Qtrue : argv[1]; if (SYMBOL_P(name)) { - id = SYM2ID(name); - if (!rb_is_const_id(id)) goto wrong_id; + if (!rb_is_const_sym(name)) goto wrong_name; + id = rb_check_id(&name); + if (!id) return rb_const_missing(mod, name); return RTEST(recur) ? rb_const_get(mod, id) : rb_const_get_at(mod, id); } @@ -2125,8 +2128,7 @@ rb_mod_const_get(int argc, VALUE *argv, https://github.com/ruby/ruby/blob/trunk/object.c#L2128 if (p >= pend || !*p) { wrong_name: - rb_raise(rb_eNameError, "wrong constant name %"PRIsVALUE, - QUOTE(name)); + rb_name_error_str(name, "wrong constant name % "PRIsVALUE, name); } if (p + 2 < pend && p[0] == ':' && p[1] == ':') { @@ -2165,16 +2167,17 @@ rb_mod_const_get(int argc, VALUE *argv, https://github.com/ruby/ruby/blob/trunk/object.c#L2167 QUOTE(part)); } else if (!rb_method_basic_definition_p(CLASS_OF(mod), id_const_missing)) { - id = rb_intern_str(part); + part = rb_str_intern(part); + mod = rb_const_missing(mod, part); + continue; } else { - rb_name_error_str(part, "uninitialized constant %"PRIsVALUE"%"PRIsVALUE, + rb_name_error_str(part, "uninitialized constant %"PRIsVALUE"% "PRIsVALUE, rb_str_subseq(name, 0, beglen), - QUOTE(part)); + part); } } if (!rb_is_const_id(id)) { - wrong_id: rb_name_error(id, "wrong constant name %"PRIsVALUE, QUOTE_ID(id)); } @@ -2260,17 +2263,14 @@ rb_mod_const_defined(int argc, VALUE *ar https://github.com/ruby/ruby/blob/trunk/object.c#L2263 const char *pbeg, *p, *path, *pend; ID id; - if (argc == 1) { - name = argv[0]; - recur = Qtrue; - } - else { - rb_scan_args(argc, argv, "11", &name, &recur); - } + rb_check_arity(argc, 1, 2); + name = argv[0]; + recur = (argc == 1) ? Qtrue : argv[1]; if (SYMBOL_P(name)) { - id = SYM2ID(name); - if (!rb_is_const_id(id)) goto wrong_id; + if (!rb_is_const_sym(name)) goto wrong_name; + id = rb_check_id(&name); + if (!id) return Qfalse; return RTEST(recur) ? rb_const_defined(mod, id) : rb_const_defined_at(mod, id); } @@ -2286,8 +2286,7 @@ rb_mod_const_defined(int argc, VALUE *ar https://github.com/ruby/ruby/blob/trunk/object.c#L2286 if (p >= pend || !*p) { wrong_name: - rb_raise(rb_eNameError, "wrong constant name %"PRIsVALUE, - QUOTE(name)); + rb_name_error_str(name, "wrong constant name % "PRIsVALUE, name); } if (p + 2 < pend && p[0] == ':' && p[1] == ':') { @@ -2325,7 +2324,6 @@ rb_mod_const_defined(int argc, VALUE *ar https://github.com/ruby/ruby/blob/trunk/object.c#L2324 } } if (!rb_is_const_id(id)) { - wrong_id: rb_name_error(id, "wrong constant name %"PRIsVALUE, QUOTE_ID(id)); } Index: ext/-test-/symbol/init.c =================================================================== --- ext/-test-/symbol/init.c (revision 48532) +++ ext/-test-/symbol/init.c (revision 48533) @@ -8,11 +8,18 @@ sym_find(VALUE dummy, VALUE sym) https://github.com/ruby/ruby/blob/trunk/ext/-test-/symbol/init.c#L8 return rb_check_symbol(&sym); } +static VALUE +sym_pinneddown_p(VALUE dummy, VALUE sym) +{ + return rb_check_id(&sym) ? Qtrue : Qfalse; +} + void Init_symbol(void) { VALUE mBug = rb_define_module("Bug"); VALUE klass = rb_define_class_under(mBug, "Symbol", rb_cSymbol); rb_define_singleton_method(klass, "find", sym_find, 1); + rb_define_singleton_method(klass, "pinneddown?", sym_pinneddown_p, 1); TEST_INIT_FUNCS(init); } Index: internal.h =================================================================== --- internal.h (revision 48532) +++ internal.h (revision 48533) @@ -1157,6 +1157,7 @@ extern const signed char ruby_digit36_to https://github.com/ruby/ruby/blob/trunk/internal.h#L1157 void rb_gc_mark_global_tbl(void); void rb_mark_generic_ivar(VALUE); void rb_mark_generic_ivar_tbl(void); +VALUE rb_const_missing(VALUE klass, VALUE name); int rb_st_insert_id_and_value(VALUE obj, st_table *tbl, ID key, VALUE value); st_table *rb_st_copy(VALUE obj, struct st_table *orig_tbl); Index: test/-ext-/symbol/test_inadvertent_creation.rb =================================================================== --- test/-ext-/symbol/test_inadvertent_creation.rb (revision 48532) +++ test/-ext-/symbol/test_inadvertent_creation.rb (revision 48533) @@ -15,19 +15,31 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L15 @obj = Object.new end + def assert_not_pinneddown(name, msg = nil) + assert_not_send([Bug::Symbol, :pinneddown?, name], msg) + end + def assert_not_interned(name, msg = nil) assert_not_send([Bug::Symbol, :find, name], msg) end - def assert_not_interned_error(obj, meth, name, msg = nil) - e = assert_raise(NameError, msg) {obj.__send__(meth, name)} - assert_not_interned(name, msg) + def assert_not_interned_error(obj, meth, name, msg = nil, &block) + e = assert_raise(NameError, msg) {obj.__send__(meth, name, &block)} + if Symbol === name + assert_not_pinneddown(name, msg) + else + assert_not_interned(name, msg) + end e end def assert_not_interned_false(obj, meth, name, msg = nil) assert_not_send([obj, meth, name], msg) - assert_not_interned(name, msg) + if Symbol === name + assert_not_pinneddown(name, msg) + else + assert_not_interned(name, msg) + end end Feature5072 = '[ruby-core:38367]' @@ -37,6 +49,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L49 name = noninterned_name("A") assert_not_interned_error(cl, :const_get, name, Feature5072) + + assert_not_interned_error(cl, :const_get, name.to_sym) end def test_module_const_defined? @@ -44,6 +58,9 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L58 name = noninterned_name("A") assert_not_interned_false(cl, :const_defined?, name, Feature5072) + + name = noninterned_name + assert_not_interned_error(cl, :const_defined?, name.to_sym) end def test_respond_to_missing @@ -115,8 +132,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L132 end s = noninterned_name("A") - # assert_not_interned_error(c, :const_get, s, feature5089) - assert_not_interned_false(c, :autoload?, s, feature5089) + assert_not_interned_error(c, :const_get, s.to_sym, feature5089) + assert_not_interned_false(c, :autoload?, s.to_sym, feature5089) end def test_aliased_method @@ -206,6 +223,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L223 mod = Module.new assert_raise(NameError) {mod.const_set(name, true)} assert_not_interned(name) + assert_raise(NameError) {mod.const_set(name.to_sym, true)} + assert_not_pinneddown(name) end def test_module_cvar_set @@ -213,6 +232,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L232 mod = Module.new assert_raise(NameError) {mod.class_variable_set(name, true)} assert_not_interned(name) + assert_raise(NameError) {mod.class_variable_set(name.to_sym, true)} + assert_not_pinneddown(name) end def test_object_ivar_set @@ -220,6 +241,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L241 obj = Object.new assert_raise(NameError) {obj.instance_variable_set(name, true)} assert_not_interned(name) + assert_raise(NameError) {obj.instance_variable_set(name.to_sym, true)} + assert_not_pinneddown(name) end def test_struct_new @@ -247,6 +270,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L270 mod = Module.new assert_raise(NameError) {mod.module_eval {attr(name)}} assert_not_interned(name) + assert_raise(NameError) {mod.module_eval {attr(name.to_sym)}} + assert_not_pinneddown(name) end def test_invalid_attr_reader @@ -254,6 +279,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L279 mod = Module.new assert_raise(NameError) {mod.module_eval {attr_reader(name)}} assert_not_interned(name) + assert_raise(NameError) {mod.module_eval {attr_reader(name.to_sym)}} + assert_not_pinneddown(name) end def test_invalid_attr_writer @@ -261,6 +288,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L288 mod = Module.new assert_raise(NameError) {mod.module_eval {attr_writer(name)}} assert_not_interned(name) + assert_raise(NameError) {mod.module_eval {attr_writer(name.to_sym)}} + assert_not_pinneddown(name) end def test_invalid_attr_accessor @@ -268,6 +297,8 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L297 mod = Module.new assert_raise(NameError) {mod.module_eval {attr_accessor(name)}} assert_not_interned(name) + assert_raise(NameError) {mod.module_eval {attr_accessor(name.to_sym)}} + assert_not_pinneddown(name) end def test_gc_attrset -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/