Repository navigation
Attach comments - #4255
Merged
Merged
Attach comments#4255
Conversation
kddnewton
force-pushed
the
attach-comments
branch
from
October 12, 2026 16:00
409c847 to
bbcd5b2
Compare
Revisit how we attach comments, allowing consumers to do it in C.
kddnewton
force-pushed
the
attach-comments
branch
from
October 12, 2026 16:02
bbcd5b2 to
eb3528e
Compare
Largo
added a commit
to khasinski/ironruby
that referenced
this pull request
Oct 13, 2026
CI builds libprism from prism's main branch, and since ruby/prism#4255 (attach_comments, merged 2026-10-12) every build job has failed in the smoke test with "Unknown prism node type: 0 (TypeError)" - master, Largo#17, Largo#18 and Largo#19 alike. That change altered prism's serialization in two places without moving the serialization version off 1.9, so the version check in PrismLoader could not catch it: - the options blob has a new attach_comments byte after freeze. The blob IronRuby wrote was one byte short, so prism read the scope count from the wrong offset; - the serialized header has a comment-attachment list after the comments (node id, count, comment indices). PrismLoader read its count as the magic comment count and walked off from there. PrismParser.BuildOptionsData writes the byte (0: IronRuby never asks for attachments) and PrismLoader.Load skips the list. PrismLex only reads the token stream, which ends before the header, so it needs nothing. The rest of the revision comes along so that every piece names the same prism commit, as Src/Prism/README.md asks: - Src/Prism/Generated is regenerated from cd76b0d1's config.yml: SymbolNode#value_loc became the non-optional content_loc (the bridge reads neither), and a WORDS_SEP_IMPLICIT token was added after WORDS_SEP. Ripper looks tokens up by name and skips the ones it has no event for, which is what prism's own lex_compat does with this one. - Src/StdLib/ruby/4.0/prism is prism's lib/ at cd76b0d1, generated files included; prism.rb keeps IronRuby's backend branch. parse_result/comments.rb is gone upstream - attaching comments moved into libprism. - Src/StdLib/ironruby/prism/ironruby.rb follows ffi.rb: attach_comments is a known option and is dumped after freeze, and the *_file APIs take a Pathname (to_path / to_str) the way PrismSource.with_file now does. - release.yml's PRISM_REF and the README build instructions move to cd76b0d1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4044
Closes #4062
Allows us to attach comments to the AST in C. We do this without changing the size of the Ruby objects or the C structs, by keeping two additional lists around, only when the option is passed.