Skip to content

Add support for signed and unsigned LEB128 to pack/unpack.#15589

Merged
tenderlove merged 1 commit intoruby:masterfrom
tenderlove:leb128
Dec 18, 2025
Merged

Add support for signed and unsigned LEB128 to pack/unpack.#15589
tenderlove merged 1 commit intoruby:masterfrom
tenderlove:leb128

Conversation

@tenderlove
Copy link
Member

@tenderlove tenderlove commented Dec 16, 2025

This commit adds a new pack format command R and r for unsigned and signed LEB128 encoding. The "r" mnemonic is because this is a "vaRiable" length encoding scheme.

LEB128 is used in various formats including DWARF, WebAssembly, MQTT, and Protobuf.

Copy link
Member

@nobu nobu left a comment

Choose a reason for hiding this comment

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

K and k cases have pretty duplicate code.

@tenderlove
Copy link
Member Author

K and k cases have pretty duplicate code.

Yes, I wasn't sure if the feature would be accepted. Matz seems positive about the feature so I will try to refactor the code a bit. 😄

@launchable-app

This comment has been minimized.

@tenderlove tenderlove force-pushed the leb128 branch 4 times, most recently from bfdf2a3 to 78f52be Compare December 17, 2025 20:29
@tenderlove
Copy link
Member Author

@nobu I reduced the duplicated code. What do you think?

@byroot I added some tests for very very large numbers to both the signed / unsigned tests. Should I add these to RubySpec too? Or move them to Ruby Spec?

@tenderlove tenderlove force-pushed the leb128 branch 6 times, most recently from 60815b2 to b37e605 Compare December 17, 2025 22:38
This commit adds a new pack format command `R` and `r` for unsigned and
signed LEB128 encoding.  The "r" mnemonic is because this is a
"vaRiable" length encoding scheme.

LEB128 is used in various formats including DWARF, WebAssembly, MQTT,
and Protobuf.

[Feature #21785]
@tenderlove tenderlove enabled auto-merge (rebase) December 18, 2025 21:55
@tenderlove tenderlove merged commit d0b7242 into ruby:master Dec 18, 2025
93 checks passed
@tenderlove tenderlove deleted the leb128 branch December 18, 2025 23:04
@eregon
Copy link
Member

eregon commented Dec 19, 2025

@byroot I added some tests for very very large numbers to both the signed / unsigned tests. Should I add these to RubySpec too? Or move them to Ruby Spec?

It would be great to have ruby/spec specs for this too, it will make it far easier to support on other Ruby implementations, and the specs for pack/unpack are in general pretty extensive.

Re add/move it's fine to add only to ruby/spec (since CRuby CI also runs that), but since you already have it in test-all then I think having it in both makes sense.
I personally think writing ruby/spec is better because it provides more structure which encourages testing all relevant cases, i.e. the full "matrix" of cases vs "just" the tests one thinks about.

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.

4 participants