diff options
author | John Hawthorn <john@hawthorn.email> | 2021-12-02 15:53:39 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-12-02 15:53:39 -0800 |
commit | 733500e9d02b11ff60fbbdb8daa43c2e9cfbd750 (patch) | |
tree | 1d253644dcfd6166bfb4f0b505c0c5bb659f0ddb /vm_insnhelper.c | |
parent | 1f4af993835219efa8feaf76a0b36252028691f1 (diff) |
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@trebex.net>
* 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@trebex.net>
Notes
Notes:
Merged-By: jhawthorn <john@hawthorn.email>
Diffstat (limited to 'vm_insnhelper.c')
-rw-r--r-- | vm_insnhelper.c | 38 |
1 files changed, 14 insertions, 24 deletions
diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 741deb6b42..c6b079e7d5 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -892,7 +892,7 @@ rb_vm_rewrite_cref(rb_cref_t *cref, VALUE old_klass, VALUE new_klass, rb_cref_t } static rb_cref_t * -vm_cref_push(const rb_execution_context_t *ec, VALUE klass, const VALUE *ep, int pushed_by_eval) +vm_cref_push(const rb_execution_context_t *ec, VALUE klass, const VALUE *ep, int pushed_by_eval, int singleton) { rb_cref_t *prev_cref = NULL; @@ -907,40 +907,30 @@ vm_cref_push(const rb_execution_context_t *ec, VALUE klass, const VALUE *ep, int } } - return vm_cref_new(klass, METHOD_VISI_PUBLIC, FALSE, prev_cref, pushed_by_eval); + return vm_cref_new(klass, METHOD_VISI_PUBLIC, FALSE, prev_cref, pushed_by_eval, singleton); } static inline VALUE vm_get_cbase(const VALUE *ep) { const rb_cref_t *cref = vm_get_cref(ep); - VALUE klass = Qundef; - while (cref) { - if ((klass = CREF_CLASS(cref)) != 0) { - break; - } - cref = CREF_NEXT(cref); - } - - return klass; + return CREF_CLASS_FOR_DEFINITION(cref); } static inline VALUE vm_get_const_base(const VALUE *ep) { const rb_cref_t *cref = vm_get_cref(ep); - VALUE klass = Qundef; while (cref) { - if (!CREF_PUSHED_BY_EVAL(cref) && - (klass = CREF_CLASS(cref)) != 0) { - break; - } - cref = CREF_NEXT(cref); + if (!CREF_PUSHED_BY_EVAL(cref)) { + return CREF_CLASS_FOR_DEFINITION(cref); + } + cref = CREF_NEXT(cref); } - return klass; + return Qundef; } static inline void @@ -1060,7 +1050,7 @@ vm_get_cvar_base(const rb_cref_t *cref, const rb_control_frame_t *cfp, int top_l while (CREF_NEXT(cref) && (NIL_P(CREF_CLASS(cref)) || FL_TEST(CREF_CLASS(cref), FL_SINGLETON) || - CREF_PUSHED_BY_EVAL(cref))) { + CREF_PUSHED_BY_EVAL(cref) || CREF_SINGLETON(cref))) { cref = CREF_NEXT(cref); } if (top_level_raise && !CREF_NEXT(cref)) { @@ -4657,14 +4647,14 @@ vm_define_method(const rb_execution_context_t *ec, VALUE obj, ID id, VALUE iseqv rb_method_visibility_t visi; rb_cref_t *cref = vm_ec_cref(ec); - if (!is_singleton) { - klass = CREF_CLASS(cref); - visi = vm_scope_visibility_get(ec); - } - else { /* singleton */ + if (is_singleton) { klass = rb_singleton_class(obj); /* class and frozen checked in this API */ visi = METHOD_VISI_PUBLIC; } + else { + klass = CREF_CLASS_FOR_DEFINITION(cref); + visi = vm_scope_visibility_get(ec); + } if (NIL_P(klass)) { rb_raise(rb_eTypeError, "no class/module to add method"); |