Skip to content

Conversation

@ahorek
Copy link
Contributor

@ahorek ahorek commented Jan 16, 2025

fixes #42
replaces #55

@headius headius merged commit 40844c0 into jruby:master Jan 16, 2025
3 checks passed
@headius headius added this to the 1.0.60 milestone Jan 16, 2025
@headius
Copy link
Member

headius commented Jan 16, 2025

Without totally understanding the bug, this fix looks good and seems to match the C version. I also found the CRuby test that's failing on JRuby 10, which should be fixed by this:

https://github.com/jruby/jruby/actions/runs/12797824384/job/35680596698#step:7:1890

@eregon
Copy link
Member

eregon commented Mar 4, 2025

@djoooooe started the update from JCodings 1.0.58 to 1.0.61 and these MRI tests regressed, which we think might be caused by this PR:

Details
  1) Failure:
Emoji::TestDoCoMo#test_to_kddi [/b/b/e/.tmp/tmpt1vvipn2/truffleruby/test/mri/tests/ruby/enc/test_emoji.rb:88]:
Exception raised:
<#<Encoding::CompatibilityError: incompatible character encodings: ISO-2022-JP-KDDI and UTF-8>>
Backtrace:
  <internal:core> core/string.rb:892:in `each_line'
  <internal:core> core/enumerator.rb:105:in `each_with_block'
  <internal:core> core/enumerator.rb:96:in `each'
  <internal:core> core/enumerable.rb:308:in `to_a'
  <internal:core> core/string.rb:914:in `lines'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:455:in `pretty_print'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:255:in `block (2 levels) in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:282:in `nest'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:254:in `block in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:267:in `group_sub'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:253:in `group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:158:in `guard_inspect_key'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:638:in `pretty_inspect'.

  2) Failure:
Emoji::TestKDDI#test_from_sjis [/b/b/e/.tmp/tmpt1vvipn2/truffleruby/test/mri/tests/ruby/enc/test_emoji.rb:150]:
Exception raised:
<#<Encoding::CompatibilityError: incompatible character encodings: ISO-2022-JP-KDDI and UTF-8>>
Backtrace:
  <internal:core> core/string.rb:892:in `each_line'
  <internal:core> core/enumerator.rb:105:in `each_with_block'
  <internal:core> core/enumerator.rb:96:in `each'
  <internal:core> core/enumerable.rb:308:in `to_a'
  <internal:core> core/string.rb:914:in `lines'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:455:in `pretty_print'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:255:in `block (2 levels) in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:282:in `nest'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:254:in `block in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:267:in `group_sub'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:253:in `group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:158:in `guard_inspect_key'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:638:in `pretty_inspect'.

  3) Failure:
Emoji::TestKDDI#test_from_utf8 [/b/b/e/.tmp/tmpt1vvipn2/truffleruby/test/mri/tests/ruby/enc/test_emoji.rb:144]:
Exception raised:
<#<Encoding::CompatibilityError: incompatible character encodings: ISO-2022-JP-KDDI and UTF-8>>
Backtrace:
  <internal:core> core/string.rb:892:in `each_line'
  <internal:core> core/enumerator.rb:105:in `each_with_block'
  <internal:core> core/enumerator.rb:96:in `each'
  <internal:core> core/enumerable.rb:308:in `to_a'
  <internal:core> core/string.rb:914:in `lines'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:455:in `pretty_print'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:255:in `block (2 levels) in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:282:in `nest'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:254:in `block in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:267:in `group_sub'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:253:in `group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:158:in `guard_inspect_key'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:638:in `pretty_inspect'.

  4) Failure:
Emoji::TestKDDI#test_kddi [/b/b/e/.tmp/tmpt1vvipn2/truffleruby/test/mri/tests/ruby/enc/test_emoji.rb:186]:
Exception raised:
<#<Encoding::CompatibilityError: incompatible character encodings: ISO-2022-JP-KDDI and UTF-8>>
Backtrace:
  <internal:core> core/string.rb:892:in `each_line'
  <internal:core> core/enumerator.rb:105:in `each_with_block'
  <internal:core> core/enumerator.rb:96:in `each'
  <internal:core> core/enumerable.rb:308:in `to_a'
  <internal:core> core/string.rb:914:in `lines'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:455:in `pretty_print'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:255:in `block (2 levels) in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:282:in `nest'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:254:in `block in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:267:in `group_sub'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:253:in `group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:158:in `guard_inspect_key'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:638:in `pretty_inspect'.

  5) Failure:
