doc: add a Claude Code rule for the C/Ruby boundary - #6998
Conversation
`CONTRIBUTING.md` covers C99 conformance, minimal library dependencies, the newline after a return type, and ISO/IEC 30170:2012 conformance for Ruby code, but it says nothing about when a C function may re-enter the VM. The convention surfaced in fd5e075. `Regexp.__match_pattern`, added in mruby#6994, compiled an accepted `String` with `mrb_funcall_argv1(Regexp, new, str)`, and matz split it so that the type check stays in C while the `Regexp.new` construction moves to mrblib. This file states that boundary along with the existing exemptions, such as the `to_str` conversion protocol and the `initialize` call in `mrb_obj_new`, so that coding agents working in this tree do not have to rediscover it.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a repository rule that defines the C/Ruby boundary in mruby. It assigns low-level operations to C, dispatching behavior to Ruby, and documents VM-entry exemptions and review requirements. ChangesC/Ruby boundary guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.claude/rules/mruby-c-ruby-boundary.md (1)
85-98: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDocument the intentional
hashVM entry.
mrb_obj_hash_codeinsrc/hash.c:339-382callsmrb_funcall_argvfor non-primitive keys so user-definedhashmethods work. This call is not listed under the existing exemptions. Add it, or state that operations whose Ruby semantics require overridablehashdispatch are exempt.Without this clarification, future reviews may apply the prohibition too broadly and remove required Ruby dispatch.
Proposed clarification
### Existing exempt cases +- Object hashing in `mrb_obj_hash_code` in `src/hash.c`, where dispatch to + `hash` is required to preserve Ruby's overridable hashing semantics.🤖 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 @.claude/rules/mruby-c-ruby-boundary.md around lines 85 - 98, Update the “Existing exempt cases” section to explicitly exempt mrb_obj_hash_code in src/hash.c, including its intentional mrb_funcall_argv dispatch for non-primitive keys, or state the equivalent rule that Ruby operations requiring overridable hash dispatch are exempt. Preserve the prohibition for unrelated C-to-Ruby calls.
🤖 Prompt for all review comments with 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.
Inline comments:
In @.claude/rules/mruby-c-ruby-boundary.md:
- Around line 2-7: The path patterns in the rule’s paths configuration do not
cover nested gem sources or gem-local headers. Replace the non-recursive mrbgems
src and mrblib patterns with recursive equivalents, and add a recursive mrbgems
include header pattern so all gem C, header, and Ruby files are matched.
---
Nitpick comments:
In @.claude/rules/mruby-c-ruby-boundary.md:
- Around line 85-98: Update the “Existing exempt cases” section to explicitly
exempt mrb_obj_hash_code in src/hash.c, including its intentional
mrb_funcall_argv dispatch for non-primitive keys, or state the equivalent rule
that Ruby operations requiring overridable hash dispatch are exempt. Preserve
the prohibition for unrelated C-to-Ruby calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cc9d74b8-21ec-4ae2-86a1-1811d800774b
📒 Files selected for processing (1)
.claude/rules/mruby-c-ruby-boundary.md
The rule already applied to `include/**/*.h` at the top level but not to gem-local headers such as `mrbgems/mruby-regexp/include/re_internal.h`, and it missed the C sources under `mrbgems/*/tools/`, which implement `mirb` and `mrdb`. The gem `src` and `mrblib` patterns stay non-recursive on purpose. The only nested sources under a gem `src` directory are the vendored prism files in `mrbgems/mruby-compiler/lib/prism/src/`, and no gem has a nested `mrblib` directory at all.
|
Thank you for writing this up. You found a real gap, and I want to be clear that the substance is right: the convention exists, it is not written down anywhere in the tree, and you had no way to discover it before writing the code in #6994. I checked every exemption you listed against the source and they are all accurate. I am going to decline the location and take the content. Why not
|
|
Thank you for the detailed explanation and for incorporating the substance into I had not realized that the absence of tool-specific files was a deliberate repository policy. The distinction makes sense: a project-wide convention should live in the canonical contributor documentation, where human contributors and coding agents can use the same source of truth. The condensed section preserves the important boundary, its rationale, the The note about Thank you again for reviewing the details and preserving the substance. |
2 participants
This adds one Markdown file under
.claude/, describing where the boundarybetween C and Ruby lies in this tree. It documents an existing convention for
coding agents. It does not propose a new one, and it does not change any rule
that applies to human contributors.
CONTRIBUTING.mddocuments the coding conventions that a patch has to satisfy:C99 conformance, keeping library dependencies minimal, the newline after a
return type, and ISO/IEC 30170:2012 conformance for Ruby code. It says nothing
about where the boundary between C and Ruby lies, in particular about when a C
function may call back into the VM. That convention exists, but it is currently
learned only through review.
Background
In #6994 I added
Regexp.__match_patternin C so thatString#matchcouldreject an argument that merely pretends to be a
Regexp. The check ran in C,but the accepted
Stringwas then compiled withmrb_funcall_argv1(Regexp, new, str)in the same function.In fd5e075 matz split it: the type check stays in C, and the
Regexp.newconstruction moves to mrblib, because
Module#===cannot be redefined either,so the caller can compile the pattern without weakening the check.
The reasoning generalizes well beyond that one function, and I had no way to
find it stated anywhere in the tree. This patch writes it down.
What this adds
A single documentation file,
.claude/rules/mruby-c-ruby-boundary.md, stating:low-level primitives inside C.
normalization. The test is not whether the operation is a conversion, but
whether it involves method dispatch.
existing code that relies on them, so that they are not mistaken for
violations. This covers
convert_typeand theto_strprotocol insrc/object.c,mrb_obj_newand theinherited/included/method_missinghooks insrc/class.c, the default proc insrc/hash.c,and the
to_a/to_enumdelegations insrc/array.c.re-entrancy, exception propagation, non-local exits, GC rooting, and
evaluation order.
Every exemption listed is existing mruby code that stays exactly as it is.
Note on the location
This introduces a
.claude/directory, which the repository does not havetoday. The path is what Claude Code loads automatically for files matching the
paths:list in the front matter, so an agent editingsrc/*.cormrbgems/*/src/*.cpicks the convention up without being told.If you would rather not carry a tool-specific directory, I am happy to move the
content into
CONTRIBUTING.mdordoc/, or to drop the front matter entirely.The text itself is plain Markdown and does not depend on the location. Please
say which you prefer.
Impact
Documentation only. Nothing here runs in CI, and no workflow, source, build, or
test file is touched. The file is formatted with
prettier@3.7.4, matching themanual
prettierhook in.pre-commit-config.yaml.Summary by CodeRabbit