vm2gol v2 製作メモ(28) リファクタリング / ダンプ表示の改良


リファクタリングします!

参照の解決処理をメソッド抽出

前回雑にコピペで追加しまくった

fn_arg_pos = fn_arg_names.index(args[1]) + 2
"[bp+#{fn_arg_pos}]"

この部分を抽出してメソッド化しようと思います。 2行だけとはいえ、さすがに何度も同じことをやっていてどうも煩雑なので。

--- a/vgcg.rb
+++ b/vgcg.rb
@@ -8,6 +8,11 @@ require './common'
 
 $label_id = 0
 
+def to_fn_arg_addr(fn_arg_names, fn_arg_name)
+  index = fn_arg_names.index(fn_arg_name)
+  "[bp+#{index + 2}]"
+end
+
 def codegen_case(when_blocks)
   alines = []
   $label_id += 1
@@ -103,8 +108,7 @@ def codegen_exp(fn_arg_names, lvar_names, exp)
         lvar_pos = lvar_names.index(args[0]) + 1
         "[bp-#{lvar_pos}]"
       when fn_arg_names.include?(args[0])
-        fn_arg_pos = fn_arg_names.index(args[0]) + 2
-        "[bp+#{fn_arg_pos}]"
+        to_fn_arg_addr(fn_arg_names, args[0])
       else
         raise not_yet_impl("left", args[0])
       end
@@ -119,8 +123,7 @@ def codegen_exp(fn_arg_names, lvar_names, exp)
     when String
       case
       when fn_arg_names.include?(args[1])
-        fn_arg_pos = fn_arg_names.index(args[1]) + 2
-        "[bp+#{fn_arg_pos}]"
+        to_fn_arg_addr(fn_arg_names, args[1])
       else
         raise not_yet_impl("right", args[1])
       end
@@ -175,8 +178,7 @@ def codegen_set(fn_arg_names, lvar_names, rest)
       alines += codegen_exp(fn_arg_names, lvar_names, exp)
       "reg_a"
     when fn_arg_names.include?(rest[1])
-      fn_arg_pos = fn_arg_names.index(rest[1]) + 2
-      "[bp+#{fn_arg_pos}]"
+      to_fn_arg_addr(fn_arg_names, rest[1])
     when /^vram\[(.+)\]$/ =~ rest[1]
       vram_addr = $1
       alines << "  get_vram #{vram_addr} reg_a"

適当に to_fn_arg_addr というメソッド名にしたものの、 このメソッドで行っていることは参照の解決なので resolve_〜 という名前の方が良かったかなあ? とか考えたりしつつ……。

まあこれで進めましょう。


ローカル変数の解決の方も同様に共通化します。

--- a/vgcg.rb
+++ b/vgcg.rb
@@ -13,6 +13,11 @@ def to_fn_arg_addr(fn_arg_names, fn_arg_name)
   "[bp+#{index + 2}]"
 end
 
+def to_lvar_addr(lvar_names, lvar_name)
+  index = lvar_names.index(lvar_name)
+  "[bp-#{index + 1}]"
+end
+
 def codegen_case(when_blocks)
   alines = []
   $label_id += 1
@@ -105,8 +110,7 @@ def codegen_exp(fn_arg_names, lvar_names, exp)
     when String
       case
       when lvar_names.include?(args[0])
-        lvar_pos = lvar_names.index(args[0]) + 1
-        "[bp-#{lvar_pos}]"
+        to_lvar_addr(lvar_names, args[0])
       when fn_arg_names.include?(args[0])
         to_fn_arg_addr(fn_arg_names, args[0])
       else
@@ -194,14 +198,14 @@ def codegen_set(fn_arg_names, lvar_names, rest)
     when /^\d+$/ =~ vram_addr
       alines << "  set_vram #{vram_addr} #{src_val}"
     when lvar_names.include?(vram_addr)
-      lvar_pos = lvar_names.index(vram_addr) + 1
-      alines << "  set_vram [bp-#{lvar_pos}] #{src_val}"
+      lvar_addr = to_lvar_addr(lvar_names, vram_addr)
+      alines << "  set_vram #{lvar_addr} #{src_val}"
     else
       raise not_yet_impl("vram_addr", vram_addr)
     end
   else
-    lvar_pos = lvar_names.index(dest) + 1
-    alines << "  cp #{src_val} [bp-#{lvar_pos}]"
+    lvar_addr = to_lvar_addr(lvar_names, dest)
+    alines << "  cp #{src_val} #{lvar_addr}"
   end
 
   alines
@@ -236,8 +240,8 @@ def codegen_func_def(rest)
         when String
           case
           when lvar_names.include?(fn_arg)
