From: William Woodruff <william@...>
Date: 2015-08-09T23:51:10-04:00
Subject: [ruby-core:70295] Behavior of sprintf with dynamic hashes,	possible improvement?

This is a multi-part message in MIME format.
--------------010600060004080500040204
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

Hello all,

I came across this behavior while trying to use a dynamic hash to
populate a string via `sprintf`. While normal formatting works:

     "%{a}" % { a: "value" } # => "value"

Dynamic hashes do not:

     "%{a}" % Hash.new { |hash, key| hash[key] = "value" } # => KeyError

Ox0dea on #ruby helped me trace this down to trunk/sprintf.c#611:

     if (sym != Qnil) nextvalue = rb_hash_lookup2(hash, sym, Qundef);

and trunk/hash.c#795:

    return def; /* without Hash#default */

It seems like using `rb_hash_aref` instead of `rb_hash_lookup2` in
sprintf.c would solve this problem. To prevent the string from being
populated with empty strings when `rb_hash_aref` returns nil as the
default value, this could be guarded with a check like this:

     if (sym != Qnil) nextvalue = rb_hash_aref(hash, sym);
     if (nextvalue == Qnil) {
         nextvalue = rb_hash_lookup2(hash, sym, Qundef);
         if (nextvalue == Qundef) {
             rb_enc_raise(enc, rb_eKeyError, "key%.*s not found", len, 
start);
         }
     }

I've tried this (patch attached) on a local build, but I'm running into
an ArgumentError that I'm having trouble tracing down:

     "%{a}" % Hash.new { |hash, key| hash[key] = "value\" }"

produces "unnumbered(1) mixed with named (ArgumentError)"

I'm still in the process of tracing that down.

Does anybody have any thoughts on this? Is formatting with dynamic
hashes something that could be included/supported in the future, or
am I pushing the principle too far in this case?

--
William Woodruff
william@tuffbizz.com


--------------010600060004080500040204
Content-Type: text/x-patch;
 name="sprintf.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="sprintf.diff"

diff --git a/sprintf.c b/sprintf.c
index 4549791..3aeb00e 100644
--- a/sprintf.c
+++ b/sprintf.c
@@ -608,9 +608,12 @@ rb_str_format(int argc, const VALUE *argv, VALUE fmt)
 		sym = rb_check_symbol_cstr(start + 1,
 					   len - 2 /* without parenthesis */,
 					   enc);
-		if (sym != Qnil) nextvalue = rb_hash_lookup2(hash, sym, Qundef);
-		if (nextvalue == Qundef) {
-		    rb_enc_raise(enc, rb_eKeyError, "key%.*s not found", len, start);
+		if (sym != Qnil) nextvalue = rb_hash_aref(hash, sym);
+		if (nextvalue == Qnil) {
+			nextvalue = rb_hash_lookup2(hash, sym, Qundef);
+			if (nextvalue == Qundef) {
+				rb_enc_raise(enc, rb_eKeyError, "key%.*s not found", len, start);
+			}
 		}
 		if (term == '}') goto format_s;
 		p++;
--------------010600060004080500040204--