word::gt / word::lt: internal accumulator variable is mislabeled in comments (swapped between the two procedures)
- 主要语言
- Rust
- 星标
- 772
- 派生
- 352
- 平均合并
- 1 天 12 小时
- 30 天内合并 PR
- 93
描述
## 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.
贡献指南
评估
这个 Issue 还没有评估数据。