[ruby-dev:27004] Re: [proposal] ANSI style function

山本です。

>varはvariable.cの中で使っているはずですが。いつのまにか直し
>ちゃったのかな？

ああ、そういえば rb_define_virtual_variable の仕様がよくわからないまま、
rb_define_virtual_variable で 

struct global_variable {
    int   counter;
    void *data;

の data にアクセスしているファイルもないし、デフォルトの挙動も getter が
Qfalse(0) を返すような挙動でよくわからなかったので、
とりあえず引数なし関数ポインタを追放するとどんな感じになるか知るのを優先して
内部ストレージ (data) には触れない実装にしてしまいました。（virtual というぐらいだし）

# virtual でも、内部ストレージを使うんでしょうか？

あと、protoize が変更部分の改行文字を LF にしてしまっていたようで、
http://www.ccsnet.ne.jp/~ocean/ のパッチにも余計な変更（改行の変更のみの行）
が含まれていたので、差し替えました。

加えて、

>var に NULL を渡すようにして
>同じ型にしたほうがいいでしょうか？コールバックを書くときに混乱したので
>（というか、そうすればunion を使わなくてもよかったかも）

の方針でとりあえず make できるようにしてみました。
一通りやってみてネックに感じたのは

  1. コールバック関数を定義している側にも変更が必要。いままでファジーに
     VALUE hoge(void) な関数を getter に使っていたような部分も修正する
     必要がある

これはしかたないとしても、

  2. 内部の挙動も知る必要がある

これは誤算でした。

Index: eval.c
===================================================================
--- eval.c	(revision 6)
+++ eval.c	(working copy)
@@ -223,8 +223,8 @@
    4 - no global (non-tainted) variable modification/no direct output
 */
 
-static VALUE safe_getter _((void));
-static void safe_setter _((VALUE val));
+static VALUE safe_getter(ID id, VALUE *var);
+static void safe_setter(VALUE val, ID id, VALUE *var);
 
 void
 rb_secure(int level)
@@ -7305,7 +7305,7 @@
 }
 
 static VALUE
-errat_getter(ID id)
+errat_getter(ID id, VALUE *var)
 {
     return get_backtrace(ruby_errinfo);
 }
@@ -9808,13 +9808,13 @@
 }
 
 static VALUE
-safe_getter(void)
+safe_getter(ID id, VALUE *var)
 {
     return INT2NUM(ruby_safe_level);
 }
 
 static void
-safe_setter(VALUE val)
+safe_setter(VALUE val, ID id, VALUE *var)
 {
     int level = NUM2INT(val);
 
Index: io.c
===================================================================
--- io.c	(revision 3)
+++ io.c	(working copy)
@@ -5300,14 +5300,14 @@
 }
 
 static VALUE
-opt_i_get(void)
+opt_i_get(ID id, VALUE *var)
 {
     if (!ruby_inplace_mode) return Qnil;
     return rb_str_new2(ruby_inplace_mode);
 }
 
 static void
