Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String format %c pretty incorrect #2369

Closed
chrisseaton opened this issue Jun 2, 2021 · 2 comments
Closed

String format %c pretty incorrect #2369

chrisseaton opened this issue Jun 2, 2021 · 2 comments

Comments

@chrisseaton
Copy link
Collaborator

puts RUBY_DESCRIPTION
p sprintf('%c', 0xf).bytes
p sprintf('%c', 0xff).bytes
p sprintf('%c', 0xffff).bytes
p sprintf('%c', 0xfffffff).bytes
ruby 2.7.3p183 (2021-04-05 revision 6847ee089d) [x86_64-darwin20]
[15]
[195, 191]
[239, 191, 191]
Traceback (most recent call last):
	1: from test.rb:5:in `<main>'
test.rb:5:in `sprintf': invalid character (ArgumentError)
truffleruby 21.2.0-dev-55ec022f, like ruby 2.7.3, GraalVM CE JVM [x86_64-darwin]
[15]
[255]
[255]
[255]

Found while working on #2368.

@eregon
Copy link
Member

eregon commented Jun 3, 2021

We also noticed this one when running https://github.com/ruby/ruby/blob/master/benchmark/app_aobench.rb (GR-13417).
PR welcome, it seems a bug in format nodes.

My notes from then:
This turns out to be a bug in Kernel#sprintf. Replacing with IO#write(byte.chr(Encoding::BINARY)) produces the correct image.

The logic there seems to mostly ignore encodings, which is obviously wrong.

There is already a failing spec showing the problem:

"Kernel#sprintf other formats c supports Unicode characters" which can be run with:

$ jt test spec/ruby/core/kernel/sprintf_spec.rb

I started to work on it but gave up because this logic is fundamentally problematic as it uses Java strings for formatting, losing encoding information, instead of using byte[] or Ruby String methods.

I push my rebased WIP at master...eregon:fix-format-c

@andrykonchin andrykonchin self-assigned this Aug 2, 2022
@andrykonchin andrykonchin added this to the 22.3.0 milestone Aug 23, 2022
@andrykonchin
Copy link
Member

Fixed in 773fd3f. Thank you for reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants