Add mruby-string-bitops gem - #7011

mrbgems/mruby-string-bitops/test/string_bitops.rb (2)

148-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding a self-aliasing case for the bang operations.

The bang kernels are called with dst == lhs. When the caller passes the receiver as the operand, rhs also aliases dst. The kernels read each word into locals before the store, so the result should be correct. A test would lock that behavior in.

💚 Proposed test addition

   assert_equal s.object_id, s.bitwise_xor!("\xFF").object_id
   assert_equal "\x33", s
+
+  # Self-aliasing: receiver and operand are the same object.
+  u = "\xA5" * 20
+  u.bitwise_xor!(u)
+  assert_equal "\x00" * 20, u

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mrbgems/mruby-string-bitops/test/string_bitops.rb` around lines 148 - 154,
Add self-aliasing coverage for the bang operations in the existing string bitops
test, invoking bitwise_and!, bitwise_or!, and bitwise_xor! with the receiver
itself as the operand. Assert each operation preserves correct in-place results
and returns the same receiver object.

---

171-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering zero-length operands on both sides.

The length-mismatch cases are covered. The valid zero-length case is not. Add "".bitwise_and(""), "".bitwise_not, and "".bit_get(0) to confirm that the kernels and the bounds checks handle len == 0.

💚 Proposed test addition

+  assert_equal "", "".bitwise_and("")
+  assert_equal "", "".bitwise_or("")
+  assert_equal "", "".bitwise_xor("")
+  assert_equal "", "".bitwise_not
+  assert_nil "".bit_get(0)
   # Length mismatch: other longer, shorter, and empty.
   assert_raise(ArgumentError) { "\xF0".bitwise_and("\x00\x00") }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mrbgems/mruby-string-bitops/test/string_bitops.rb` around lines 171 - 179,
Add zero-length operand coverage to the string bitops tests by asserting valid
results for "".bitwise_and(""), "".bitwise_not, and "".bit_get(0), alongside the
existing mismatch cases. Ensure these assertions verify the kernels and bounds
behavior for len == 0.

mrbgems/mruby-string-bitops/README.md (1)

22-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the lsb_first validation rule.

The implementation raises ArgumentError when lsb_first is neither true nor false. The test file asserts this behavior. Add that rule here so users know that nil is rejected instead of being treated as false.

📝 Proposed documentation addition

 `IndexError` is raised when `offset` is negative, or (for the mutating
 methods) when it is beyond the end of the string.
+
+`lsb_first` accepts only `true` or `false`. Any other value, including
+`nil`, raises `ArgumentError`.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mrbgems/mruby-string-bitops/README.md` around lines 22 - 28, Update the
argument-validation documentation in the mruby-string-bitops README to state
that lsb_first must be exactly true or false; any other value, including nil,
raises ArgumentError. Keep the existing offset and IndexError behavior
documentation unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

mrbgems/mruby-string-bitops/src/string_bitops.c:478

• If to_str raises an exception, this helper continues and then raises a new TypeError, which hides the original exception. Re-raise immediately when mrb_funcall_argv sets mrb->exc so exceptions from user-defined to_str propagate correctly.

  if (mrb_respond_to(mrb, other, MRB_SYM(to_str))) {
    mrb_value converted = mrb_funcall_argv(mrb, other, MRB_SYM(to_str), 0, NULL);
    if (mrb_string_p(converted)) {
      return converted;
    }
  }

mrbgems/mruby-string-bitops/src/string_bitops.c:496

• This conversion error uses %t (type name). Elsewhere in mruby, conversion errors typically use %Y so the message includes the actual offending value (see mrb_ensure_string_type in src/object.c). Switching to %Y improves diagnostics and keeps messages consistent.

  mrb_raisef(mrb, E_TYPE_ERROR, "%t cannot be converted to String", other);

mrbgems/mruby-string-bitops/src/string_bitops.c:291

• Conversion error messages here use %t (type name) but mruby’s core conversion helpers use %Y to include the offending value (e.g. mrb_ensure_integer_type in src/object.c). Using %Y makes the error more informative and consistent across the codebase.

This issue also appears on line 496 of the same file.

    mrb_raisef(mrb, E_TYPE_ERROR, "%t cannot be converted to Integer", index);

mrbgems/mruby-string-bitops/src/string_bitops.c:286

• If to_int raises inside mrb_funcall_argv, mrb->exc will be set and this helper currently continues, potentially overwriting the original exception with the later TypeError and/or inspecting an invalid return value. Check mrb->exc immediately after the call and re-raise to preserve the original exception semantics.

      mrb_value converted = mrb_funcall_argv(mrb, index, MRB_SYM(to_int), 0, NULL);
      if (mrb_type(converted) == MRB_TT_INTEGER || mrb_type(converted) == MRB_TT_BIGINT) {

mrbgems/mruby-string-bitops/src/string_bitops.c:492

• If to_str raises inside mrb_funcall_argv, mrb->exc will be set and this helper currently continues, potentially overwriting the original exception with the later TypeError and/or inspecting an invalid return value. Check mrb->exc immediately after the call and re-raise to preserve the original exception semantics.

    mrb_value converted = mrb_funcall_argv(mrb, other, MRB_SYM(to_str), 0, NULL);
    if (mrb_string_p(converted)) {