a.md

少し大きめのリファクタリングを入れたので何をしたかを書いておく。

• Refactor HirMakerContext by yhara · Pull Request #257 · yhara/shiika

元のコード

リファクタリングしたのはHirMakerContextという構造体である。こいつはASTからHIR(高レベル中間表現)を作る際に使うもので、もとはこういう定義だった。

#[derive(Debug)]
pub struct HirMakerContext {
    /// Type of this ctx
    pub kind: CtxKind,
    /// Where this ctx is in the ctx_stack
    pub depth: usize,
    /// Signature of the current method (Used to get the list of parameters)
    /// None if out of a method
    pub method_sig: Option<MethodSignature>,
    /// The type of current `self`
    pub self_ty: TermTy,
    /// Names of the type parameter of the current class (or method, in the future)
    pub typarams: Vec<String>,
    /// Current namespace
    /// `""` for toplevel
    pub namespace: ClassFullname,
    /// Current local variables
    pub lvars: HashMap<String, CtxLVar>,
    /// List of free variables captured in this context
    pub captures: Vec<LambdaCapture>,
    //
    // ivar-related stuffs
    //
    /// List of instance variables in an initializer found so far
    pub iivars: SkIVars,
    /// Number of inherited ivars. Only used when kind is Initializer
    pub super_ivars: SkIVars, // TODO: this can be just &'a SkIVars
}

何が問題かというと、項目が不必要に多い。kindはToplevel/Class/Method/Initializer/Lambdaの5種類を取るのだが、それらで使う項目が全部ここに入ってしまっている。たとえばcapturesはLambdaでしか使わないし、iivarsはInitializerでしか使わない。

リファクタ案1

これを整理してみたのが以下。CtxDetailというenumを定義して、ある場面でしか使わない項目はそこに移動した。どうだろうか？

#[derive(Debug)]
pub struct HirMakerContext {
    /// Type of this ctx
    pub kind: CtxKind,
    (中略)
    /// Additional information
    pub detail: CtxDetail,
}

#[derive(Debug)]
pub enum CtxDetail {
    Toplevel,
    Class,
    Method {
        /// Signature of the current method
        signature: MethodSignature,
    },
    Initializer {
        /// List of instance variables in an initializer found so far
        iivars: SkIVars,
        /// List of inherited ivars
        super_ivars: SkIVars, // TODO: this can be just &'a SkIVars
        /// Signature of the current method
        signature: MethodSignature,
    },
    Lambda {
        /// List of free variables captured in this context
        captures: Vec<LambdaCapture>,
        /// Signature of the current method
        signature: MethodSignature,
    },
}

リファクタ案1の問題点

一見すっきりして良いように見えるが、実際にコードを書いてみるとどうも不便なことに気づいた。元のコードだとctxのcapturesを参照するには

ctx.captures

と書くだけで良かったが、修正後はいちいち

        if let CtxBody::Lambda { captures, .. } = &self.body {
            &captures
        } else {
            panic!("this ctx is not Lambda")
        }

のように条件分岐を挟まなければいけない。このようにヘルパーメソッドを書いてみたものの、structの要素ごとにこれをやっていたのでは大量のヘルパーメソッドができてしまいそうだ。

せめてctxの種類だけヘルパーメソッドを作ることができればまだましなのだけど、Rustではenumのvariant(ここでいうCtxBody::Lambdaなど)は型ではないので、「CtxBody::Lambdaを返すメソッド」を定義することはできない。

リファクタ案2

いろいろ考えているうちに、「一部だけ違うstructを一つの配列に突っ込んでいるのがだめなのかもしれない」という考えに至った。

#[derive(Debug)]
pub struct HirMakerContext_ {
    pub toplevel: ToplevelCtx,
    pub classes: Vec<ClassCtx>,
    pub method: Option<MethodCtx>,
    pub lambdas: Vec<LambdaCtx>,
}

#[derive(Debug)]
pub struct ToplevelCtx {}

#[derive(Debug)]
pub struct ClassCtx {}

#[derive(Debug)]
pub struct MethodCtx {}

#[derive(Debug)]
pub struct LambdaCtx {
    /// List of free variables captured in this context
    pub captures: Vec<LambdaCapture>,
}

別解1

そうじゃなくて、タプルを返せばよかったのではないか。

impl HirMakerContext {
  pub fn method_ctx(&self) -> (signature, 略) {
    match self.detail {
      CtxDetail::Method { signature, 略 } => (signature, 略),
      _ => panic!()
    }
  }
}

別解2

あー、タプルがいけるならつまり、それぞれのvariantをstructにすればいいのか？struct -> enum -> structという構成。

#[derive(Debug)]
pub struct HirMakerContext {
    /// Type of this ctx
    pub kind: CtxKind,
    (中略)
    /// Additional information
    pub detail: CtxDetail,
}

#[derive(Debug)]
pub enum CtxDetail {
    Toplevel,
    Class,
    Method(MethodCtx),
    略
}

pub struct MethodCtx {
  /// Signature of the current method
  signature: MethodSignature,
}

これでちょっと試してみるか。