hoodwink.d enhanced
RSS
2.0
XHTML
1.0

RedHanded

Nobu Grades My Homework #

by why in inspect

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.

said on 04 Apr 2005 at 14:15

All very well and good, but when are you going to compile with -Wall? :)

said on 04 Apr 2005 at 19:03

or -O3

said on 04 Apr 2005 at 19:27

-O3 does bad things to Ruby.

said on 04 Apr 2005 at 20:00

Compilation should always succeed -Werror -Wall -pedantic -ansi. It’s unit testing for C :)

said on 04 Apr 2005 at 22:54

Isn’t this pre-Ansi C? I haven’t seen anything like this for 15+ years. Why this choice?

said on 09 Apr 2005 at 12:16

What if you want pre-1.8 compatability, do you not use the rb_check*_type functions?

Comments are closed for this entry.