From c795d1bcbcc08c18411e7c52151c3d942ee1b1a0 Mon Sep 17 00:00:00 2001
From: Brian Ford <bford@engineyard.com>
Date: Tue, 4 Aug 2009 11:43:43 -0700
Subject: [PATCH] Resurrect ConcatArgs. All array compiler specs pass.

Like a nauseatingly sweet cake, the compiler currently has
3 thick layers of obfuscating logic: 1) the sexp rewriting,
2) the sexp consumption and args methods, 3) the bytecode
generation. The compiler is a bad marriage between a sexp
processor and an object-oriented compiler. All this is built
on the often confusing foundation of MRI's parse tree.

I mean, seriously, rewrite argscat into splats that are wrapped
in arrays so other nodes can take them apart again? Seriously?

So I am the divorce judge and this union is being destroyed.
Sorry, sexps, you get nothing. Dismissed. Goodbye. Objects,
you get stripped of your twisty internal logic, to the extent
reasonable. How? By creating more classes as appropriate that
each do their own simple thing.

Again, this is a WIP, but the general principle is that if a
thing needs a particular form, it should be a particular thing,
not some murky shape-shifter.
---
 lib/ext/melbourne/visitor.cpp |  6 ++--
 lib/melbourne/nodes.rb        | 64 ++++++++++++++++++++++-------------
 lib/melbourne/processor.rb    | 12 +++----
 3 files changed, 48 insertions(+), 34 deletions(-)

diff --git a/lib/ext/melbourne/visitor.cpp b/lib/ext/melbourne/visitor.cpp
index dfe27bbbcb..5544a9ac01 100644
--- a/lib/ext/melbourne/visitor.cpp
+++ b/lib/ext/melbourne/visitor.cpp
@@ -443,9 +443,9 @@ namespace melbourne {
       break;
     }
     case NODE_ARGSCAT: {
-      VALUE head = process_parse_tree(parse_state, ptp, node->nd_head, locals);
-      VALUE body = process_parse_tree(parse_state, ptp, node->nd_body, locals);
-      tree = rb_funcall(ptp, rb_sArgsCat, 3, line, head, body);
+      VALUE array = process_parse_tree(parse_state, ptp, node->nd_head, locals);
+      VALUE rest = process_parse_tree(parse_state, ptp, node->nd_body, locals);
+      tree = rb_funcall(ptp, rb_sArgsCat, 3, line, array, rest);
       break;
     }
     case NODE_ARGSPUSH: {
diff --git a/lib/melbourne/nodes.rb b/lib/melbourne/nodes.rb
index 71fb0bf7a6..73c5e23277 100644
--- a/lib/melbourne/nodes.rb
+++ b/lib/melbourne/nodes.rb
@@ -134,9 +134,17 @@ def self.from(p, args, defaults, splat)
     class ArrayLiteral < Node
       def self.from(p, array)
         node = ArrayLiteral.new p.compiler
-        node.args array
+        node.body = array
         node
       end
+
+      def bytecode(g)
+        @body.each do |x|
+          x.bytecode(g)
+        end
+
+        g.make_array @body.size
+      end
     end
 
     class AttrAssign < Call
@@ -276,6 +284,22 @@ def nest_scope(scope)
       end
     end
 
+    class ConcatArgs < DynamicArguments
+      def self.from(p, array, rest)
+        node = ConcatArgs.new p.compiler
+        node.array = array
+        node.rest = rest
+        node
+      end
+
+      def bytecode(g)
+        @array.bytecode(g)
+        @rest.bytecode(g)
+        g.cast_array
+        g.send :+, 1
+      end
+    end
+
     class ConstAccess < Node
       def self.from(p, outer, name)
         node = ConstAccess.new p.compiler
@@ -408,6 +432,12 @@ def self.from(p, str, array)
       end
     end
 
+    class EmptyArray < Node
+      def self.from(p)
+        EmptyArray.new p.compiler
+      end
+    end
+
     class Ensure < Node
       def self.from(p, head, ensr)
         node = Ensure.new p.compiler
@@ -593,27 +623,6 @@ def bytecode(g)
       end
     end
 
-    class SLocalAssignment < LocalAssignment
-      def self.from(p, name, expr)
-        node = SLocalAssignment.new p.compiler
-        node.name = name
-        node.value = expr
-        node
-      end
-
-      def bytecode(g)
-        pos(g)
-
-        if @value
-          @value.bytecode(g)
-          g.cast_array
-          g.send :+, 1
-        end
-
-        @variable.set_bytecode(g)
-      end
-    end
-
     class LocalVariable < Node
       def find_local(allocate=true)
         iter = find_parent Iter, ClosedScope
@@ -874,11 +883,20 @@ def bytecode(g)
     end
 
     class Splat < DynamicArguments
+      attr_accessor :value
+
       def self.from(p, expr)
         node = Splat.new p.compiler
-        node.args
+        node.value = expr
         node
       end
+
+      def bytecode(g)
+        g.make_array 0
+        @value.bytecode(g)
+        g.cast_array
+        g.send :+, 1
+      end
     end
 
     class StringLiteral < Node
diff --git a/lib/melbourne/processor.rb b/lib/melbourne/processor.rb
index c72cdf46e7..a1b06ed598 100644
--- a/lib/melbourne/processor.rb
+++ b/lib/melbourne/processor.rb
@@ -51,8 +51,8 @@ def process_args(line, args, defaults, splat)
       AST::Arguments.from self, args, defaults, splat
     end
 
-    def process_argscat(line, normal, splat)
-      AST::ArgList.from self, normal, splat
+    def process_argscat(line, array, rest)
+      AST::ConcatArgs.from self, array, rest
     end
 
     def process_argspush(line, splat, value)
@@ -242,11 +242,7 @@ def process_ivar(line, name)
     end
 
     def process_lasgn(line, name, expr)
-      if expr.kind_of? AST::SValue
-        AST::SLocalAssignment.from self, name, expr
-      else
-        AST::LocalAssignment.from self, name, expr
-      end
+      AST::LocalAssignment.from self, name, expr
     end
 
     def process_lit(line, sym)
@@ -415,7 +411,7 @@ def process_yield(line, expr, flags)
     end
 
     def process_zarray(line)
-      AST::ArrayLiteral.from self, []
+      AST::EmptyArray.from self
     end
 
     def process_zsuper(line)