Skip to content

fix(hy-check): reject mismatched operand types in binary arithmetic and comparisons (fixes #922) - #1458

Closed
Ljunqiang wants to merge 1 commit into
limitlessprojects:mainfrom
Ljunqiang:fix/issue-922-binop-operand-types
Closed

Ljunqiang wants to merge 1 commit into
limitlessprojects:mainfrom
Ljunqiang:fix/issue-922-binop-operand-types

Conversation

@Ljunqiang

Copy link
Copy Markdown

Fixes #922

Root cause

BuildableChecker::check_binary_operands compared numeric families, not concrete kinds:

arm old mismatch term effect
< <= > >= lk.is_numeric() != rk.is_numeric() int vs float64 both numeric → never mismatched
+ same x + f accepted
- * / kind membership {Unknown, Int, Float} only x - f, x * 2.5 accepted

All six issue repros checked clean (0 diagnostics), then go build refused each with invalid operation: ... (mismatched types int and float64) — GUARANTEES.md Row 13 (BUILDABLE).

Fix

New Collector::numeric_adapt_conflict(lk, rk, lhs, rhs) — applied as an else if in the three arms — rejects cross-kind numeric pairs unless Go's only implicit numeric conversion bridges them:

spelling verdict why
1 + 2.5, a + b (both untyped consts) accept untyped constant expression
f + 1, 1 + f, f + k (untyped const k = 1) accept int const → float64 adaptation
x + 1, x < 10, s + "b", s < "c" accept same-kind / string families
x + f, f + x, x - f, x * f, x / f reject two typed operands
x < f, f <= x, x > f, x >= f reject two typed operands
x + 2.5, 2.5 - x, x < 2.5 reject untyped float const against typed int
f + k with const k int = 1 reject annotation makes k typed

Define.untyped_const (new field, from the already-recorded LocalDecl.ty) distinguishes fn-local const k = 1 from const k int = 1 — the annotation existed but was never consulted (tightens #1112's fail-open on this site; assignment sites keep #1112's rule until a dedupe).

Validation (all on this branch @ 47eb793)

  • hy-check/tests/binop_operand_types_issue_922.rs 24/24 (15 reject faces, 9 accept controls)
  • cargo test --workspace green except pre-existing local-CRLF spec_t3_strings (reproduces on clean upstream/main)
  • corpus pin binop-mismatched-numeric-reject.hy — EXPECTED-VERDICT: REJECT BUILDABLE + HYB-E1002; census 126 → 127 on all 7 surfaces (parse-all assertion included)
  • harness 127/127, tripwire not fired
  • negative flip: baseline (upstream/main buildable.rs) accepts the pin → harness 126/127 with the pin FAILing; restoring the fix → 127/127
  • differential 40/40 accept (build+run) + 87/87 reject

Credit / coordination

…nd comparisons (fixes limitlessprojects#922)

Root cause: check_binary_operands compared numeric FAMILIES, not concrete
kinds — for `< <= > >=` and `+` the mismatch term was
`lk.is_numeric() != rk.is_numeric()` (int and float64 are both numeric, so
never mismatched), and `- * /` only checked kind membership. `x + f`,
`x - f`, `x * f`, `x / f`, `x < f`, `f <= x` checked clean while go build
refused each with `mismatched types int and float64` (GUARANTEES Row 13).

Fix: a new Collector::numeric_adapt_conflict routes cross-kind operand
pairs through Go's only implicit numeric conversion — untyped-constant
adaptation — and emits HYB-E1002 when it cannot bridge them:
  - `1 + 2.5` (both untyped) and `f + 1` / `1 + f` (int const -> float64)
    keep building, including untyped `const` identifiers (`f + k`);
  - `x + f`, `f + x`, `x - f`, `x < f`, `f <= x`, and the untyped float
    const against a typed int (`x + 2.5`) are rejected;
  - `Define.untyped_const` (from LocalDecl.ty) distinguishes `const k = 1`
    (untyped) from `const k int = 1` (typed like a variable) for fn-local
    consts — the annotation was recorded but never consulted.

Validation: new hy-check/tests/binop_operand_types_issue_922.rs 24/24
(15 reject faces + 9 accept controls); corpus pin
binop-mismatched-numeric-reject.hy (REJECT BUILDABLE + HYB-E1002), census
126 -> 127 on all 7 surfaces; harness 127/127, tripwire not fired;
negative flip: baseline upstream accepts the pin (126/127, pin FAILs) and
restores to 127/127; differential 40/40 accept + 87/87 reject;
cargo test --workspace green except the pre-existing local-CRLF
spec_t3_strings failure (reproduces on clean HEAD).

Harvest credit: stale companion rickymulya PR limitlessprojects#923 (bot:stale-base +
bot:conflict) proposed the same family of rules with literal-only
helpers; this take supersedes it with const-ident support and the
untyped_const annotation check.

Coordination: limitlessprojects#1428, limitlessprojects#1430 and limitlessprojects#1447 also bump census 126 -> 127 — the
PR that lands last rebases the count to 128+. The numeric_adapt_conflict
doc carries a dedupe NOTE against limitlessprojects#1447's is_untyped_constant (the
kinds_conflict / PR limitlessprojects#620 duplicate-helper precedent) for whoever lands
second.
@Ljunqiang
Ljunqiang force-pushed the fix/issue-922-binop-operand-types branch from 47eb793 to 84a050e Compare October 24, 2026 12:16
@limitlessprojects

Copy link
Copy Markdown
Owner

Queue-reset sweep: this repo now runs a rolling conveyor, and open PRs
outside the active review window are being closed as not planned to keep the
queue honest — a stale queue wastes every contributor's time, including yours.

This is not a verdict on the work. If this change still matters:

  1. rebase against current main (main has landed a lot since this PR opened),
  2. confirm the diff still applies and the affected verdicts still hold,
  3. reopen — the triage board picks reopened PRs up as fresh arrivals.

Wildcard holds (verdict window 2026-11-01) and Pareto land candidates are
unaffected. Thanks for the effort — high-volume repos only stay reviewable
with a queue that actually drains.

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.

[Soundness] hy-check: binary arithmetic and relational operators permit mismatched numeric types (HYB-E1002)

2 participants