Skip to content

Convert MRI comments to @CRuby - #9804

Open
zev wants to merge 1 commit into
jruby:masterfrom
zev:cruby-annotation-convert
Open

Convert MRI comments to @CRuby#9804
zev wants to merge 1 commit into
jruby:masterfrom
zev:cruby-annotation-convert

Conversation

@zev

@zev zev commented Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Follow-up to #9782: converts the existing comment-style CRuby references to @CRuby. The tool that checks the annotations against a CRuby checkout will come in a separate PR, so it can be discussed on its own.

Before After
// MRI: rb_hash_new @CRuby(value = "rb_hash_new", file = "hash.c")
/** rb_cmperr
*
*/		 @CRuby(value = "rb_cmperr", file = "compar.c")
// MRI: ary_reject_bang and reject_bang_i @CRuby(value = "ary_reject_bang", file = "array.c")
@CRuby(value = "reject_bang_i", file = "array.c")
/**
* rb_make_metaclass
* @param superClass ...
*/		 /**
* @param superClass ...
*/
@CRuby(value = "rb_make_metaclass", file = "class.c")
/** rb_ary_push - specialized rb_ary_store */ unchanged (has extra text; left for review)

Survey of the existing comments

Comments in core/src/main/java that name CRuby, checked against CRuby 4.1 (master at 6203c21e58):

Category Before After What this PR does
clean: one C name above a declaration 1046 59 converted; the 59 left name functions CRuby does not define
multi: several C names above a declaration 7 1 converted to repeated @CRuby
prose: C name alongside other comment text, e.g. a Javadoc 121 12 C name converted, the rest of the comment kept
qualified: MRI: name, extra text on one line 107 107 left for review; some may become note = "..."
elsewhere: C name inside a method body, or not above any declaration 49 49 left as comments
mention: says "MRI" but names no C function 407 412 not a mapping

That comes to 1130 @CRuby annotations (1126 new) on 1116 declarations in 91 files.

Changes

  • Converted only comment lines that consist of nothing but C names CRuby defines. file is given when exactly one CRuby file defines the name, and left out otherwise (the 43 notes).
  • Skipped generated files: the jay parsers and the JFlex lexers.
  • import org.jruby.anno.CRuby; added where needed.

Testing

  • Every converted name was checked against the CRuby checkout above with the upcoming checker: 1130 annotations, 0 missing, 0 in the wrong file, and 43 names defined in more than one CRuby file (these have no file).
  • ./mvnw -Pbootstrap builds; spec/ruby/core/{array,comparable,string,io} pass with the existing tags (8583 examples, 0 failures).

Not in this PR

  • The 59 + 1 + 11 comments that name functions CRuby does not define (renamed or removed upstream). They need checking one by one.
  • The 107 qualified comments and the 49 comments in method bodies.
  • The checker (tool/cruby_annotations.rb) and its CONTRIBUTING / Javadoc mentions; that will be a separate PR.
  • The conversion was done with a local script that is not committed. I can share it if anyone wants to rerun it.

🤖 Generated with Claude Code

@zev @claude
Convert MRI comments above declarations to @cruby
121d066 
Replaces the "// MRI: rb_foo" and "/** rb_foo */" comments that only name
CRuby functions with @cruby annotations, giving `file` when exactly one
CRuby file defines the name. Comment lines with more text, names CRuby
does not define, comments inside method bodies and generated parser and
lexer files are left as they are. Any rest of a Javadoc is kept.

Every converted name was checked against CRuby 4.1 (master 6203c21e58).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zev
zev force-pushed the cruby-annotation-convert branch from dc9b113 to 121d066 Compare October 15, 2026 04:34
@zev zev changed the title Convert MRI comments to @CRuby and add a checker Convert MRI comments to @CRuby Oct 15, 2026
@zev

zev commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

@headius here is my first round attempt to convert many of the comments to CRuby annotations, let me know if we are good to proceed with this or you want to have more discussions with @enebo and @kares . We can see that there are many ways of marking comments with cruby implementation functions so I think there can be value in having a consistent format going forward.

@kares

kares commented Oct 15, 2026

Copy link
Copy Markdown
Member

I don't have any feedback other than changing @CRuby annotation's retention to RetentionPolicy.SOURCE

@headius

headius commented Oct 15, 2026

Copy link
Copy Markdown
Member

Yeah I like the cleanup and consistency.

As for the retention...

I tend to err on the side of retaining always "just in case" we have any tooling that wants to use it within a running JRuby. However... we have no such tools and no plans to make them right now.

The annotations add about 17k to lib/jruby.jar.

I'm fine with making this SOURCE retention until we have a need for more.

@headius headius added this to the JRuby 10.1.3.0 milestone Oct 15, 2026
@zev
zev marked this pull request as ready for review October 15, 2026 19:28
@zev

zev commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! I've made this ready for review/merge when you are up to it.

This was referenced Oct 15, 2026
Open
Merged
headius pushed a commit that referenced this pull request Oct 17, 2026
@zev @claude
Use SOURCE retention for @cruby
a0e09fd 
Nothing reads @cruby at runtime, and RUNTIME retention adds about 17k to
lib/jruby.jar. Agreed on #9804:
#9804 (comment)
#9804 (comment)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

JRuby 10.1.3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@zev @kares @headius