From 8eb0c810b228df1f8352c005a7ae882ad4179b4b Mon Sep 17 00:00:00 2001 From: nobu Date: Fri, 21 Nov 2014 16:11:55 +0000 Subject: 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. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@48533 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 8 ++++ ext/-test-/symbol/init.c | 7 +++ internal.h | 1 + object.c | 64 +++++++++++++------------- test/-ext-/symbol/test_inadvertent_creation.rb | 43 ++++++++++++++--- variable.c | 25 +++++----- 6 files changed, 96 insertions(+), 52 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2f32cf98ed..e256838ccf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +Sat Nov 22 01:11:53 2014 Nobuyoshi Nakada + + * 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 * common.mk (ext/ripper/ripper.c): revert about srcdir and top_srcdir. diff --git a/ext/-test-/symbol/init.c b/ext/-test-/symbol/init.c index 3b7cf15899..9e42e1a38b 100644 --- a/ext/-test-/symbol/init.c +++ b/ext/-test-/symbol/init.c @@ -8,11 +8,18 @@ sym_find(VALUE dummy, VALUE sym) 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); } diff --git a/internal.h b/internal.h index 4e5c01a638..e7b866bdd6 100644 --- a/internal.h +++ b/internal.h @@ -1157,6 +1157,7 @@ extern const signed char ruby_digit36_to_number_table[]; 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); diff --git a/object.c b/object.c index eadc1be693..567dec1b10 100644 --- a/object.c +++ b/object.c @@ -1912,17 +1912,17 @@ rb_class_get_superclass(VALUE klass) } #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_id_p)(ID), int (*valid_name_p)(VALUE), 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) 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, VALUE mod) 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, VALUE mod) 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, VALUE mod) 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 *argv, VALUE mod) 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 *argv, VALUE mod) 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 *argv, VALUE mod) } } if (!rb_is_const_id(id)) { - wrong_id: rb_name_error(id, "wrong constant name %"PRIsVALUE, QUOTE_ID(id)); } diff --git a/test/-ext-/symbol/test_inadvertent_creation.rb b/test/-ext-/symbol/test_inadvertent_creation.rb index 9c5fa04407..14e6abd7a3 100644 --- a/test/-ext-/symbol/test_inadvertent_creation.rb +++ b/test/-ext-/symbol/test_inadvertent_creation.rb @@ -15,19 +15,31 @@ module Test_Symbol @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 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 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 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 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 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 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 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 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 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 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 diff --git a/variable.c b/variable.c index 816088f40b..20991dbb40 100644 --- a/variable.c +++ b/variable.c @@ -1475,23 +1475,24 @@ rb_obj_remove_instance_variable(VALUE obj, VALUE name) 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 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 exclude, int recurse, int visibility) goto retry; } - value = const_missing(klass, id); - rb_vm_inc_const_missing_count(); - return value; + return rb_const_missing(klass, ID2SYM(id)); } VALUE -- cgit v1.2.3