-opt_i_set(VALUE val)
+opt_i_set(VALUE val, ID id, VALUE *var)
 {
     if (!RTEST(val)) {
 	if (ruby_inplace_mode) free(ruby_inplace_mode);
@@ -5320,6 +5320,18 @@
     ruby_inplace_mode = strdup(RSTRING(val)->ptr);
 }
 
+static VALUE
+lastline_get(ID id, VALUE *var)
+{
+    return rb_lastline_get();
+}
+
+static void
+lastline_set(VALUE val, ID id, VALUE *var)
+{
+    rb_lastline_set(val);
+}
+
 /*
  *  Class <code>IO</code> is the basis for all input and output in Ruby.
  *  An I/O stream may be <em>duplexed</em> (that is, bidirectional), and
@@ -5468,7 +5480,7 @@
     rb_define_hooked_variable("$\\", &rb_output_rs, 0, rb_str_setter);
 
     rb_define_hooked_variable("$.", &lineno, 0, lineno_setter);
-    rb_define_virtual_variable("$_", rb_lastline_get, rb_lastline_set);
+    rb_define_virtual_variable("$_", lastline_get, lastline_set);
 
     rb_define_method(rb_cIO, "initialize_copy", rb_io_init_copy, 1);
     rb_define_method(rb_cIO, "reopen", rb_io_reopen, -1);
Index: process.c
===================================================================
--- process.c	(revision 2)
+++ process.c	(working copy)
@@ -3540,6 +3540,11 @@
 VALUE rb_mProcGID;
 VALUE rb_mProcID_Syscall;
 
+static VALUE
+pid_getter(ID id, VALUE *var)
+{
+    return get_pid();
+}
 
 /*
  *  The <code>Process</code> module is a collection of methods used to
@@ -3549,7 +3554,7 @@
 void
 Init_process(void)
 {
-    rb_define_virtual_variable("$$", get_pid, 0);
+    rb_define_virtual_variable("$$", pid_getter, 0);
     rb_define_readonly_variable("$?", &rb_last_status);
     rb_define_global_function("exec", rb_f_exec, -1);
     rb_define_global_function("fork", rb_f_fork, 0);
Index: re.c
===================================================================
--- re.c	(revision 3)
+++ re.c	(working copy)
@@ -1060,25 +1060,25 @@
 }
 
 static VALUE
-last_match_getter(void)
+last_match_getter(ID id, VALUE *var)
 {
     return rb_reg_last_match(rb_backref_get());
 }
 
 static VALUE
-prematch_getter(void)
+prematch_getter(ID id, VALUE *var)
 {
     return rb_reg_match_pre(rb_backref_get());
 }
 
 static VALUE
-postmatch_getter(void)
+postmatch_getter(ID id, VALUE *var)
 {
     return rb_reg_match_post(rb_backref_get());
 }
 
 static VALUE
-last_paren_match_getter(void)
+last_paren_match_getter(ID id, VALUE *var)
 {
     return rb_reg_match_last(rb_backref_get());
 }
@@ -2080,7 +2080,7 @@
 }
 
 static VALUE
-kcode_getter(void)
+kcode_getter(ID id, VALUE *var)
 {
     return rb_str_new2(rb_get_kcode());
 }
@@ -2119,20 +2119,20 @@
 }
 
 static void
-kcode_setter(VALUE val)
+kcode_setter(VALUE val, ID id, VALUE *var)
 {
     may_need_recompile = 1;
     rb_set_kcode(StringValuePtr(val));
 }
 
 static VALUE
-ignorecase_getter(void)
+ignorecase_getter(ID id, VALUE *var)
 {
     return ruby_ignorecase?Qtrue:Qfalse;
 }
 
 static void
-ignorecase_setter(VALUE val, ID id)
+ignorecase_setter(VALUE val, ID id, VALUE *var)
 {
     rb_warn("modifying %s is deprecated", rb_id2name(id));
     may_need_recompile = 1;
@@ -2140,7 +2140,7 @@
 }
 
 static VALUE
-match_getter(void)
+get_match(void)
 {
     VALUE match = rb_backref_get();
 
@@ -2149,8 +2149,14 @@
     return match;
 }
 
+static VALUE
+match_getter(ID id, VALUE *var)
+{
+    return get_match();
+}
+
 static void
-match_setter(VALUE val)
+match_setter(VALUE val, ID id, VALUE *var)
 {
     if (!NIL_P(val)) {
 	Check_Type(val, T_MATCH);
@@ -2183,7 +2189,7 @@
     if (rb_scan_args(argc, argv, "01", &nth) == 1) {
 	return rb_reg_nth_match(NUM2INT(nth), rb_backref_get());
     }
-    return match_getter();
+    return get_match();
 }
 
 
Index: ruby.c
===================================================================
--- ruby.c	(revision 2)
+++ ruby.c	(working copy)
@@ -1023,7 +1023,7 @@
 #endif
 
 static void
-set_arg0(VALUE val, ID id)
+set_arg0(VALUE val, ID id, VALUE *var)
 {
     char *s;
     long i;
@@ -1141,7 +1141,7 @@
 }
 
 static VALUE
-opt_W_getter(VALUE val, ID id)
+opt_W_getter(ID id, VALUE *var)
 {
     if (ruby_verbose == Qnil) return INT2FIX(0);
     if (ruby_verbose == Qfalse) return INT2FIX(1);
Index: ruby.h
===================================================================
--- ruby.h	(revision 1)
+++ ruby.h	(working copy)
@@ -493,8 +493,8 @@
 void rb_extend_object _((VALUE,VALUE));
 
 void rb_define_variable _((const char*,VALUE*));
-void rb_define_virtual_variable _((const char*,VALUE(*)(ANYARGS),void(*)(ANYARGS)));
-void rb_define_hooked_variable _((const char*,VALUE*,VALUE(*)(ANYARGS),void(*)(ANYARGS)));
+void rb_define_virtual_variable (const char*,VALUE(*)(ID,VALUE*),void(*)(VALUE,ID,VALUE*));
+void rb_define_hooked_variable (const char*,VALUE*,VALUE(*)(ID,VALUE*),void(*)(VALUE,ID,VALUE*));
 void rb_define_readonly_variable _((const char*,VALUE*));
 void rb_define_const _((VALUE,const char*,VALUE));
 void rb_define_global_const _((const char*,VALUE));
Index: variable.c
===================================================================
--- variable.c	(revision 3)
+++ variable.c	(working copy)
@@ -284,17 +284,19 @@
 
 struct trace_var {
     int removed;
-    void (*func)();
+    void (*func)(VALUE, VALUE);
     VALUE data;
     struct trace_var *next;
 };
 
+enum globval_variable_state { GVAR_UNDEF, GVAR_VALUE, GVAR_VIRTUAL, GVAR_HOOK };
+
 struct global_variable {
-    int   counter;
-    void *data;
-    VALUE (*getter)();
-    void  (*setter)();
-    void  (*marker)();
+    int counter;
+    enum global_variable_state state;
+    VALUE data; /* used as VALUE when GVAR_VALUE. used as VALUE* when GVAR_HOOK */
+    VALUE (*getter)(ID id, VALUE *var); /* used when GVAR_VIRTUAL or GVER_HOOK */
+    void  (*setter)(VALUE val, ID id, VALUE *var); /* ditto */
     int block_trace;
     struct trace_var *trace;
 };