Emoji::TestSoftBank#test_to_kddi [/b/b/e/.tmp/tmpt1vvipn2/truffleruby/test/mri/tests/ruby/enc/test_emoji.rb:310]:
Exception raised:
<#<Encoding::CompatibilityError: incompatible character encodings: ISO-2022-JP-KDDI and UTF-8>>
Backtrace:
  <internal:core> core/string.rb:892:in `each_line'
  <internal:core> core/enumerator.rb:105:in `each_with_block'
  <internal:core> core/enumerator.rb:96:in `each'
  <internal:core> core/enumerable.rb:308:in `to_a'
  <internal:core> core/string.rb:914:in `lines'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:455:in `pretty_print'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:255:in `block (2 levels) in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:282:in `nest'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:254:in `block in group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:267:in `group_sub'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/prettyprint.rb:253:in `group'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:201:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `block in pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:158:in `guard_inspect_key'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:97:in `pp'
  /b/b/e/.tmp/tmpjglqaors/graalvm/lib/mri/pp.rb:638:in `pretty_inspect'.

Those backtraces unfortunately say nothing besides the line in the test file because they are errors inside pretty_inspect.

But I reduced it to:

$ ruby -ve 'p "\x1B$B\x75\x41\x1B(B".force_encoding("ISO-2022-JP-KDDI"), "\u{E63E}".force_encoding("UTF8-DoCoMo").encode("ISO-2022-JP-KDDI")'  
ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-linux]
"\e\x24\x42\x75\x41\e\x28\x42"
"\e\x24\x42\x75\x41\e\x28\x42"

JCodings 1.0.58
$ jt -q ruby -ve 'p "\x1B$B\x75\x41\x1B(B".force_encoding("ISO-2022-JP-KDDI"), "\u{E63E}".force_encoding("UTF8-DoCoMo").encode("ISO-2022-JP-KDDI")'
truffleruby 25.0.0-dev-48a0a626, like ruby 3.3.5, Interpreted JVM [x86_64-linux]
"\x1B\x24\x42\x75\x41\x1B\x28\x42"
"\x1B\x24\x42\x75\x41\x1B\x28\x42"

JCodings 1.0.61
$ jt -q ruby -ve 'p "\x1B$B\x75\x41\x1B(B".force_encoding("ISO-2022-JP-KDDI"), "\u{E63E}".force_encoding("UTF8-DoCoMo").encode("ISO-2022-JP-KDDI")'
truffleruby 25.0.0-dev-48a0a626, like ruby 3.3.5, Interpreted JVM [x86_64-linux]
"\x1B\x24\x42\x75\x41\x1B\x28\x42"
"\x75\x41"

@headius
Copy link
Member

headius commented Mar 4, 2025

@eregon Until we can circle back to this, perhaps you or @djoooooe can figure out what differs from the equivalent C code?

@ahorek If you get a chance, we'd ideally like to solve the regression without losing the original fix...

@ahorek
Copy link
Contributor Author

ahorek commented Mar 4, 2025

Because those MRI tests are excluded on JRuby, so they didn't catch the regression. I'll see what could be wrong...

@headius
Copy link
Member

headius commented Mar 4, 2025

We haven't done a pass over excludes in a while. We'll do so as part of stabilizing 10 (probably after 10.0 release).

graalvmbot pushed a commit to truffleruby/truffleruby that referenced this pull request Mar 11, 2025
@eregon
Copy link
Member

eregon commented Mar 13, 2025

@ahorek So #67 is the fix for the issue I mentioned above, right?

@ahorek
Copy link
Contributor Author

ahorek commented Mar 13, 2025

@ahorek So #67 is the fix for the issue I mentioned above, right?

correct! Sorry for the typo. Unfortunately, those encoding tests were commented out in JRuby, even though they were mostly passing before, so we didn't catch the regression earlier. Thanks again for reporting it.

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.

ArrayIndexOutOfBoundsExceptions when transcoding UTF8-SoftBank=>SJIS-KDDI or CP51932=>CP50220

3 participants