Merged
Conversation
"Enumerator#inspect produces an ASCII-8BIT encoded String" This was fixed by PR jruby#7531 "Enumerator#inspect: Use String concatenation not just bytelist appends"
Without the fix, the following sequence:
c = Class.new{define_method(:"\u{3042}"){}}
c.new.enum_for(:"\u{3042}").inspect
would give:
"#<Enumerator: #<#<Class:0x79cfc008>:0x78077f1c>:\xE3\x81\x82>"
(with ASCII_8BIT encoding)
While working on the inspect behavior, I noticed that there was a much more
serious bug: the enumerator produced by the tested sequence would not work!
c = Class.new{define_method("\u{3042}"){}}
e = c.new.enum_for("\u{3042}")
e.next # => raise StopIteration
Instead, this would produce the following error:
uri:classloader:/jruby/kernel/enumerator.rb:72:in `block in initialize\': undefined method `B\' for #<#<Class:0x4834e816>:0x3389c69d> (NoMethodError)
Converting the String to a Symbol in RubyKernel#obj_to_enum before calling
asJavaString fixes this problem.
Member
|
@cboos we have very slowly been finding places where we are not appropriately encoding m17n symbols into Strings. Mostly in error strings but you found out we also have issues with some inspect()/to_s() (in reflective sorts of things). Most people do not consume these things in a progmatic way so I think that partially explains why they have not been fixed. This is great though. Thanks for your help! |
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
2 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.
I wanted to contribute a regression test for issue #7529, as a follow-up to its fix by @enebo in #7531, but I noticed that there was already a corresponding test case in the MRI test suite.
Before the fix, with master 278aaac:
However, after the fix, with 1d97237, that test still failed!
With JRuby we got:
"#<Enumerator: #<#<Class:0x43312512>:0x1b868ef0>:?>"instead of the expected"#<Enumerator: #<#<Class:0x000002c3ad457ec8>:0x000002c3ad5dacc8>:あ>"as with CRuby 3.0, for example.Reproducing with irb what the
TestEnumerator#test_inspect_encodingtest does gives the following:Even worse, trying to use that enumerator leads to:
This pull request adds some follow-up tests and fixes for the above.