@@ -304,18 +306,6 @@
     ID id;
 };
 
-static VALUE undef_getter(ID id);
-static void  undef_setter(VALUE val, ID id, void *data, struct global_variable *var);
-static void  undef_marker(void);
-
-static VALUE val_getter(ID id, VALUE val);
-static void  val_setter(VALUE val, ID id, void *data, struct global_variable *var);
-static void  val_marker(VALUE data);
-
-static VALUE var_getter(ID id, VALUE *var);
-static void  var_setter(VALUE val, ID id, VALUE *var);
-static void  var_marker(VALUE *var);
-
 struct global_entry*
 rb_global_entry(ID id)
 {
@@ -328,11 +318,7 @@
 	entry->id = id;
 	entry->var = var;
 	var->counter = 1;
-	var->data = 0;
-	var->getter = undef_getter;
-	var->setter = undef_setter;
-	var->marker = undef_marker;
-
+	var->state = GVAR_UNDEF;
 	var->block_trace = 0;
 	var->trace = 0;
 	st_add_direct(rb_global_tbl, id, (st_data_t)entry);
@@ -341,67 +327,26 @@
 }
 
 static VALUE
-undef_getter(ID id)
+default_getter(ID id, VALUE *var)
 {
-    rb_warning("global variable `%s' not initialized", rb_id2name(id));
-
-    return Qnil;
-}
-
-static void
-undef_setter(VALUE val, ID id, void *data, struct global_variable *var)
-{
-    var->getter = val_getter;
-    var->setter = val_setter;
-    var->marker = val_marker;
-
-    var->data = (void*)val;
-}
-
-static void
-undef_marker(void)
-{
-}
-
-static VALUE
-val_getter(ID id, VALUE val)
-{
-    return val;
-}
-
-static void
-val_setter(VALUE val, ID id, void *data, struct global_variable *var)
-{
-    var->data = (void*)val;
-}
-
-static void
-val_marker(VALUE data)
-{
-    if (data) rb_gc_mark_maybe(data);
-}
-
-static VALUE
-var_getter(ID id, VALUE *var)
-{
     if (!var) return Qnil;
     return *var;
 }
 
 static void
-var_setter(VALUE val, ID id, VALUE *var)
+default_setter(VALUE val, ID id, VALUE *var)
 {
     *var = val;
 }
 
-static void
-var_marker(VALUE *var)
+static VALUE
+writeonly_getter(ID id, VALUE *var)
 {
-    if (var) rb_gc_mark_maybe(*var);
+    rb_name_error(id, "%s is a write-only variable", rb_id2name(id));
 }
 
 static void