-            lvar_pos = lvar_names.index(fn_arg) + 1
-            alines << "  push [bp-#{lvar_pos}]"
+            lvar_addr = to_lvar_addr(lvar_names, fn_arg)
+            alines << "  push #{lvar_addr}"
           else
             raise not_yet_impl(fn_arg)
           end
@@ -256,8 +260,8 @@ def codegen_func_def(rest)
       alines << "  call #{fn_name}"
       alines << "  add_sp #{fn_args.size}"
 
-      lvar_pos = lvar_names.index(lvar_name) + 1
-      alines << "  cp reg_a [bp-#{lvar_pos}]"
+      lvar_addr = to_lvar_addr(lvar_names, lvar_name)
+      alines << "  cp reg_a #{lvar_addr}"
     when "var"
       lvar_names << stmt_rest[0]
       alines << "  sub_sp 1"

コード量の変化はあまりないものの、 DRY になり、ちょっとだけすっきりした……気がします。


あと目に付くのは、前回も修正しまくったこういうやつですね。

    case args[0]
    when Integer
      args[0]
    when String
      case
      when lvar_names.include?(args[0])
        to_lvar_addr(lvar_names, args[0])
      when fn_arg_names.include?(args[0])
        to_fn_arg_addr(fn_arg_names, args[0])
      else
        raise not_yet_impl("left", args[0])
      end
    else
      raise not_yet_impl("left", args[0])
    end

これも同じようなパターンが何度も出てくるので、 共通化したくなるのが人情ではありますが……

  • 似ているように見えて微妙に違っているので意外と手間がかかるかも?
    • 通化できるか調べる手間
      • 「早すぎる共通化」ではないか?
      • 抽象化に漏れはないか?
    • 抽出できる形に整える手間
  • テストコードで守られていない

といったあたりを考慮して、ここはぐっとこらえて、やめておきました。 下手なことしてバグを入れて時間をロスするのもつまらないですしね。

ひとまず寝かせておいて、 どうしてもやりたかったら完成した後でやってもいいでしょう。


似たようなところで言えば codegen_func_def()codegen_stmts() の内容が被っているのも気にはなっているんですが、 同様の理由で放置しています ( v1 のときは適当なところで共通化していたのですが、 v2 ではなんかタイミングを逃してしまいました…… )。

ダンプ表示の改良

この段階でもう一つ改良しておきます。

ライフゲームの実装に入り、 メインメモリのダンプ表示の行数が増え、 ダンプ表示全体が 1画面に収まらなくなってきました。

これは困るので、対処しましょう。 これからどんどん長くなっていくので、 そろそろ対策しておかないとこの後めんどくさいのです。

pc を基準にして、その前後の部分だけを表示するようにします。

--- a/vgvm.rb
+++ b/vgvm.rb
@@ -12,6 +12,8 @@ end
 class Memory
   attr_accessor :main, :stack, :vram
 
+  MAIN_DUMP_WIDTH = 30
+
   def initialize(stack_size)
     @main = []
 
@@ -34,7 +36,12 @@ class Memory
       addr += 1 + num_args
     end
 
