Skip to content

Implement bits.* builtins#149

Open
ume3445 wants to merge 1 commit into
open-policy-agent:mainfrom
ume3445:implement-bits-builtins
Open

Implement bits.* builtins#149
ume3445 wants to merge 1 commit into
open-policy-agent:mainfrom
ume3445:implement-bits-builtins

Conversation

@ume3445

@ume3445 ume3445 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements bits.and, bits.or, bits.xor, bits.negate, bits.lsh, and bits.rsh as described in #127.

Changes

  • Added BitsBuiltins.java following the same numeric-builtin pattern as ArithmeticBuiltins.java (single class, builtins() map of method references)
  • Uses BigInteger throughout for arbitrary-precision arithmetic, required per the compliance fixtures showing shift operations on max int32/int64 values must not overflow or truncate
  • Validates operand types and shift-amount sign per the exact error message wording in the compliance fixtures (e.g. "bits.and: operand 1 must be integer number but got floating-point number", "bits.lsh: operand 2 must be an unsigned integer number but got a negative integer")

Testing

  • Ran the full compliance suite locally: all 19 bits/* test cases pass across all six functions
  • Full opa-evaluator suite shows no new regressions (confirmed by reverting the change and reproducing the same pre-existing, unrelated failure count)

Closes #127.

…n-policy-agent#127)

Signed-off-by: Muhammad Umer Hammad <umerhammad010@gmail.com>

@sspaink sspaink left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bitwise math matches OPA's big.Int semantics and the negative-shift / type-error message wording matches the reference. Two operand-validation gaps make it diverge from OPA on integer-valued and fractional floats, and one edge case throws an uncaught exception. All verified against the OPA binary and v1/topdown/bits.go. Findings 1 and 2 share one fix: the integer check should reject only non-integral/infinite values (Double.isFinite(d) && d == Math.rint(d)) and be applied to both x and the shift amount s — matching OPA, which accepts 2.0 but rejects 2.5.

RegoNumber s = getArg(args, 1, RegoNumber.class);
requireInteger(x, "bits.lsh", 1);
requireUnsigned(s, "bits.lsh");
return new RegoBigInt(x.getBigIntValue().shiftLeft(s.getBigIntValue().intValueExact()));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shift operand s is never validated as an integer, so fractional shifts are silently truncated instead of erroring. lsh/rsh call requireInteger on x but only requireUnsigned on s — never requireInteger(s, ...). For bits.lsh(1, 2.5), RegoDecimal.getBigIntValue() does BigInteger.valueOf((long) 2.5) = 2, so this returns 4. OPA raises bits.lsh: operand 2 must be integer number but got floating-point number (undefined without --strict-builtin-errors, eval_type_error with it) — confirmed on the OPA binary. Reachable via JSON input {"s": 2.5}. Same issue in rsh at line 120.

return new RegoBigInt(x.getBigIntValue().shiftRight(s.getBigIntValue().intValueExact()));
}

private static void requireInteger(RegoNumber n, String fn, int operandIndex) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integer-valued floats are wrongly rejected across all six functions. requireInteger throws for any isDecimal(), but OPA accepts floats that are exactly integral (its NumberToInt only fails when the value isn't big.Exact). Confirmed on the OPA binary: bits.and(2.0, 3)2, bits.negate(5.0)-6, bits.lsh(1, 2.0)4, but all throw TypeError here. Reachable via JSON input {"n": 2.0}, which Jackson maps to RegoDecimal (RegoValueModule.java:182-183). Static Rego literals like 2.0 are safe (TypeUtils.parseStringToNumber folds whole floats to integer types), so only input-supplied values trigger it. Suggest checking integrality (Double.isFinite(d) && d == Math.rint(d)) rather than isDecimal().

RegoNumber s = getArg(args, 1, RegoNumber.class);
requireInteger(x, "bits.lsh", 1);
requireUnsigned(s, "bits.lsh");
return new RegoBigInt(x.getBigIntValue().shiftLeft(s.getBigIntValue().intValueExact()));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intValueExact() throws an uncaught ArithmeticException for shift amounts above Integer.MAX_VALUE. s.getBigIntValue().intValueExact() throws ArithmeticException (not a TypeError/BuiltinError) for e.g. bits.lsh(1, 2147483648), so it escapes the builtin error path as an unexpected exception. OPA instead does uint(b.Uint64()) and attempts the shift. Lower severity (huge shifts are impractical either way), but the failure mode differs from a clean Rego error. Also applies to rsh at line 120.

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.

Implement bitwise builtins (bits.*)

2 participants