Skip to content

docs: add CFG 开发和设计文档 (Topic 11) - #41

Open
muykokoro wants to merge 3 commits into
ScratchV-Compiler:mainfrom
muykokoro:codex/cfg-docs
Open

docs: add CFG 开发和设计文档 (Topic 11)#41
muykokoro wants to merge 3 commits into
ScratchV-Compiler:mainfrom
muykokoro:codex/cfg-docs

Conversation

@muykokoro

Copy link
Copy Markdown

sorry to rushing to hit the ddl qwq

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

🤖 AI Code Review

共审查 10 个变更文件

📁 .gitignore

🟡 Over-broad pattern: *.png — 忽略所有 PNG 文件。如果仓库中有需要提交的资源文件(如图标、截图、文档配图),它们将无法被版本控制。

Suggestion: 考虑限定路径,如 cfg_output/*.pngbenchmarks/*.png,仅在已知生成目录中忽略。

💭 Style: 注释格式 — 新增注释 # CFG visualization artifacts (generated, never commit) 与文件中原有注释风格不一致(原文件无同类注释)。不是问题,只是风格偏差。

💭 Nit: .idea/ — 如果团队中有人使用 VS Code、PyCharm 等,可能也希望添加 .vscode/__pycache__/。不过如果已有其他机制处理,则无需追加。


📁 docs/CFG_Design.md

🔴 边构建规则歧义 — 第4.5节:BR_IF 只有 1 个 targettrue→target + FALLTHROUGH→下一个块 作为 false 路径。标准语义中,单目标条件跳转通常是"条件满足则跳转,不满足则 fallthrough",但这里把 fallthrough 标为 false 路径,与条件跳转的直觉相反。建议明确说明条件方向的约定,或在 EdgeType 中标注 condition 字段时统一语义。

🔴 支配集迭代复杂度描述错误 — 第4.2节:写"理论最坏 O(N²) 轮",实际应为 O(N) 轮(每轮至少缩小一个 Dom 集),每轮 O(N²) 工作量,总计 O(N³)。N<100 的结论不受影响,但复杂度标注有误。

🟡 instructions 引用而非拷贝 — 第3.2节设计为引用原列表,第6节仅标记为低优先级技术债。若 CFG 构建后被其他 pass 修改 IR 指令,CFG 节点将静默失效。建议在 CFGNode 初始化时深拷贝 instructions,或在文档中明确要求调用方冻结 IR 后不可修改。

🟡 successors(name) 接口与数据结构不一致 — 第3.4节 edges 为 flat list[CFGEdge],但第5节承诺 cfg.successors(name) 返回后继。若未维护邻接表,successors 将是 O(E) 线性扫描,与"O(1) 按名查找"的设计意图矛盾。建议增加 adj: dict[str, list[str]] 或类似结构。

🟡 eliminate_unreachable 破坏纯函数一致性 — 第5节中其余函数均为纯函数,唯独此函数原地修改 CFG。下游 pass 若依赖不可变性会产生隐蔽 bug。建议改为返回新 CFG,或至少标注"mutates in-place"并在接口中统一风格。

🟡 空块边构建未覆盖 — 第4.1节说"连续 label 产生空块(保留)",第4.5节"无终止符→FALLTHROUGH→下一个块"。但空块既无终止符也无计算指令,是结构产物而非真正的"纯计算块",直接生成 FALLTHROUGH 边会产生误导性的空边。建议为纯空块生成一条隐式透传边或跳过。

💭 示例块数计数 — 第1.2节 if/else 示例:入口块 → L_then / L_else → merge(return) = 4块4边,正确。但 merge 同时是出口,is_entry/is_exit 能否同时为 false/true?若入口和出口是同一块,entry == exit 时的处理未在文档中说明。

💭 第4.4节"反向 BFS" — "从 source 反向 BFS,遇到 header 即停止",描述为 BFS 实际是沿反边做 DFS 更常见(自然循环算法标准写法)。若确用 BFS 也正确,但建议在实现时注意:当多条回边指向同一 header 时,BFS 和 DFS 的 body 集合可能不同,需验证取并集逻辑的一致性。


📁 docs/CFG_Dev.md

🟡 边构建规则不完整 — 4.5 节缺少"函数末尾的最后一个块无终止指令"的情况。如果最后一个块既不是 BR/BR_IF/RETURN,且是最后一个块,则无出边——这条规则未明确写出,开发者可能误加 FALLTHROUGH 到不存在的块。

🟡 指令引用不变量有风险 — 4.3 第 4 条说 instructions 是引用非拷贝,4.4 陷阱中又提醒"构建后应冻结原 IR"。但 build_cfg_from_instructions 是纯函数,无法强制调用方冻结原列表。建议在不变量中加一句约束:调用方在获取 CFG 后不得修改原 IR 列表。

🟡 eliminate_unreachable 语义模糊 — 4.2 说"调用后应丢弃旧 CFG 引用",但该函数是原地修改,调用方持有的就是同一个引用,不存在"旧引用"。应改为"函数会修改传入的 CFG 对象,调用方不应保留对该对象的依赖"。

🟡 12 周路线缺少依赖关系标注 — 第 3 节路线图是线性的,但实际 W8(支配树)和 W9(循环检测)依赖 W7(不可达消除)和 W4(CFG 构建)。缺少标注会误导新手以为各周独立可并行。

💭 4.4 自动命名策略遗漏边界 — BR_IF 的两条分支目标都可能是未命名块(e.g. BR_IF -> b0, b1),计数器分配规则不明确:是先分配 t1 还是 t2?建议补充分配顺序(按 t1 先于 t2)。

💭 DOT 样式表缺少"无样式"兜底 — 4.6 表中"普通"节点颜色为浅黄色,但如果一个节点同时不满足入口/出口/循环头条件(理论上不应存在),渲染行为未定义。考虑加一句"All nodes fall into exactly one category"。

💭 测试矩阵缺少关键用例TestEdgeCases 中"多函数"未说明如何处理,CFG 模块是单函数构建,多函数场景应归入其他模块。建议改为"单块无指令"或"纯标签序列(无实际指令)"。


📁 examples/cfg/if_else.dsl

Code Review: examples/cfg/if_else.dsl

🟡 无变量声明/绑定语义xy 均未声明来源或类型。如果是外部输入(参数/寄存器),建议加注释说明;如果是隐式全局,需在 DSL 规范中明确约束,否则解析器可能产生未定义行为。

🟡 注释与实现可能不同步 — 注释声明 "4个基本块, 2条JUMP + 2条BRANCH",但这属于生成后的 CFG 结构断言。如果 DSL 实现变更(如优化合并基本块),此注释会悄然过期。建议要么移除,要么添加验证(如注释后加 # expected: blocks=4, jumps=2, branches=2 供测试引用)。

🟡 endif 终止符与缩进冗余 — 同时使用 endif 显式终止 + 缩进表示块结构,两套块边界机制容易在复杂嵌套时产生歧义或误配。考虑统一为纯缩进(Python 风格)或纯关键字(end 风格)。

💭 文件名含 if_else 但 DSL 语法用 if/else/endif — 语义上没问题,但如果 DSL 支持 elif,当前示例未覆盖该路径,后续示例可补充。

💭 缺少 # 预期输出 或关联的 .ast/.cfg 快照文件引用,reviewer/CI 无法快速确认该 DSL 片段是否正确编译为预期的控制流图。


📁 examples/cfg/nested_loop.dsl

🔴 Bug: Undefined Variablessum and a are used without initialization or declaration.
Suggestion: Initialize sum = 0 before the loop and either pass a as a script argument or define it locally. Using add(nil, a) will likely crash or produce unexpected results.

🟡 Suggestion: Clarify Loop Boundsfor i = 0, 3 is ambiguous regarding inclusivity.
Suggestion: If the language supports it, use explicit syntax (e.g., 0..3 or 0 to 3) or add a comment indicating whether 3 is included in the iteration count.

💭 Nit: Comment Accuracy — Comment states 内层depth=1, but the code shows two levels of nesting.
Suggestion: Update comment to reflect actual nesting depth (likely 2) or clarify if depth counting excludes the root level.


📁 examples/cfg/unreachable.dsl

🟡 未声明变量 a, b — 这两行直接使用但从未定义。如果 DSL 没有全局隐式变量约定,示例将无法通过类型检查,读者难以判断意图。
Suggestion: 添加 a = ... / b = ... 的声明,或加注释说明假设来源。

🟡 变量名 dead 与概念混淆 — 注释已经说明"死代码",再用 dead 作为变量名会让读者困惑:这行是故意命名为 dead 还是语义标注。
Suggestion: 改为 result2 = mul(a, b)extra = mul(a, b)

💭 缺少期望输出对照 — 作为 examples/ 下的文件,仅有输入没有期望的 CFG 输出(如"期望检测到 1 个不可达块"),难以验证示例正确性。
Suggestion: 附 .expected 文件或注释标注期望的不可达块位置。


📁 examples/cfg/while_loop.dsl

🔴 变量未初始化iaccx 在第 2-5 行使用,但文件中没有任何声明或初始化。如果此文件需作为独立可执行/可分析的片段,这将导致 undefined behavior。

Suggestion: 添加初始化语句,例如 let i = 0; let acc = 0; let x = <某值>;,或明确注释说明这是某个更大程序中的片段。


🟡 缺少上下文说明 — 文件名和注释暗示这是 CFG 测试用例("含回边"),但未说明前置条件。建议补充说明此片段对应的完整程序结构(如参数来源、变量定义),方便阅读者理解 CFG 的完整入口。


💭 endwhilewhile 拼写不一致 — 关键字 while 是简写形式,而 endwhile 是完整形式。考虑统一为 end(类似 Python 的 pass 风格隐式结束)或 end while(空格分隔)。非功能性问题,纯属一致性。


📁 scratchv/ir/cfg.py

🔴 Bug: 支配树立即支配节点计算错误compute_dominator_tree (lines 358-366): 当前逻辑检查的是"d 不是其他严格支配节点的严格支配者",这找到的不一定是最接近的 IDOM。正确逻辑应该是:对每个严格支配节点 d,验证 d 是否支配了所有其他严格支配节点。

# 当前(错误):
if all(d not in (dom_sets.get(o, set()) - {o}) or o == d for o in strict):
# 应为(正确):
for d in sorted(strict, key=lambda x: len(dom_sets.get(x, set()) - {x})):
    if all(d in (dom_sets.get(o, set()) - {o}) or o == d for o in strict - {d}):
        idom[node] = d
        break

🔴 Bug: BOM 字符 — 文件第 1 行包含 UTF-8 BOM (# U+FEFF),可能导致模块加载或字符串比较出现隐性 bug。移除 BOM。

🟡 重复代码build_cfg_from_instructionsCFGBuilder.build 中节点创建和边构建逻辑几乎完全一致(约 50 行重复)。建议提取为共享的 _finalize_cfg 内部方法,让 CFGBuilder.build 在其块结构上调用,或直接复用 build_cfg_from_instructions(先将 blocks 拍平为指令序列)。

🟡 入口名称不一致build_cfg_from_instructions 中当首个基本块来源于 LABEL 时,cfg.entry 被设为标签名(如 "L_0")而非 "entry"。如果后续代码(如 eliminate_unreachable)假设 entry 存在对应节点但名称为 "entry",会导致整个函数被标记为不可达。

🟡 detect_nested_loops 后写入覆盖问题 — 内层循环中 inner.parentinner.nesting_depth 会被最后一次匹配的外层循环覆盖,可能将子循环挂到错误的外层上。应在找到最浅层的外层后 break,或按外层从深到浅排序再分配。

🟡 _dfs 对缺失 entry 的容忍度 — 若 cfg.entry 不在 cfg.nodes 中(由上述入口命名 bug 触发),DFS 返回空集,eliminate_unreachable 会删除所有节点。建议在 _dfs 入口加断言或 fallback。

💭 to_dot 截断魔数str(instr)[:60][:3] 的截断阈值建议提取为参数,方便在长指令场景下调整。


📁 scripts/visualize_cfg.py

🔴 Bug: 异常未捕获 — 行 44-52:解析器抛异常(如语法错误)时程序直接崩溃,没有 try/except 包裹。
Suggestion: 捕获 Exception 并输出有意义的错误信息后返回 1,与文件不存在的处理保持一致。

🟡 Suggestion: detect_loops 多次调用无缓存 — 行 68 和行 79 各调用一次 detect_loops(cfg)show-loops 时还会额外调用 detect_nested_loops。若检测昂贵,考虑缓存结果:

loops = builder.detect_loops(cfg)

后续 --show-loopsto_dot 复用同一个 loops 变量即可。

🟡 Suggestion: eliminate_unreachable 返回值语义不清 — 行 62 对返回值 removed 做 truthy 判断,但不清楚它返回的是 int、list 还是 bool。调用方依赖隐式真值判断,建议显式类型或文档注释说明返回值含义。

🟡 Suggestion: 输出路径未校验 — 行 76 Path(args.output).with_suffix(".dot") 写文件前未检查父目录是否存在,目录不存在时会抛出 FileNotFoundError 且没有提示。

💭 Nit: 行 33print(...); return 1 写在同一行,与文件其他风格不一致,建议拆为两行。


📁 tests/test_cfg.py

🔴 BOM 字符 — 第 1 行: """ 前有 BOM (``)。会干扰 Python 导入、diff 显示。移除它。

🟡 断言过松test_if_else_blocks L132: len(cfg.nodes) >= 4test_if_else_edge_types L140-141: jumps >= 2 / branches >= 2。松的断言几乎不会 fail,失去测试价值。建议用 == 精确匹配已知 CFG 拓扑,至少用具体值如 >= 4 → 明确预期值。

🟡 test_auto_naming 不完整 — L88: 只断言 "b0" in names,没断言 len(blocks) == 2。如果实现返回 1 个块且恰好叫 b0,测试仍通过。

🟡 DOT 测试过弱 — L157: assert dot.startswith("digraph") 不区分有效/无效 DOT。建议至少检查包含节点 ID 和边结构(如 ->),或验证颜色键值完整格式 "fillcolor="#90EE90""

🟡 缺少边对称性校验test_cfg_successors_predecessors 只测了单条边。没有测试双向一致性(如加入 A→B 和 B→C 后,predecessors("C")"B"successors("A")"B"),这是 CFG 核心的不变量。

🟡 缺少回边/后向边测试 — 循环检测只验证了存在,没验证 NaturalLoop 的 header 是 back-edge 的 target,也没测 post-dominator 或 loop 的 body 节点集合。

🟡 test_removes_isolated 依赖实现细节 — L179: 直接构造 cfg.edges = [CFGEdge("A", "B")],但 eliminate_unreachable 可能依赖 successors()/predecessors() 而非 edges 列表。测试脆弱,若实现改数据结构就假阴性。

💭 多语句一行 — L248: prog.add_function(f1); prog.add_function(f2) 分号同行,与文件其余风格不一致,拆两行。

💭 缺 test 边界 — 没有覆盖:LABEL 作为第一条指令、连续多个 BRRETURN 后仍有指令(应被分入独立块或标记异常)。


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant