Nobu Grades My Homework #
It’s like I’ve got these superintellegent falcons looking over my shoulders. And, occassionally, when I truly screw up, they swoop down and wrest the keyboard from me and pound up some superior extension code. And I’m fine with these superintellegent falcons. I would feed them hambone and mouse bellies, but they are hovering too distantly, far across the world, peering at me through a loose association of ssh keys.
I maintain Syck, the YAML processor which comes with Ruby. And while I handle the large body of commits to Syck (with a huge one coming up very soon), every few months I get a surprise when Nobu or Matz dips into my code and crosses a huge calligraphic T.
I’ve been sorting through a largish diff from a few months ago, working on merging it into Syck 0.54. Here’s a few things I’ve picked up from watching Nobu that I thought I’d pass on:
On checking strings, hashes, and arrays.
This is documented in the PickAxe II. So it’s not terribly new news, but I thought I would reiterate here, since it’s an important change in the 1.8 series.
With YAML, I’m boiling every object down into a string, array or hash. Typing tags are used in the YAML to indicate what that object is in Ruby’s native types. (For example, the number 1
in YAML is loaded as an integer, but you can also get an integer by using !int "1"
.) In one part of the extension, I need to determine if an incoming VALUE is a string, array or hash. My previous code looked something like this:
if ( rb_respond_to( obj, rb_intern("to_str") ) ) { /* string conversion to YAML */ } else if ( rb_obj_is_kind_of( obj, rb_cArray ) ) { /* array conversion to YAML */ } else if ( rb_obj_is_kind_of( obj, rb_cHash ) ) { /* hash conversion to YAML */ } else { /* raise exception */ }
Nobu replaced the code with the new 1.8 duck-friendly turnstyle:
VALUE tmp; if (!NIL_P(tmp = rb_check_string_type(obj))) { obj = tmp; /* string */ } else if (!NIL_P(tmp = rb_check_array_type(obj))) { obj = tmp; /* array */ } else if (!NIL_P(tmp = rb_check_convert_type(obj, T_HASH, "Hash", "to_hash"))) { obj = tmp; /* hash */ } else { /* exception */ }
Note to_hash
. In other places, where I was keeping VALUEs in my own structs, Nobu allowed looser type checking. I imagine there’s a but of overhead in the above that can be avoided if the types have been checked already.
VALUE dest = (VALUE)emitter->bonus; if (TYPE(dest) T_STRING) { rb_str_cat( dest, str, len ); } else { rb_io_write( dest, rb_str_new( str, len ) ); }
Doing StringValue() upfront.
In the extension, I have some write
methods for the YAML emitter. Strings can get passed directly to the YAML stream. At the beginning of the method, I load the emitter object and check the string.
Data_Get_Struct(self, SyckEmitter, emitter); StringValue(str);
Wherever I had code like this, Nobu swapped the lines.
StringValue(str); Data_Get_Struct(self, SyckEmitter, emitter);
It occured to me that StringValue
could throw an exception and it is also the more likely of the two to throw an exception. I’m guessing he put it first to get the simple check out of the way and to avoid the overhead of retrieving the SyckEmitter struct if the str
ends up being something else.
Using StringValue()’s return.
Nobu also consolidated my conditionals which checked the pointer and the length for a string. The previous code:
if (NIL_P(type) ||!RSTRING(type)->ptr || RSTRING(type)->len 0)
Became:
if (NIL_P(type) ||RSTRING(StringValue(type))->len == 0)
No more overriding Klass#new.
Finally, and this is in the PickAxe II as well, Nobu moved all my singletons which were defining new
for various classes. He turned them into 1.8 allocators.
Previously, I had methods like this:
/* YAML::Syck::Emitter.new declaration */ VALUE syck_emitter_new(argc, argv, class) int argc; VALUE *argv; VALUE class; { ...; } /* Later, in Init_Syck... */ rb_define_singleton_method( cEmitter, "new", syck_emitter_new, -1 );
Nobu’s allocator involved very simple changes:
/* YAML::Syck::Emitter.allocate declaration */ VALUE syck_emitter_s_alloc(class) VALUE class; { ...; } /* Later, in Init_Syck... */ rb_define_alloc_func( cEmitter, syck_emitter_s_alloc );
Many of these changes illustrated the simplicity of the new API the Ruby-Core team has provided with 1.8. Most changes involved a simple line swap and were more powerful, to boot. And I’m also very glad for Nobu and Matz, their attention to my little module, their pruning and caretaking from time-to-time.
Daniel Berger
All very well and good, but when are you going to compile with -Wall? :)
the AC
or -O3
Daniel Berger
-O3 does bad things to Ruby.
e
Compilation should always succeed -Werror -Wall -pedantic -ansi. It’s unit testing for C :)
Anon
Isn’t this pre-Ansi C? I haven’t seen anything like this for 15+ years. Why this choice?
D
What if you want pre-1.8 compatability, do you not use the rb_check*_type functions?
Comments are closed for this entry.