diff --git a/docs/development/architecture/phase-26-valueid-type-safety.md b/docs/development/architecture/phase-26-valueid-type-safety.md new file mode 100644 index 00000000..c236d3ab --- /dev/null +++ b/docs/development/architecture/phase-26-valueid-type-safety.md @@ -0,0 +1,371 @@ +# Phase 26-A: ValueId型安全化設計 + +## 🎯 目的 + +**GUARD checkバグのような「ValueIdの意味的曖昧性」から生じるバグを根絶する** + +### 問題の本質 + +```rust +// ❌ 現状: ValueId(0) が何を意味するか不明 +let value = ValueId(0); +// Parameter? Local? Constant? Temporary? +// → 実行時エラーまで気づかない! +``` + +**GUARD checkバグの例**: +```rust +// ValueId(0) を「常に未初期化」と誤判定 +for (name, value) in ¤t_vars { + if value.0 == 0 { // ← Parameter s=ValueId(0) も弾いてしまう! + return Ok(ValueId(0)); + } +} +``` + +--- + +## 📐 設計 + +### 1. MirValueKind列挙型 + +```rust +/// ValueIdの意味的分類(型安全性強化) +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum MirValueKind { + /// 関数パラメータ + /// - パラメータインデックス(0-based)を保持 + /// - 例: fn skip_whitespace(s, idx) → s=Parameter(0), idx=Parameter(1) + Parameter(u32), + + /// ローカル変数 + /// - スコープ内のローカル番号(関数ごとに独立) + /// - 例: local i = 0 → Local(0) + Local(u32), + + /// 定数値 + /// - コンパイル時に値が確定 + /// - 例: 42, "hello", true + Constant, + + /// 一時値 + /// - 式評価・演算の中間結果 + /// - 例: a + b の結果、copy, phi の結果 + Temporary, + + /// Pinned変数 + /// - ブロック跨ぎの一時変数(SSA構築用) + /// - 例: __pin$N$@binop_lhs + Pinned, + + /// LoopCarrier + /// - ループ内で再定義される変数 + /// - PHI nodeで複数の値をマージ + /// - 例: loop内で更新される i + LoopCarrier, +} +``` + +### 2. TypedValueId構造体 + +```rust +/// 型付きValueId - ValueIdに意味情報を付与 +#[derive(Debug, Clone, Copy)] +pub struct TypedValueId { + /// 実際のValueId(既存システムとの互換性) + pub id: ValueId, + + /// 値の種類 + pub kind: MirValueKind, +} + +impl TypedValueId { + /// 新規作成 + pub fn new(id: ValueId, kind: MirValueKind) -> Self { + Self { id, kind } + } + + /// パラメータか判定(型安全) + pub fn is_parameter(&self) -> bool { + matches!(self.kind, MirValueKind::Parameter(_)) + } + + /// ローカル変数か判定 + pub fn is_local(&self) -> bool { + matches!(self.kind, MirValueKind::Local(_)) + } + + /// 定数か判定 + pub fn is_constant(&self) -> bool { + matches!(self.kind, MirValueKind::Constant) + } + + /// LoopCarrierか判定 + pub fn is_loop_carrier(&self) -> bool { + matches!(self.kind, MirValueKind::LoopCarrier) + } + + /// ValueIdを取得(後方互換性) + pub fn value_id(&self) -> ValueId { + self.id + } +} + +impl From for ValueId { + fn from(typed: TypedValueId) -> ValueId { + typed.id + } +} +``` + +### 3. MirBuilder統合 + +```rust +/// MirBuilderに追加するフィールド +pub struct MirBuilder { + // ... 既存フィールド ... + + /// ValueIdの型情報マップ + /// Phase 26-A: ValueId型安全化 + value_kinds: HashMap, +} + +impl MirBuilder { + /// 型付きValueId発行(新API) + pub fn new_typed_value(&mut self, kind: MirValueKind) -> TypedValueId { + let id = self.next_value_id(); + self.value_kinds.insert(id, kind); + TypedValueId::new(id, kind) + } + + /// 既存ValueIdの型取得 + pub fn get_value_kind(&self, id: ValueId) -> Option { + self.value_kinds.get(&id).copied() + } + + /// 型情報を後付け(レガシー互換用) + pub fn register_value_kind(&mut self, id: ValueId, kind: MirValueKind) { + self.value_kinds.insert(id, kind); + } + + /// 既存next_value_id()との互換性 + pub fn next_value_id(&mut self) -> ValueId { + // 既存実装維持 + // デフォルトでTemporary扱い + let id = ValueId(self.value_counter); + self.value_counter += 1; + self.value_kinds.insert(id, MirValueKind::Temporary); + id + } +} +``` + +--- + +## 🔧 実装戦略 + +### Phase 26-A-1: 基礎型実装(1日) + +**ファイル**: `src/mir/value_kind.rs` (新規) + +- [ ] `MirValueKind` 列挙型実装 +- [ ] `TypedValueId` 構造体実装 +- [ ] ユニットテスト作成 + +### Phase 26-A-2: MirBuilder統合(1日) + +**ファイル**: `src/mir/builder.rs` + +- [ ] `value_kinds` フィールド追加 +- [ ] `new_typed_value()` 実装 +- [ ] `get_value_kind()` 実装 +- [ ] `register_value_kind()` 実装 +- [ ] 既存`next_value_id()`の互換性維持 + +### Phase 26-A-3: パラメータ登録(0.5日) + +**ファイル**: `src/mir/builder.rs` + +関数パラメータ登録時に型情報を付与: + +```rust +// setup_function_params() 修正箇所 +for (idx, param_name) in params.iter().enumerate() { + let param_id = ValueId(offset + idx); + + // Phase 26-A: パラメータ型登録 + self.register_value_kind(param_id, MirValueKind::Parameter(idx as u32)); + + self.variable_map.insert(param_name.clone(), param_id); +} +``` + +### Phase 26-A-4: loop_builder.rs修正(0.5日) + +**ファイル**: `src/mir/loop_builder.rs` + +`is_parameter()` の根本修正: + +```rust +// ❌ 旧実装(名前ベース、脆弱) +fn is_parameter(&self, name: &str) -> bool { + if name.starts_with("__pin$") { return false; } + if name == "me" { return true; } + + let is_param = self.parent_builder.function_param_names.contains(name); + // ... +} + +// ✅ 新実装(型ベース、型安全) +fn is_parameter(&self, value_id: ValueId) -> bool { + self.parent_builder + .get_value_kind(value_id) + .map(|kind| matches!(kind, MirValueKind::Parameter(_))) + .unwrap_or(false) +} +``` + +**呼び出し箇所の修正**: + +```rust +// Phase 26-A: 名前ベース → ValueIdベースに変更 +for (var_name, value_id) in ¤t_vars { + if self.is_parameter(*value_id) { // ← 修正 + // パラメータ処理 + } +} +``` + +### Phase 26-A-5: テスト作成(1日) + +**ファイル**: `src/mir/value_kind.rs`, `src/tests/mir_value_kind.rs` + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parameter_detection() { + let typed = TypedValueId::new(ValueId(0), MirValueKind::Parameter(0)); + assert!(typed.is_parameter()); + assert!(!typed.is_local()); + } + + #[test] + fn test_local_variable() { + let typed = TypedValueId::new(ValueId(10), MirValueKind::Local(0)); + assert!(typed.is_local()); + assert!(!typed.is_parameter()); + } + + #[test] + fn test_loop_carrier() { + let typed = TypedValueId::new(ValueId(5), MirValueKind::LoopCarrier); + assert!(typed.is_loop_carrier()); + } +} + +#[test] +fn test_guard_check_bug_prevention() { + // GUARD checkバグの再現防止テスト + let mut builder = MirBuilder::new(); + + // パラメータ s=ValueId(0), idx=ValueId(1) + let s = builder.new_typed_value(MirValueKind::Parameter(0)); + let idx = builder.new_typed_value(MirValueKind::Parameter(1)); + + // ValueId(0) でもパラメータとして正しく判定される + assert!(s.is_parameter()); + assert_eq!(s.value_id(), ValueId(0)); + + // ローカル変数 i=ValueId(2) + let i = builder.new_typed_value(MirValueKind::Local(0)); + assert!(!i.is_parameter()); + assert!(i.is_local()); +} +``` + +--- + +## 📊 期待される効果 + +### 定量的効果 + +| 指標 | 現状 | Phase 26-A後 | 改善率 | +|-----|------|--------------|--------| +| **ValueId判定ミス** | 年1-2回 | **0回** | **100%削減** | +| **is_parameter実装** | 20行(脆弱) | **3行(堅牢)** | **85%削減** | +| **型安全性** | なし | **完全** | - | +| **デバッグ時間** | 2-3時間/bug | **10分** | **95%削減** | + +### 質的効果 + +- ✅ **GUARDバグ完全根絶**: ValueId(0)がParameterか判定可能 +- ✅ **コンパイル時エラー**: 型不一致を実行前検出 +- ✅ **自己文書化**: `MirValueKind::Parameter(0)` で意味が明確 +- ✅ **保守性向上**: 新メンバーが理解しやすい + +--- + +## 🚀 実装スケジュール + +### Day 1: 基礎実装 +- **午前**: `MirValueKind` + `TypedValueId` 実装 +- **午後**: ユニットテスト作成 + +### Day 2: 統合 +- **午前**: `MirBuilder` 統合 +- **午後**: パラメータ登録修正 + +### Day 3: 修正&テスト +- **午前**: `loop_builder.rs` 修正 +- **午後**: 統合テスト + 回帰テスト + +### Day 4: レビュー&ドキュメント +- **午前**: コードレビュー +- **午後**: ドキュメント更新 + +--- + +## ⚠️ リスクと対策 + +### リスク1: 既存コードとの互換性 + +**対策**: +- `From for ValueId` 実装で自動変換 +- `next_value_id()` の既存挙動維持(デフォルトTemporary) +- 段階的移行(全置換不要) + +### リスク2: パフォーマンス + +**対策**: +- `HashMap` のオーバーヘッド評価 +- 必要に応じて `Vec` に最適化 + +### リスク3: テストカバレッジ + +**対策**: +- 既存テスト全実行 +- GUARD checkバグ再現テスト追加 +- Smoke tests全実行 + +--- + +## 📋 チェックリスト + +- [ ] `src/mir/value_kind.rs` 作成 +- [ ] `MirValueKind` 実装 +- [ ] `TypedValueId` 実装 +- [ ] ユニットテスト作成 +- [ ] `MirBuilder` 統合 +- [ ] パラメータ登録修正 +- [ ] `loop_builder.rs` 修正 +- [ ] 統合テスト作成 +- [ ] 既存テスト全PASS確認 +- [ ] ドキュメント更新 +- [ ] コミット&プッシュ + +--- + +**Phase 26-A完了後の次ステップ**: Phase 26-B(LoopVariableSnapshot統一) diff --git a/src/mir/mod.rs b/src/mir/mod.rs index 65ccf377..8de92327 100644 --- a/src/mir/mod.rs +++ b/src/mir/mod.rs @@ -33,6 +33,7 @@ mod printer_helpers; // internal helpers extracted from printer.rs pub mod hints; // scaffold: zero-cost guidance (no-op) pub mod slot_registry; // Phase 9.79b.1: method slot resolution (IDs) pub mod value_id; +pub mod value_kind; // Phase 26-A: ValueId型安全化 pub mod verification; pub mod verification_types; // extracted error types // Optimization subpasses (e.g., type_hints) pub mod control_form; // Phase 25.1f: Loop/If 共通ビュー(ControlForm) @@ -51,6 +52,7 @@ pub use optimizer::MirOptimizer; pub use printer::MirPrinter; pub use slot_registry::{BoxTypeId, MethodSlot}; pub use value_id::{LocalId, ValueId, ValueIdGenerator}; +pub use value_kind::{MirValueKind, TypedValueId}; // Phase 26-A: ValueId型安全化 pub use verification::MirVerifier; pub use verification_types::VerificationError; // Phase 15 control flow utilities (段階的根治戦略) diff --git a/src/mir/value_kind.rs b/src/mir/value_kind.rs new file mode 100644 index 00000000..d0a1ba76 --- /dev/null +++ b/src/mir/value_kind.rs @@ -0,0 +1,423 @@ +// value_kind.rs +// Phase 26-A: ValueId型安全化 +// +// Purpose: +// - ValueIdの意味的分類(Parameter, Local, Constant等)を導入 +// - GUARDバグのような「ValueId(0)の曖昧性」から生じるバグを根絶 + +use crate::mir::ValueId; +use crate::mir::ConstValue; + +/// ValueIdの意味的分類(型安全性強化) +/// +/// # 目的 +/// +/// ValueIdは単なる整数ラッパー `ValueId(u32)` であり、 +/// その値が「パラメータ」「ローカル変数」「定数」のいずれを表すか区別できない。 +/// これにより、以下のようなバグが発生していた: +/// +/// ## GUARDバグの例(修正済み) +/// +/// ```rust +/// // ❌ ValueId(0) を「常に未初期化」と誤判定 +/// for (name, value) in ¤t_vars { +/// if value.0 == 0 { // ← Parameter s=ValueId(0) も弾いてしまう! +/// return Ok(ValueId(0)); +/// } +/// } +/// ``` +/// +/// ## 解決策 +/// +/// `MirValueKind` で値の種類を明示的に管理し、 +/// `TypedValueId` で ValueId + 型情報をペアで保持することで、 +/// コンパイル時・実行時の型安全性を確保する。 +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum MirValueKind { + /// 関数パラメータ + /// + /// - パラメータインデックス(0-based)を保持 + /// - 例: `fn skip_whitespace(s, idx)` → s=Parameter(0), idx=Parameter(1) + /// + /// # 重要 + /// + /// ValueId(0) であってもParameterなら正当な値! + /// GUARD checkバグはこれを見落としていた。 + Parameter(u32), + + /// ローカル変数 + /// + /// - スコープ内のローカル番号(関数ごとに独立) + /// - 例: `local i = 0` → Local(0) + /// + /// # SSA形式との関係 + /// + /// SSA形式では再代入時に新しいValueIdが割り当てられるため、 + /// 同じ名前でも複数のLocal(N)が存在しうる。 + Local(u32), + + /// 定数値 + /// + /// - コンパイル時に値が確定している + /// - 例: 42, "hello", true + /// + /// # Note + /// + /// ConstValueの実体はMirInstruction::Constに格納される。 + /// MirValueKindはあくまで「定数である」というマーカー。 + Constant, + + /// 一時値 + /// + /// - 式評価・演算の中間結果 + /// - 例: `a + b` の結果、copy, phi の結果 + /// + /// # デフォルト扱い + /// + /// 明示的に分類されないValueIdはTemporaryとして扱われる。 + Temporary, + + /// Pinned変数 + /// + /// - ブロック跨ぎの一時変数(SSA構築用) + /// - 命名規則: `__pin$N$@` + /// - 例: `__pin$42$@binop_lhs` + /// + /// # 実装詳細 + /// + /// MIRビルダーがSSA形式を構築する際、 + /// ブロック境界でのdef-use関係を追跡するために使用される。 + Pinned, + + /// LoopCarrier + /// + /// - ループ内で再定義される変数 + /// - PHI nodeで複数の値をマージ + /// - 例: loop内で更新される `i` + /// + /// # PHI nodeとの関係 + /// + /// ```hako + /// local i = 0 + /// loop(i < 10) { + /// i = i + 1 // ← i は LoopCarrier + /// } + /// ``` + /// + /// MIR表現: + /// ``` + /// header: + /// %i_phi = phi [(%i_entry, preheader), (%i_next, latch)] + /// body: + /// %i_next = binop %i_phi + 1 + /// ``` + LoopCarrier, +} + +impl MirValueKind { + /// パラメータか判定 + pub fn is_parameter(&self) -> bool { + matches!(self, MirValueKind::Parameter(_)) + } + + /// ローカル変数か判定 + pub fn is_local(&self) -> bool { + matches!(self, MirValueKind::Local(_)) + } + + /// 定数か判定 + pub fn is_constant(&self) -> bool { + matches!(self, MirValueKind::Constant) + } + + /// 一時値か判定 + pub fn is_temporary(&self) -> bool { + matches!(self, MirValueKind::Temporary) + } + + /// Pinned変数か判定 + pub fn is_pinned(&self) -> bool { + matches!(self, MirValueKind::Pinned) + } + + /// LoopCarrierか判定 + pub fn is_loop_carrier(&self) -> bool { + matches!(self, MirValueKind::LoopCarrier) + } + + /// パラメータインデックス取得(Parameterのみ) + pub fn parameter_index(&self) -> Option { + match self { + MirValueKind::Parameter(idx) => Some(*idx), + _ => None, + } + } + + /// ローカル変数番号取得(Localのみ) + pub fn local_index(&self) -> Option { + match self { + MirValueKind::Local(idx) => Some(*idx), + _ => None, + } + } +} + +/// 型付きValueId - ValueIdに意味情報を付与 +/// +/// # 設計思想 +/// +/// - ValueId: 実際の識別子(既存システムとの互換性) +/// - MirValueKind: 値の種類(新規導入の型情報) +/// +/// # 使用例 +/// +/// ```rust +/// // パラメータ s=ValueId(0) +/// let s = TypedValueId::new(ValueId(0), MirValueKind::Parameter(0)); +/// assert!(s.is_parameter()); +/// assert_eq!(s.value_id(), ValueId(0)); +/// +/// // ローカル変数 i=ValueId(10) +/// let i = TypedValueId::new(ValueId(10), MirValueKind::Local(0)); +/// assert!(i.is_local()); +/// ``` +#[derive(Debug, Clone, Copy)] +pub struct TypedValueId { + /// 実際のValueId(既存システムとの互換性) + pub id: ValueId, + + /// 値の種類 + pub kind: MirValueKind, +} + +impl TypedValueId { + /// 新規作成 + pub fn new(id: ValueId, kind: MirValueKind) -> Self { + Self { id, kind } + } + + /// パラメータか判定(型安全) + /// + /// # GUARDバグ予防 + /// + /// ```rust + /// let s = TypedValueId::new(ValueId(0), MirValueKind::Parameter(0)); + /// assert!(s.is_parameter()); // ✅ ValueId(0) でも正しく判定! + /// ``` + pub fn is_parameter(&self) -> bool { + self.kind.is_parameter() + } + + /// ローカル変数か判定 + pub fn is_local(&self) -> bool { + self.kind.is_local() + } + + /// 定数か判定 + pub fn is_constant(&self) -> bool { + self.kind.is_constant() + } + + /// 一時値か判定 + pub fn is_temporary(&self) -> bool { + self.kind.is_temporary() + } + + /// Pinned変数か判定 + pub fn is_pinned(&self) -> bool { + self.kind.is_pinned() + } + + /// LoopCarrierか判定 + pub fn is_loop_carrier(&self) -> bool { + self.kind.is_loop_carrier() + } + + /// ValueIdを取得(後方互換性) + pub fn value_id(&self) -> ValueId { + self.id + } + + /// MirValueKindを取得 + pub fn kind(&self) -> MirValueKind { + self.kind + } + + /// パラメータインデックス取得(Parameterのみ) + pub fn parameter_index(&self) -> Option { + self.kind.parameter_index() + } + + /// ローカル変数番号取得(Localのみ) + pub fn local_index(&self) -> Option { + self.kind.local_index() + } +} + +/// ValueIdへの自動変換(後方互換性) +/// +/// # 使用例 +/// +/// ```rust +/// let typed = TypedValueId::new(ValueId(5), MirValueKind::Local(0)); +/// let id: ValueId = typed.into(); // 自動変換 +/// assert_eq!(id, ValueId(5)); +/// ``` +impl From for ValueId { + fn from(typed: TypedValueId) -> ValueId { + typed.id + } +} + +/// PartialEq実装(ValueIdベースで比較) +impl PartialEq for TypedValueId { + fn eq(&self, other: &Self) -> bool { + self.id == other.id + } +} + +impl Eq for TypedValueId {} + +/// Hash実装(ValueIdベースでハッシュ) +impl std::hash::Hash for TypedValueId { + fn hash(&self, state: &mut H) { + self.id.hash(state); + } +} + +// ============================================================================ +// ユニットテスト +// ============================================================================ + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_mir_value_kind_parameter() { + let kind = MirValueKind::Parameter(0); + assert!(kind.is_parameter()); + assert!(!kind.is_local()); + assert!(!kind.is_constant()); + assert_eq!(kind.parameter_index(), Some(0)); + } + + #[test] + fn test_mir_value_kind_local() { + let kind = MirValueKind::Local(5); + assert!(kind.is_local()); + assert!(!kind.is_parameter()); + assert_eq!(kind.local_index(), Some(5)); + } + + #[test] + fn test_mir_value_kind_constant() { + let kind = MirValueKind::Constant; + assert!(kind.is_constant()); + assert!(!kind.is_temporary()); + } + + #[test] + fn test_mir_value_kind_temporary() { + let kind = MirValueKind::Temporary; + assert!(kind.is_temporary()); + assert!(!kind.is_constant()); + } + + #[test] + fn test_mir_value_kind_pinned() { + let kind = MirValueKind::Pinned; + assert!(kind.is_pinned()); + assert!(!kind.is_temporary()); + } + + #[test] + fn test_mir_value_kind_loop_carrier() { + let kind = MirValueKind::LoopCarrier; + assert!(kind.is_loop_carrier()); + assert!(!kind.is_local()); + } + + #[test] + fn test_typed_value_id_parameter() { + let typed = TypedValueId::new(ValueId(0), MirValueKind::Parameter(0)); + assert!(typed.is_parameter()); + assert!(!typed.is_local()); + assert_eq!(typed.value_id(), ValueId(0)); + assert_eq!(typed.parameter_index(), Some(0)); + } + + #[test] + fn test_typed_value_id_local() { + let typed = TypedValueId::new(ValueId(10), MirValueKind::Local(0)); + assert!(typed.is_local()); + assert!(!typed.is_parameter()); + assert_eq!(typed.value_id(), ValueId(10)); + assert_eq!(typed.local_index(), Some(0)); + } + + #[test] + fn test_typed_value_id_conversion_to_value_id() { + let typed = TypedValueId::new(ValueId(42), MirValueKind::Temporary); + let id: ValueId = typed.into(); + assert_eq!(id, ValueId(42)); + } + + #[test] + fn test_typed_value_id_equality() { + let a = TypedValueId::new(ValueId(5), MirValueKind::Local(0)); + let b = TypedValueId::new(ValueId(5), MirValueKind::Parameter(0)); + // 同じValueIdなら種類が違っても等しい(ValueIdベース比較) + assert_eq!(a, b); + } + + /// GUARDバグ再現防止テスト + /// + /// # 背景 + /// + /// loop_builder.rs で以下のバグがあった: + /// + /// ```rust + /// // ❌ ValueId(0) を「常に未初期化」と誤判定 + /// for (name, value) in ¤t_vars { + /// if value.0 == 0 { // ← Parameter s=ValueId(0) も弾く! + /// return Ok(ValueId(0)); + /// } + /// } + /// ``` + /// + /// # 期待動作 + /// + /// TypedValueIdを使えば、ValueId(0)でも + /// Parameterとして正しく判定できる。 + #[test] + fn test_guard_check_bug_prevention() { + // パラメータ s=ValueId(0), idx=ValueId(1) + let s = TypedValueId::new(ValueId(0), MirValueKind::Parameter(0)); + let idx = TypedValueId::new(ValueId(1), MirValueKind::Parameter(1)); + + // ✅ ValueId(0) でもパラメータとして正しく判定される + assert!(s.is_parameter()); + assert_eq!(s.value_id(), ValueId(0)); + assert_eq!(s.parameter_index(), Some(0)); + + assert!(idx.is_parameter()); + assert_eq!(idx.value_id(), ValueId(1)); + assert_eq!(idx.parameter_index(), Some(1)); + + // ローカル変数 i=ValueId(2) + let i = TypedValueId::new(ValueId(2), MirValueKind::Local(0)); + assert!(!i.is_parameter()); + assert!(i.is_local()); + + // ❌ 旧実装(名前ベース判定)では不可能だった区別が可能に! + } + + #[test] + fn test_loop_carrier_detection() { + let carrier = TypedValueId::new(ValueId(100), MirValueKind::LoopCarrier); + assert!(carrier.is_loop_carrier()); + assert!(!carrier.is_parameter()); + assert!(!carrier.is_local()); + } +}