0xMiden/miden-vm

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

オープン

#3,676 opened on 2026/08/19

 (0 件のコメント) (0 件のリアクション) (0 人の担当者)Rust (303 件のフォーク)github user discovery
corelibdocumentationgood first issue

Repository metrics

Stars
 (753 個のスター)
PR merge metrics
 (平均マージ 6d 2h) (30d で 71 merged PRs)

説明

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.

コントリビューターガイド