Re: integer range literals in conditionals with -e option

Hi,

At Tue, 11 Sep 2007 05:19:44 +0900,
David Flanagan wrote in [ruby-core:12124]:
> There appears to be code to implement this in parse.y, but I can't make 
> it work.  (See the range_op method, which appears to insert a == 
> comparison with $. Which is surprising, since it ought to insert a === 
> comparison, I'd think.  The code never seems to be triggered, though.)

It's not a comparison with range object, but comparisons with
the each edge values.

> Anyone know what the history and current status of this is?  Is the code 
> in parse.y vestigal, or is  a regresson that this feature stopped 
> working in 1.8?

Sorry, it was an unintentional side effect of r8425, which was
to fix SEGV caused by the race condition of a fixnum range
literal.

# I'm surprised that nobody noticed it over 2 years.

I thought of these alternatives:

a) delay the transformation of NODE_DOT till conditional
   expressions take place (the following patch)

b) use Range#=== (as you thought), but this would alter the
   semantics and make some inconsistency

c) eliminate the feature.


Index: parse.y
===================================================================
--- parse.y	(revision 13426)
+++ parse.y	(working copy)
@@ -125,4 +125,6 @@ static ID cur_mid = 0;
 static int command_start = Qtrue;
 
+static NODE *deferred_nodes;
+
 static NODE *cond();
 static NODE *logop();
@@ -182,4 +184,6 @@ static void top_local_init();
 static void top_local_setup();
 
+static void fixup_nodes();
+
 #define RE_OPTION_ONCE 0x80
 
@@ -392,4 +396,5 @@ compstmt	: stmts opt_terms
 		    {
 			void_stmts($1);
+			fixup_nodes(&deferred_nodes);
 		        $$ = $1;
 		    }
@@ -1077,11 +1082,8 @@ arg		: lhs '=' arg
 			value_expr($1);
 			value_expr($3);
+			$$ = NEW_DOT2($1, $3);
 			if (nd_type($1) == NODE_LIT && FIXNUM_P($1->nd_lit) &&
 			    nd_type($3) == NODE_LIT && FIXNUM_P($3->nd_lit)) {
-			    $1->nd_lit = rb_range_new($1->nd_lit, $3->nd_lit, Qfalse);
-			    $$ = $1;
-			}
-			else {
-			    $$ = NEW_DOT2($1, $3);
+			    deferred_nodes = list_append(deferred_nodes, $$);
 			}
 		    }
@@ -1090,11 +1092,8 @@ arg		: lhs '=' arg
 			value_expr($1);
 			value_expr($3);
+			$$ = NEW_DOT3($1, $3);
 			if (nd_type($1) == NODE_LIT && FIXNUM_P($1->nd_lit) &&
 			    nd_type($3) == NODE_LIT && FIXNUM_P($3->nd_lit)) {
-			    $1->nd_lit = rb_range_new($1->nd_lit, $3->nd_lit, Qtrue);
-			    $$ = $1;
-			}
-			else {
-			    $$ = NEW_DOT3($1, $3);
+			    deferred_nodes = list_append(deferred_nodes, $$);
 			}
 		    }
@@ -2604,4 +2603,5 @@ yycompile(f, line)
     ruby_current_node = 0;
     ruby_sourcefile = rb_source_filename(f);
+    deferred_nodes = 0;
     n = yyparse();
     ruby_debug_lines = 0;
@@ -2615,4 +2615,5 @@ yycompile(f, line)
     in_def = 0;
     cur_mid = 0;
+    deferred_nodes = 0;
 
     vp = ruby_dyna_vars;
@@ -5288,4 +5289,34 @@ warning_unless_e_option(node, str)
 }
 
+static void
+fixup_nodes(rootnode)
+    NODE **rootnode;
+{
+    NODE *node, *next, *head;
+
+    for (node = *rootnode; node; node = next) {
+	enum node_type type;
+	VALUE val;
+
+	next = node->nd_next;
+	head = node->nd_head;
+	rb_gc_force_recycle((VALUE)node);
+	*rootnode = next;
+	switch (type = nd_type(head)) {
+	  case NODE_DOT2:
+	  case NODE_DOT3:
+	    val = rb_range_new(head->nd_beg->nd_lit, head->nd_end->nd_lit,
+			       type == NODE_DOT3 ? Qtrue : Qfalse);
+	    rb_gc_force_recycle((VALUE)head->nd_beg);
+	    rb_gc_force_recycle((VALUE)head->nd_end);
+	    nd_set_type(head, NODE_LIT);
+	    head->nd_lit = val;
+	    break;
+	  default:
+	    break;
+	}
+    }
+}
+
 static NODE *cond0();
 
@@ -5296,9 +5327,6 @@ range_op(node)
     enum node_type type;
 
-    if (!e_option_supplied()) return node;
     if (node == 0) return 0;
 
-    value_expr(node);
-    node = cond0(node);
     type = nd_type(node);
     if (type == NODE_NEWLINE) {
@@ -5306,9 +5334,10 @@ range_op(node)
 	type = nd_type(node);
     }
+    value_expr(node);
     if (type == NODE_LIT && FIXNUM_P(node->nd_lit)) {
 	warn_unless_e_option(node, "integer literal in conditional range");
 	return call_op(node,tEQ,1,NEW_GVAR(rb_intern("$.")));
     }
-    return node;
+    return cond0(node);
 }
 
@@ -5826,4 +5855,5 @@ rb_gc_mark_parser()
     rb_gc_mark(lex_input);
     rb_gc_mark((VALUE)lex_strterm);
+    rb_gc_mark((VALUE)deferred_nodes);
 }
 


-- 
Nobu Nakada