[pull] master from ruby:master - #1407
Merged
Merged
Conversation
It's failing compilation on `rmv7a-linux-androideabi30-clang`:
```
escape.c:223:34: error: call to undeclared function 'vqtbl1q_u8'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
223 | const uint8x16_t looked_up = vqtbl1q_u8(escape_char_by_low_nibble, low_nibbles);
| ^
escape.c:223:34: note: did you mean 'vtbl1_u8'?
/home/chkbuild/opt/android-ndk-r29/toolchains/llvm/prebuilt/linux-x86_64/lib/clang/21/include/arm_neon.h:33478:48: note: 'vtbl1_u8' declared here
33478 | __ai __attribute__((target("neon"))) uint8x8_t vtbl1_u8(uint8x8_t __p0, uint8x8_t __p1) {
| ^
escape.c:223:22: error: initializing 'const uint8x16_t' (vector of 16 'uint8_t' values) with an expression of incompatible type 'int'
223 | const uint8x16_t looked_up = vqtbl1q_u8(escape_char_by_low_nibble, low_nibbles);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
escape.c:236:14: error: call to undeclared function 'vpaddq_u8'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
236 | folded = vpaddq_u8(folded, folded);
| ^
escape.c:236:12: error: assigning to 'uint8x16_t' (vector of 16 'uint8_t' values) from incompatible type 'int'
236 | folded = vpaddq_u8(folded, folded);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
escape.c:237:12: error: assigning to 'uint8x16_t' (vector of 16 'uint8_t' values) from incompatible type 'int'
237 | folded = vpaddq_u8(folded, folded);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
escape.c:238:12: error: assigning to 'uint8x16_t' (vector of 16 'uint8_t' values) from incompatible type 'int'
238 | folded = vpaddq_u8(folded, folded);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
escape.c:256:25: error: call to undeclared function 'vpaddq_u8'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
256 | uint8x16_t folded = vpaddq_u8(vpaddq_u8(t0, t1), vpaddq_u8(t2, t3));
| ^
escape.c:256:16: error: initializing 'uint8x16_t' (vector of 16 'uint8_t' values) with an expression of incompatible type 'int'
256 | uint8x16_t folded = vpaddq_u8(vpaddq_u8(t0, t1), vpaddq_u8(t2, t3));
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
escape.c:257:12: error: assigning to 'uint8x16_t' (vector of 16 'uint8_t' values) from incompatible type 'int'
257 | folded = vpaddq_u8(folded, folded);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
```
ruby/erb@45b481473c
(ruby/stringio#225) `Arrays.fill(byte[], int, int, byte)` takes **fromIndex, toIndex**, but `ungetbyteCommon` passes `memset`'s **destination, length**: ```java if (rest > cl) Arrays.fill(strBytes, len, rest - cl, (byte) 0); ``` straight from `ext/stringio/stringio.c:1181`, `memset(s + len, 0, rest - cl)`. It also drops the ByteList `begin` offset that the C `s +` supplies. Pushing back onto a StringIO positioned more than the pushback length past the end of its string then throws instead of zero-filling: ```ruby s = StringIO.new("abc"); s.pos = 5; s.ungetbyte("d") # JRuby: Java::JavaLang::IllegalArgumentException: fromIndex(3) > toIndex(1) # CRuby: s.string == "abc\0d", s.pos == 4 ``` The two siblings in this file already spell the correct form — `truncate` (`StringIO.java:1462`) uses `buf.getBegin() + plen, buf.getBegin() + l`, and `strioExtend` (`:1044-1048`) uses `begin + olen, begin + pos`. This one line is the odd one out, so the fix reuses their shape rather than adding anything. The added expectations extend the existing `test_ungetc_padding` / `test_ungetbyte_padding`, which stop one short: both start from `StringIO.new()`, so `len == 0` makes the wrong call accidentally correct. <details> <summary>Verification</summary> Both backends built from this clone, `$LOADED_FEATURES` printed and checked on every row. A differential sweep of 2100 cases (7 base strings x 25 positions x 6 pushback values x `ungetbyte`/`ungetc`), JRuby against the C extension: | build | rows differing from CRuby | |---|---| | current `master` | **132** | | with the fix | **0** | Every one of the 132 is the `IllegalArgumentException`. | row | result | |---|---| | the two padding tests on `master` | 2 errors | | with the fix | 2 tests, 12 assertions, 0 failures, 0 errors | | revert | 2 errors | | widen the guard to `rest > 0` | 1 error | | `fromIndex` one low | 1 failure, 1 error | | `toIndex` one high | passes — see below | That last mutant survives, and I checked whether it was a gap in my test or genuinely equivalent: the extra byte lands at `s + pos` after `pos -= cl`, which `System.arraycopy(..., strBytes, s + pos, cl)` two lines later overwrites unconditionally. Sweeping it over the same 2100 cases gives **0 differences from the fixed build**, so it is an equivalent mutant rather than an uncovered line, and I did not add a row for it. Full `test/stringio/test_stringio.rb`: CRuby 3.2 **104 tests, 646 assertions, 0 failures, 0 errors**; JRuby 10.0.6 **104 tests, 643 assertions, 0 failures, 0 errors**. Every added expectation was run against the C extension first, so it encodes CRuby behaviour rather than my reading of it. Two things I want on the record. The `s +` half is not separately covered — I could not construct a StringIO whose backing ByteList has `begin != 0` through the public API. It is included because both siblings and the C both require it; if you would rather keep the change minimal, the two-index correction alone fixes the crash. And `ruby/.github`'s SECURITY.md routes vulnerabilities privately: this is the JVM's own bounds check firing rather than memory unsafety, and I could not produce any stale-byte disclosure, so I filed publicly — say the word if you would rather it had gone the other route. The CI matrix runs `jruby-head`; the closest I could obtain is JRuby 10.0.6. Note `jruby:9.4` cannot compile `ext/java` at master at all (`Helpers.memchr` signature mismatch), so 10.x is already the floor. </details> --- Disclosure: I used Claude (an AI assistant) while preparing this change. Every result above I ran and verified myself. ruby/stringio@9d3e502b3a
This commit changes Enuemrator::Lazy#uniq to use a temporary Set instead
of Hash for performance and memory savings. We can see in the following
benchmark:
a = ((0...10_000).to_a + (0...10_000).to_a).lazy
1_000.times { a.uniq.to_a }
Results:
Benchmark 1: master
Time (mean ± σ): 867.5 ms ± 13.6 ms [User: 857.1 ms, System: 7.8 ms]
Range (min … max): 853.9 ms … 899.4 ms 10 runs
Benchmark 2: branch
Time (mean ± σ): 801.4 ms ± 7.1 ms [User: 791.3 ms, System: 7.4 ms]
Range (min … max): 789.4 ms … 810.7 ms 10 runs
Summary
branch ran
1.08 ± 0.02 times faster than master
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )