[ANN] patch to "make def return something useful"

[Peter]

In RCR 277 it is proposed to have def return something useful, more
specifically the symbol under which the method is defined. Something like
this has been proposed/requested a few times on ruby-talk too (dating back
as far as 2001 [ruby-talk:15434]).

At https://developer.berlios.de/project/showfiles.php?group_id=1216&release_id=4105
you can find a patch that implements the idea but returns an UnboundMethod
instead of a symbol (or a Method when defining a singleton method). It
extends Module#public/private/protected to take a single (Unbound)?Method
as parameter. The patch is against the source of Ruby 1.8.2.

I'm still in the dark about the true speed hit of this change, as it
actually seems to run slightly faster on my machine, or at least for the
few tests I've conducted.

Disclaimer: the existence of this patch does not reflect whether or not
I'm in favor of actually having this in Ruby. I'm just building these
things because that is the best way for me to learn about Ruby's 'guts'.

Peter

 

[Austin Ziegler]

In RCR 277 it is proposed to have def return something useful, more
specifically the symbol under which the method is defined. Something like
this has been proposed/requested a few times on ruby-talk too (dating back
as far as 2001 [ruby-talk:15434]).

At https://developer.berlios.de/project/showfiles.php?group_id=1216&release_id=4105
you can find a patch that implements the idea but returns an UnboundMethod
instead of a symbol (or a Method when defining a singleton method). It
extends Module#public/private/protected to take a single (Unbound)?Method
as parameter. The patch is against the source of Ruby 1.8.2.

I'm still in the dark about the true speed hit of this change, as it
actually seems to run slightly faster on my machine, or at least for the
few tests I've conducted.

Disclaimer: the existence of this patch does not reflect whether or not
I'm in favor of actually having this in Ruby. I'm just building these
things because that is the best way for me to learn about Ruby's 'guts'.

A Method still isn't good, because you can't easily get the name of
the Method without calling #inspect.

-austin

 

[nobu.nokada]

Hi,

At Mon, 6 Dec 2004 09:39:43 +0900,
Peter wrote in [ruby-talk:122619]:

At https://developer.berlios.de/project/showfiles.php?group_id=1216&release_id=4105
you can find a patch that implements the idea but returns an UnboundMethod
instead of a symbol (or a Method when defining a singleton method). It
extends Module#public/private/protected to take a single (Unbound)?Method
as parameter. The patch is against the source of Ruby 1.8.2.

I wonder if this might cause confution.

class Foo
public def foo
end
end
class Bar
private Foo.instance_methodfoo)
end

Aren't Method#public etc. superfluous?


Index: eval.c
===================================================================
RCS file: /cvs/ruby/src/ruby/eval.c,v
retrieving revision 1.742
diff -U2 -p -d -r1.742 eval.c
--- eval.c 3 Dec 2004 04:56:24 -0000 1.742
+++ eval.c 6 Dec 2004 02:50:59 -0000
@@ -147,4 +147,7 @@ static VALUE rb_mod_define_method _((int
NORETURN(static void rb_raise_jump _((VALUE)));
static VALUE rb_make_exception _((int argc, VALUE *argv));
+static VALUE um_new _((VALUE klass, NODE *body, ID id));
+static VALUE m_new _((VALUE klass, VALUE recv, NODE *body, ID id));
+static ID check_method_id _((VALUE obj, VALUE mod));

static int scope_vmode;
@@ -315,5 +318,5 @@ static ID added, singleton_added;
static ID __id__, __send__, respond_to;

-void
+NODE *
rb_add_method(klass, mid, node, noex)
VALUE klass;
@@ -351,4 +354,5 @@ rb_add_method(klass, mid, node, noex)
}
}
+ return node;
}

@@ -3671,10 +3675,10 @@ rb_eval(self, n)