-readonly_setter(VALUE val, ID id, void *var)
+readonly_setter(VALUE val, ID id, VALUE *var)
 {
     rb_name_error(id, "%s is a read-only variable", rb_id2name(id));
 }
@@ -412,7 +357,15 @@
     struct trace_var *trace;
     struct global_variable *var = entry->var;
 
-    (*var->marker)(var->data);
+    switch (var->state) {
+      case GVAR_VALUE:
+	rb_gc_mark_maybe(var->data);
+	break;
+      case GVAR_HOOK:
+	if (var->data) rb_gc_mark_maybe(*((VALUE *)var->data));
+	break;
+    }
+
     trace = var->trace;
     while (trace) {
 	if (trace->data) rb_gc_mark_maybe(trace->data);
@@ -446,17 +399,17 @@
 rb_define_hooked_variable(
     const char *name,
     VALUE *var,
-    VALUE (*getter) (/* ??? */),
-    void  (*setter) (/* ??? */))
+    VALUE (*getter) (ID,VALUE*),
+    void  (*setter) (VALUE,ID,VALUE*))
 {
     struct global_variable *gvar;
     ID id = global_id(name);
 
     gvar = rb_global_entry(id)->var;
-    gvar->data = (void*)var;
-    gvar->getter = getter?getter:var_getter;
-    gvar->setter = setter?setter:var_setter;
-    gvar->marker = var_marker;
+    gvar->state = GVAR_HOOK;
+    gvar->data = (VALUE)var;
+    gvar->getter = getter?getter:default_getter;
+    gvar->setter = setter?setter:default_setter;
 }
 
 void
@@ -474,12 +427,16 @@
 void
 rb_define_virtual_variable(
     const char *name,
-    VALUE (*getter) (/* ??? */),
-    void  (*setter) (/* ??? */))
+    VALUE (*getter) (ID,VALUE*),
+    void  (*setter) (VALUE,ID,VALUE*))
 {
-    if (!getter) getter = val_getter;
-    if (!setter) setter = readonly_setter;
-    rb_define_hooked_variable(name, 0, getter, setter);
+    struct global_variable *gvar;
+    ID id = global_id(name);
+
+    gvar = rb_global_entry(id)->var;
+    gvar->state = GVAR_VIRTUAL;
+    gvar->getter = getter?getter:writeonly_getter;
+    gvar->setter = setter?setter:readonly_setter;
 }
 
 static void
@@ -616,7 +573,21 @@
 rb_gvar_get(struct global_entry *entry)
 {
     struct global_variable *var = entry->var;
-    return (*var->getter)(entry->id, var->data, var);
+
+    switch (var->state) {
+      case GVAR_UNDEF:
+	rb_warning("global variable `%s' not initialized", rb_id2name(entry->id));
+	return Qnil;
+      case GVAR_VALUE:
+	return var->data;
+      case GVAR_VIRTUAL:
+	return (*var->getter)(entry->id, NULL);
+      case GVAR_HOOK:
+	return (*var->getter)(entry->id, (VALUE*)var->data);
+    }
+
+    rb_bug("should not reach here");
+    return Qnil; /* dummy */
 }
 
 struct trace_data {
@@ -652,8 +623,21 @@
 
     if (rb_safe_level() >= 4)
 	rb_raise(rb_eSecurityError, "Insecure: can't change global variable value");
-    (*var->setter)(val, entry->id, var->data, var);
 
+    switch (var->state) {
+      case GVAR_UNDEF:
+	var->state = GVAR_VALUE;
+      case GVAR_VALUE:
+        var->data = val;
+        break;
+      case GVAR_VIRTUAL:
+	(*var->setter)(val, entry->id, NULL);
+	break;
+      case GVAR_HOOK:
+	(*var->setter)(val, entry->id, (VALUE*)var->data);
+	break;
+    }
+
     if (var->trace && !var->block_trace) {
 	var->block_trace = 1;
 	trace.trace = var->trace;
@@ -684,7 +668,7 @@
 VALUE
 rb_gvar_defined(struct global_entry *entry)
 {
-    if (entry->var->getter == undef_getter) return Qfalse;
+    if (entry->var->state == GVAR_UNDEF) return Qfalse;
     return Qtrue;
 }