-    vmcmds.map{ |vmcmd|
+    vmcmds
+    .select {|vmcmd|
+      pc - MAIN_DUMP_WIDTH <= vmcmd[:addr] &&
+      vmcmd[:addr] <= pc + MAIN_DUMP_WIDTH
+    }
+    .map {|vmcmd|
       head =
         if vmcmd[:addr] == pc
           "pc =>"

幅は適当に決めました。


それから、若干先回り気味ですが、スタック領域の方もついでに修正しておきます。

これまでは関数呼び出しのネストが浅かったので特に問題ありませんでしたが、 これからネストが深くなってくるとアドレスが大きい方 (sp 〜 スタックの底までの範囲)が伸びてきて、 これまた 1画面に収まらなくなるためです。

--- a/vgvm.rb
+++ b/vgvm.rb
@@ -80,6 +80,7 @@ class Memory
     @stack.each_with_index do |x, i|
       addr = i
       next if addr < sp - 8
+      next if addr > sp + 8
       head =
         case addr
         when sp

前回のブリンカーの初期化だけのプログラムを動かして確認します。

================================
reg_a(7) reg_b(2) reg_c(0) zf(0)
---- memory (main) ----
      07   ["cp", "sp", "bp"]
      10   ["sub_sp", 1]
      12   ["set_reg_a", "[bp+4]"]
      14   ["set_reg_b", "[bp+2]"]
      16   ["mult_ab"]
      17   ["cp", "reg_a", "[bp-1]"]
      20   ["sub_sp", 1]
      22   ["set_reg_a", "[bp-1]"]
      24   ["set_reg_b", "[bp+3]"]
      26   ["add_ab"]
      27   ["cp", "reg_a", "[bp-2]"]
      30   ["set_vram", "[bp-2]", "[bp+5]"]
      33   ["cp", "bp", "sp"]
pc => 36   ["pop", "bp"]
      38   ["ret"]
      39 ["label", "main"]
      41   ["push", "bp"]
      43   ["cp", "sp", "bp"]
      46   ["sub_sp", 1]
      48   ["cp", 5, "[bp-1]"]
      51   ["push", 1]
      53   ["push", 1]
      55   ["push", 2]
      57   ["push", "[bp-1]"]
      59   ["call", 5]
      61   ["add_sp", 4]
      63   ["push", 1]
      65   ["push", 2]
---- memory (stack) ----
         32 0
         33 0
         34 0
         35 0
         36 0
         37 0
         38 7
         39 5
sp bp => 40 47
         41 61
         42 5
         43 2
         44 1
         45 1
         46 5
         47 49
         48 2
---- memory (vram) ----
..... .....
..@.. .....
..... .....
..... .....
..... .....

1画面に収まるようになりました!

call, call_set のメソッド抽出

あと1箇所、ちょっとしたメソッド抽出もついでにやっておきます。

codegen_func_def() が長くなってきたので、 callcall_set の部分をメソッドに抽出しておきます。

--- a/vgcg.rb
+++ b/vgcg.rb
@@ -169,6 +169,32 @@ def codegen_exp(fn_arg_names, lvar_names, exp)
   alines
 end
 
+def codegen_call(lvar_names, stmt_rest)
+  alines = []
+
+  fn_name, *fn_args = stmt_rest
+  fn_args.reverse.each {|fn_arg|
+    case fn_arg
+    when Integer
+      alines << "  push #{fn_arg}"
+    when String
+      case
+      when lvar_names.include?(fn_arg)
+        lvar_addr = to_lvar_addr(lvar_names, fn_arg)
+        alines << "  push #{lvar_addr}"
+      else
+        raise not_yet_impl(fn_arg)
+      end
+    else
+      raise not_yet_impl(fn_arg)
+    end
+  }
+  alines << "  call #{fn_name}"
+  alines << "  add_sp #{fn_args.size}"
+
+  alines
+end
+
 def codegen_set(fn_arg_names, lvar_names, rest)
   alines = []
   dest = rest[0]
@@ -232,25 +258,7 @@ def codegen_func_def(rest)
     stmt_head, *stmt_rest = stmt
     case stmt_head
     when "call"
-      fn_name, *fn_args = stmt_rest
-      fn_args.reverse.each {|fn_arg|
-        case fn_arg
-        when Integer
-          alines << "  push #{fn_arg}"
-        when String
-          case
-          when lvar_names.include?(fn_arg)
-            lvar_addr = to_lvar_addr(lvar_names, fn_arg)
-            alines << "  push #{lvar_addr}"
-          else
-            raise not_yet_impl(fn_arg)
-          end
-        else
-          raise not_yet_impl(fn_arg)
-        end
-      }
-      alines << "  call #{fn_name}"
-      alines << "  add_sp #{fn_args.size}"
+      alines += codegen_call(lvar_names, stmt_rest)
     when "call_set"
       lvar_name, fn_temp = stmt_rest
       fn_name, *fn_args = fn_temp
--- a/vgcg.rb
+++ b/vgcg.rb
@@ -195,6 +195,23 @@ def codegen_call(lvar_names, stmt_rest)
   alines
 end
 
+def codegen_call_set(lvar_names, stmt_rest)
+  alines = []
+
+  lvar_name, fn_temp = stmt_rest
+  fn_name, *fn_args = fn_temp
+  fn_args.reverse.each {|fn_arg|
+    alines << "  push #{fn_arg}"
+  }
+  alines << "  call #{fn_name}"
+  alines << "  add_sp #{fn_args.size}"
+
+  lvar_addr = to_lvar_addr(lvar_names, lvar_name)
+  alines << "  cp reg_a #{lvar_addr}"
+
+  alines
+end
+
 def codegen_set(fn_arg_names, lvar_names, rest)
   alines = []
   dest = rest[0]
@@ -260,16 +277,7 @@ def codegen_func_def(rest)
     when "call"
       alines += codegen_call(lvar_names, stmt_rest)
     when "call_set"
-      lvar_name, fn_temp = stmt_rest
-      fn_name, *fn_args = fn_temp
-      fn_args.reverse.each {|fn_arg|
-        alines << "  push #{fn_arg}"
-      }
-      alines << "  call #{fn_name}"
-      alines << "  add_sp #{fn_args.size}"
-
-      lvar_addr = to_lvar_addr(lvar_names, lvar_name)
-      alines << "  cp reg_a #{lvar_addr}"
+      alines += codegen_call_set(lvar_names, stmt_rest)
     when "var"
       lvar_names << stmt_rest[0]
       alines << "  sub_sp 1"

ではライフゲームに戻りましょう!