ruby-changes:70031
From: John <ko1@a...>
Date: Fri, 3 Dec 2021 08:53:53 +0900 (JST)
Subject: [ruby-changes:70031] 733500e9d0 (master): Lazily create singletons on instance_{exec, eval} (#5146)
https://git.ruby-lang.org/ruby.git/commit/?id=733500e9d0 From 733500e9d02b11ff60fbbdb8daa43c2e9cfbd750 Mon Sep 17 00:00:00 2001 From: John Hawthorn <john@h...> Date: Thu, 2 Dec 2021 15:53:39 -0800 Subject: Lazily create singletons on instance_{exec,eval} (#5146) * Lazily create singletons on instance_{exec,eval} Previously when instance_exec or instance_eval was called on an object, that object would be given a singleton class so that method definitions inside the block would be added to the object rather than its class. This commit aims to improve performance by delaying the creation of the singleton class unless/until one is needed for method definition. Most of the time instance_eval is used without any method definition. This was implemented by adding a flag to the cref indicating that it represents a singleton of the object rather than a class itself. In this case CREF_CLASS returns the object's existing class, but in cases that we are defining a method (either via definemethod or VM_SPECIAL_OBJECT_CBASE which is used for undef and alias). This also happens to fix what I believe is a bug. Previously instance_eval behaved differently with regards to constant access for true/false/nil than for all other objects. I don't think this was intentional. String::Foo = "foo" "".instance_eval("Foo") # => "foo" Integer::Foo = "foo" 123.instance_eval("Foo") # => "foo" TrueClass::Foo = "foo" true.instance_eval("Foo") # NameError: uninitialized constant Foo This also slightly changes the error message when trying to define a method through instance_eval on an object which can't have a singleton class. Before: $ ruby -e '123.instance_eval { def foo; end }' -e:1:in `block in <main>': no class/module to add method (TypeError) After: $ ./ruby -e '123.instance_eval { def foo; end }' -e:1:in `block in <main>': can't define singleton (TypeError) IMO this error is a small improvement on the original and better matches the (both old and new) message when definging a method using `def self.` $ ruby -e '123.instance_eval{ def self.foo; end }' -e:1:in `block in <main>': can't define singleton (TypeError) Co-authored-by: Matthew Draper <matthew@t...> * Remove "under" argument from yield_under * Move CREF_SINGLETON_SET into vm_cref_new * Simplify vm_get_const_base * Fix leaf VM_SPECIAL_OBJECT_CONST_BASE Co-authored-by: Matthew Draper <matthew@t...> --- bootstraptest/test_eval.rb | 27 ++++++++++++++++++ eval_intern.h | 31 +++++++++++++++++++- gc.c | 4 +-- insns.def | 3 +- method.h | 2 +- test/ruby/test_eval.rb | 70 ++++++++++++++++++++++++++++++++++++++++++++++ test/ruby/test_undef.rb | 16 +++++++++++ vm.c | 27 ++++++++++-------- vm_eval.c | 57 +++++++++++++------------------------ vm_insnhelper.c | 38 ++++++++++--------------- 10 files changed, 197 insertions(+), 78 deletions(-) diff --git a/bootstraptest/test_eval.rb b/bootstraptest/test_eval.rb index 5d2593c3060..efc22efff40 100644 --- a/bootstraptest/test_eval.rb +++ b/bootstraptest/test_eval.rb @@ -116,6 +116,33 @@ assert_equal %q{1}, %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_eval.rb#L116 Const } } +assert_equal %q{1}, %q{ + class TrueClass + Const = 1 + end + true.instance_eval %{ + Const + } +} +assert_equal %q{[:Const]}, %q{ + mod = Module.new + mod.instance_eval %{ + Const = 1 + } + raise if defined?(Module::Const) + mod.singleton_class.constants +} +assert_equal %q{can't define singleton}, %q{ + begin + 123.instance_eval %{ + Const = 1 + } + "bad" + rescue TypeError => e + raise "bad" if defined?(Integer::Const) + e.message + end +} assert_equal %q{top}, %q{ Const = :top class C diff --git a/eval_intern.h b/eval_intern.h index 58400b5f257..e858d79c3e0 100644 --- a/eval_intern.h +++ b/eval_intern.h @@ -173,11 +173,28 @@ rb_ec_tag_jump(const rb_execution_context_t *ec, enum ruby_tag_type st) https://github.com/ruby/ruby/blob/trunk/eval_intern.h#L173 #define CREF_FL_PUSHED_BY_EVAL IMEMO_FL_USER1 #define CREF_FL_OMOD_SHARED IMEMO_FL_USER2 +#define CREF_FL_SINGLETON IMEMO_FL_USER3 + +static inline int CREF_SINGLETON(const rb_cref_t *cref); static inline VALUE CREF_CLASS(const rb_cref_t *cref) { - return cref->klass; + if (CREF_SINGLETON(cref)) { + return CLASS_OF(cref->klass_or_self); + } else { + return cref->klass_or_self; + } +} + +static inline VALUE +CREF_CLASS_FOR_DEFINITION(const rb_cref_t *cref) +{ + if (CREF_SINGLETON(cref)) { + return rb_singleton_class(cref->klass_or_self); + } else { + return cref->klass_or_self; + } } static inline rb_cref_t * @@ -216,6 +233,18 @@ CREF_PUSHED_BY_EVAL_SET(rb_cref_t *cref) https://github.com/ruby/ruby/blob/trunk/eval_intern.h#L233 cref->flags |= CREF_FL_PUSHED_BY_EVAL; } +static inline int +CREF_SINGLETON(const rb_cref_t *cref) +{ + return cref->flags & CREF_FL_SINGLETON; +} + +static inline void +CREF_SINGLETON_SET(rb_cref_t *cref) +{ + cref->flags |= CREF_FL_SINGLETON; +} + static inline int CREF_OMOD_SHARED(const rb_cref_t *cref) { diff --git a/gc.c b/gc.c index 9bdeaac2347..f889cbb81b1 100644 --- a/gc.c +++ b/gc.c @@ -6808,7 +6808,7 @@ gc_mark_imemo(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L6808 } return; case imemo_cref: - gc_mark(objspace, RANY(obj)->as.imemo.cref.klass); + gc_mark(objspace, RANY(obj)->as.imemo.cref.klass_or_self); gc_mark(objspace, (VALUE)RANY(obj)->as.imemo.cref.next); gc_mark(objspace, RANY(obj)->as.imemo.cref.refinements); return; @@ -9677,7 +9677,7 @@ gc_ref_update_imemo(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L9677 } break; case imemo_cref: - UPDATE_IF_MOVED(objspace, RANY(obj)->as.imemo.cref.klass); + UPDATE_IF_MOVED(objspace, RANY(obj)->as.imemo.cref.klass_or_self); TYPED_UPDATE_IF_MOVED(objspace, struct rb_cref_struct *, RANY(obj)->as.imemo.cref.next); UPDATE_IF_MOVED(objspace, RANY(obj)->as.imemo.cref.refinements); break; diff --git a/insns.def b/insns.def index f759c1a5c51..87624a05009 100644 --- a/insns.def +++ b/insns.def @@ -350,6 +350,7 @@ putspecialobject https://github.com/ruby/ruby/blob/trunk/insns.def#L350 (rb_num_t value_type) () (VALUE val) +// attr bool leaf = (value_type == VM_SPECIAL_OBJECT_VMCORE); /* others may raise when allocating singleton */ { enum vm_special_object_type type; @@ -715,7 +716,7 @@ defineclass https://github.com/ruby/ruby/blob/trunk/insns.def#L716 /* enter scope */ vm_push_frame(ec, class_iseq, VM_FRAME_MAGIC_CLASS | VM_ENV_FLAG_LOCAL, klass, GET_BLOCK_HANDLER(), - (VALUE)vm_cref_push(ec, klass, NULL, FALSE), + (VALUE)vm_cref_push(ec, klass, NULL, FALSE, FALSE), class_iseq->body->iseq_encoded, GET_SP(), class_iseq->body->local_table_size, class_iseq->body->stack_max); diff --git a/method.h b/method.h index 031d2ce89f3..8bff5b3b8d5 100644 --- a/method.h +++ b/method.h @@ -44,7 +44,7 @@ typedef struct rb_scope_visi_struct { https://github.com/ruby/ruby/blob/trunk/method.h#L44 typedef struct rb_cref_struct { VALUE flags; VALUE refinements; - VALUE klass; + VALUE klass_or_self; struct rb_cref_struct * next; const rb_scope_visibility_t scope_visi; } rb_cref_t; diff --git a/test/ruby/test_eval.rb b/test/ruby/test_eval.rb index bf551c68456..d55977c986f 100644 --- a/test/ruby/test_eval.rb +++ b/test/ruby/test_eval.rb @@ -219,6 +219,12 @@ class TestEval < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_eval.rb#L219 end end + def test_instance_exec_cvar + [Object.new, [], 7, :sym, true, false, nil].each do |obj| + assert_equal(13, obj.instance_exec{@@cvar}) + end + end + def test_instance_eval_method bug2788 = '[ruby-core:28324]' [Object.new, [], nil, true, false].each do |o| @@ -253,6 +259,70 @@ class TestEval < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_eval.rb#L259 assert_equal(2, bar) end + def test_instance_exec_block_basic + forall_TYPE do |o| + assert_equal nil, o.instance_exec { nil } + assert_equal true, o.instance_exec { true } + assert_equal false, o.instance_exec { false } + assert_equal o, o.instance_exec { self } + assert_equal 1, o.instance_exec { 1 } + assert_equal :sym, o.instance_exec { :sym } + + assert_equal 11, o.instance_exec { 11 } + assert_equal 12, o.instance_exec { @ivar } unless o.frozen? + assert_equal 13, o.instance_exec { @@cvar } + assert_equal 14, o.instance_exec { $gvar__eval } + assert_equal 15, o.instance_exec { Const } + assert_equal 16, o.instance_exec { 7 + 9 } + assert_equal 17, o.instance_exec { 17.to_i } + assert_equal "18", o.instance_exec { "18" } + assert_equal "19", o.instance_exec { "1#{9}" } + + 1.times { + assert_equal 12, o.instance_exec { @ivar } unless o.frozen? + assert_equal 13, o.instance_exec { @@cvar } + assert_equal 14, o.instance_exec { $gvar__eval } + assert_equal 15, o.instance_exec { Const } + } + end + end + + def test_instance_exec_method_definition + klass = Class.new + o = klass.new + + o.instance_exec do + def foo + :foo_result + end + end + + assert_respond_to o, :foo + refute_respond_to klass, :foo + refute_respond_to klass.new, :foo + + assert_equal :foo_result, o.foo + end + + def test_instance_exec_eval_method_definition + klass = Class.new + o = klass.new + + o.instance_exec do + eval %{ + def foo + :foo_result + end + } + end + + assert_respond_to o, :foo + refute_respond_to klass, (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/