0xMiden/miden-vm

word::gt / word::lt: internal accumulator variable is mislabeled in comments (swapped between the two procedures)

开放

#3,676 创建于 2026年8月19日

 (0 条评论) (0 个反应) (0 位负责人)Rust (303 个派生)github user discovery
corelibdocumentationgood first issue

仓库指标

星标
 (753 个星标)
PR 合并指标
 (平均合并 6天 2小时) (30 天内合并 71 个 PR)

描述

Summary

word::gt and word::lt in crates/lib/core/asm/word.masm are near-identical procedures (the doc comment on lt even says "this is exactly the same as word::gt except it uses lt rather than gt"). The underlying logic in both is correct, but the internal accumulator variable is mislabeled in the inline comments, in a way that's swapped between the two procedures — almost certainly a copy-paste artifact from when one was derived from the other.

  • Inside gt (which computes is_lhs_greater), the accumulator is commented as is_lhs_less.
  • Inside lt (which computes is_lhs_less), the accumulator is commented as is_lhs_greater.

Why I'm flagging this despite the logic being correct

I don't want to waste anyone's time with a false bug report, so before opening this I verified the actual logic is correct: I wrote a small Miden-stack simulator (movup/movdn/dup/swap/eq/lt/gt/and/or) and ran both procedures' exact instruction sequences against the reference is_word_greater algorithm from the doc comment, across 10 test vectors (equal words, differing at each of the 4 limb positions, all-zero, etc.) — all 20 checks (gt + lt) matched. So this is purely a labeling issue, not a functional bug.

Given that, I still think it's worth fixing: mislabeled variable names in comparison-operator internals are exactly the kind of thing that could mislead someone modifying or auditing this code later (e.g. "wait, why does gt track something called is_lhs_less?").

Where

crates/lib/core/asm/word.masm, in the push.1.0 initialization comment and the accumulator-tracking comments throughout the loop body of:

  • gt (~line 94 onward) — comments say is_lhs_less, should say is_lhs_greater
  • lt (~line 178 onward) — comments say is_lhs_greater, should say is_lhs_less

Suggested fix

Swap the two mislabeled names back to match what each procedure actually computes. Happy to open a PR for this if assigned — didn't want to submit one without an issue/assignment per the contributing guidelines.

贡献者指南