defn = copy_node_scope(node->nd_defn, ruby_cref);
- rb_add_method(ruby_class, node->nd_mid, defn, noex);
+ body = rb_add_method(ruby_class, node->nd_mid, defn, noex);
if (scope_vmode == SCOPE_MODFUNC) {
rb_add_method(rb_singleton_class(ruby_class),
node->nd_mid, defn, NOEX_PUBLIC);
}
- result = Qnil;
+ result = um_new(ruby_class, body, node->nd_mid);
}
break;
@@ -3707,7 +3711,7 @@ rb_eval(self, n)
}
defn = copy_node_scope(node->nd_defn, ruby_cref);
- rb_add_method(klass, node->nd_mid, defn,
+ body = rb_add_method(klass, node->nd_mid, defn,
NOEX_PUBLIC|(body?body->nd_noex&NOEX_UNDEF:0));
- result = Qnil;
+ result = m_new(klass, recv, body, node->nd_mid);
}
break;
@@ -6926,8 +6930,13 @@ set_method_visibility(self, argc, argv,
{
int i;
+ VALUE m;
+ ID id;

secure_visibility(self);
for (i=0; i<argc; i++) {
- rb_export_method(self, rb_to_id(argv), ex);
+ m = argv;
+ id = check_method_id(m, self);
+ if (!id) id = rb_to_id(m);
+ rb_export_method(self, id, ex);
}
rb_clear_cache_by_class(self);
@@ -7148,20 +7157,22 @@ rb_mod_modfunc(argc, argv, module)
set_method_visibility(module, argc, argv, NOEX_PRIVATE);
for (i=0; i<argc; i++) {
- VALUE m = module;
+ if (!(id = check_method_id(argv, module))) {
+ VALUE m = module;

- id = rb_to_id(argv);
- for (; {
- body = search_method(m, id, &m);
- if (body == 0) {
- body = search_method(rb_cObject, id, &m);
- }
- if (body == 0 || body->nd_body == 0) {
- rb_bug("undefined method `%s'; can't happen", rb_id2name(id));
- }
- if (nd_type(body->nd_body) != NODE_ZSUPER) {
- break; /* normal case: need not to follow 'super' link */
+ id = rb_to_id(argv);
+ for (; {
+ body = search_method(m, id, &m);
+ if (body == 0) {
+ body = search_method(rb_cObject, id, &m);
+ }
+ if (body == 0 || body->nd_body == 0) {
+ rb_bug("undefined method `%s'; can't happen", rb_id2name(id));
+ }
+ if (nd_type(body->nd_body) != NODE_ZSUPER) {
+ break; /* normal case: need not to follow 'super' link */
+ }
+ m = RCLASS(m)->super;
+ if (!m) break;
}
- m = RCLASS(m)->super;
- if (!m) break;
}
rb_add_method(rb_singleton_class(module), id, body->nd_body, NOEX_PUBLIC);
@@ -8635,5 +8646,5 @@ mnew(klass, obj, id, mklass)
}
if (TYPE(klass) == T_ICLASS) klass = RBASIC(klass)->klass;
- method = Data_Make_Struct(mklass, struct METHOD, bm_mark, free, data);
+ method = Data_Make_Struct(mklass, struct METHOD, bm_mark, -1, data);
data->klass = klass;
data->recv = obj;
@@ -8647,4 +8658,79 @@ mnew(klass, obj, id, mklass)
}

+static VALUE
+um_new(klass, body, id)
+ VALUE klass;
+ ID id;
+ NODE *body;
+{
+ VALUE method;
+ struct METHOD *data;
+
+ method = Data_Make_Struct(rb_cUnboundMethod, struct METHOD, bm_mark, -1, data);
+ data->klass = klass;
+ data->recv = Qundef;
+ data->id = id;
+ data->body = body;
+ data->rklass = klass;
+ data->oid = id;
+ OBJ_INFECT(method, klass);
+
+ return method;
+}
+
+static VALUE
+m_new(klass, recv, body, id)
+ VALUE klass, recv;
+ ID id;
+ NODE *body;
+{
+ VALUE method;
+ struct METHOD *data;
+
+ method = Data_Make_Struct(rb_cMethod, struct METHOD, bm_mark, -1, data);
+ data->klass = klass;
+ data->recv = recv;
+ data->id = id;
+ data->body = body;
+ data->rklass = klass;
+ data->oid = id;
+ OBJ_INFECT(method, klass);
+
+ return method;
+}
+
+static ID
+check_method_id(obj, mod)
+ VALUE obj, mod;
+{
+ struct METHOD *data;
+ VALUE klass;
+ ID id;
+ ID noex;
+ NODE *body;
+
+ if (TYPE(obj) != T_DATA || RDATA(obj)->dmark != bm_mark)
+ return 0;
+ data = (struct METHOD *)RDATA(obj)->data;
+ if (data->rklass != mod) {
+ rb_raise(rb_eTypeError, "%s mismatch - %s for %s",
+ (TYPE(mod) == T_CLASS ? "class" : "module"),
+ rb_obj_as_string(data->rklass), rb_obj_as_string(mod));
+ }
+ for (klass = data->rklass, id = data->oid;; klass = RCLASS(klass)->super) {
+ if ((body = rb_get_method_body(&klass, &id, &noex)) == 0) {
+ rb_raise(rb_eTypeError, "method %s in %s disappeared",
+ rb_id2name(data->oid), rb_class2name(data->rklass));
+ }
+
+ if (nd_type(body) != NODE_ZSUPER) break;
+ }
+
+ if (body != data->body) {
+ rb_raise(rb_eTypeError, "method %s in %s changed",
+ rb_id2name(data->oid), rb_class2name(data->rklass));
+ }
+ return data->oid;
+}

/**********************************************************************

 

[Peter]

I wonder if this might cause confution.

class Foo
public def foo
end
end
class Bar
private Foo.instance_methodfoo)
end

Right, at some point I thought of this, but forgot to actually add the
check.

BTW, what is the reason for setting the method's free routine to -1?

Aren't Method#public etc. superfluous?

Yep, just that that was the only way I could get it to work right with my
still limited knowledge Thanks for showing me how to remove them.

But I wonder: the download page shows 0 downloads for version 0.2 of the
patch, but you obviously must have downloaded it. Did you do something
special to bypass the counter or is it just way off?

Peter

 

[Peter]

A Method still isn't good, because you can't easily get the name of

the Method without calling #inspect.

You're right, the Ruby programmer can't get at the name of the method not
the class it is defined in. Inside the Ruby core it is of course very
easy. There are no (Unbound)?Method#(name|class), probably because there
was no use for them, but is there another reason against adding these
methods?

Peter

 

[nobu.nokada]

Hi,

At Mon, 6 Dec 2004 17:35:06 +0900,
Peter wrote in [ruby-talk:122653]:

BTW, what is the reason for setting the method's free routine to -1?

gc.cbj_free() frees it using the critcal section, if dfree is
-1.

But I wonder: the download page shows 0 downloads for version 0.2 of the
patch, but you obviously must have downloaded it. Did you do something
special to bypass the counter or is it just way off?

Nothing special, I just got it with wget.

 

[Peter]

Yep, just that that was the only way I could get it to work right with my

still limited knowledge Thanks for showing me how to remove them.

But maybe an extra clarification: in the end I settled for the solution
with Method#private! instead of looking further for the bug that plagued
my other approach. The reason was that in a way it made sense to have
those methods as well because even though "private def a ; end" looks
nicer, it is IMO less OO than Method#private! . The downside of these
methods is that they are public, while Module#private is private, which
makes it very easy to change the class without reopening it (and, as I
realize now, bypass the check of $SAFE). Any thoughts on this? Other
pros/cons?

Peter

 

[Peter]

gc.cbj_free() frees it using the critcal section, if dfree is

-1.

I see. Is this necessary because of the change I made? Because even in 1.9
in eval.c:mnew() dfree is still set to free instead of -1. I admit that
I'm not fully clear on when to use a critical section and when not. From
what I can tell, a piece of pure C code should not get interrupted, right?

Nothing special, I just got it with wget.

Maybe the counter updates only daily or so. Never mind.

Peter

 

[Martin DeMello]

But maybe an extra clarification: in the end I settled for the solution
with Method#private! instead of looking further for the bug that plagued
my other approach. The reason was that in a way it made sense to have
those methods as well because even though "private def a ; end" looks
nicer, it is IMO less OO than Method#private! . The downside of these

I'd disagree with this - privacy is done at the class level rather than
the method level. Method#private! smacks too much of having a contained
object know about the container.

martin

 

[Peter]

I'd disagree with this - privacy is done at the class level rather than

the method level. Method#private! smacks too much of having a contained
object know about the container.

Maybe I'm a bit biased, having seen the underlying implementation. Method
knows about its container, and in the end privacy is determined by both
the class and the method, and part of a Method (bound or unbound) IMHO
represents the combination of these two. And maybe this is because I'm not
quite content with the alternative where Module#private checks if the
class of the Method passed to it matches its own and raises an error if it
is not so. This is related again to Method being the combination of class
and method.

Peter

 

[nobu.nokada]

Hi,

At Mon, 6 Dec 2004 19:05:44 +0900,
Peter wrote in [ruby-talk:122658]:

I see. Is this necessary because of the change I made? Because even in 1.9
in eval.c:mnew() dfree is still set to free instead of -1. I admit that
I'm not fully clear on when to use a critical section and when not. From
what I can tell, a piece of pure C code should not get interrupted, right?

It is used to postpone signal handlers. Especially, on
MS-Windows, they are called from another native thread and
executed on the main native thread.

 

[Austin Ziegler]

Maybe I'm a bit biased, having seen the underlying implementation. Method
knows about its container, and in the end privacy is determined by both
the class and the method, and part of a Method (bound or unbound) IMHO
represents the combination of these two.

IMO, that's an implementation detail that shouldn't be exposed at the
metaprogramming level.

-austin

 

[Austin Ziegler]

You're right, the Ruby programmer can't get at the name of the method not
the class it is defined in. Inside the Ruby core it is of course very
easy. There are no (Unbound)?Method#(name|class), probably because there
was no use for them, but is there another reason against adding these
methods?

Yes, but the whole point of having def return something useful is to
make it easier for someone to use this information at the Ruby level,
not at the Ruby core level. Returning a Method or UnboundMethod is,
ultimately, useless for meta-programming.

-austin

 

[Peter]

IMO, that's an implementation detail that shouldn't be exposed at the

metaprogramming level.

Part of it is exposed indirectly. For example the class matters when
binding a method, and there could be applications that want to check this
class before binding an UnboundMethod.

Yes, but the whole point of having def return something useful is to
make it easier for someone to use this information at the Ruby level,
not at the Ruby core level. Returning a Method or UnboundMethod is,
ultimately, useless for meta-programming.

I'll repeat my disclaimer here: I made this patch, but that does not mean
that it shows how I think it should be. I've taken an idea from an RCR and
combined it with an idea I've seen come up at suby-muse and ruby-talk. I'm
not fully happy with it myself. A Method or UnboundMethod is detached from
the class it was taken from, and that class nor its name really matters
after it's been detached from the class, except in tiny details, such as
an UnboundMethod that can only be bound to an object with the original
class of the method in its ancestors. Because of this, the implementation
actually needs to check whether that method is still in the original class
and whether it is not redefined, resulting in an exception when it is gone
or redefined. In a way, even if Method provided methods for getting the
method name and class, it is the wrong information to pass to
Module#(public|protected|private) because the info contained in Method
may be out of date.

But what should it return then? A method name only isn't always
sufficient, e.g., for class methods. Returning an array containing the
name of the method and the name of the class isn't sufficient either
because classes can be anonymous. Returning the class itself instead of
just the name doesn't work for class methods, so for class methods (and
singleton methods in general), it should return the singleton class. So
the def in "class A ; def foo; end ; end" should return [:foo, A] and "def
a.foo ; end" should return [:foo, class << a ; self ; end]. That would
work, but I'd rather have def return something that I can ask for its name
or attached class instead of element 0 and element 1. A Struct of some
kind maybe?

Peter