-
-
Notifications
You must be signed in to change notification settings - Fork 394
Add specs for rb_interned_str and rb_interned_str_cstr #1327
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
base: master
Are you sure you want to change the base?
Conversation
|
Upstream: https://bugs.ruby-lang.org/issues/21842 |
Updated them to the current status of the upstream bug report, which is all strings are binary. This might change to either ASCII-7BIT or BINARY. The current specs pass with the latest upstream version of Ruby 4.1 (commit 3e13b7d4ef)
|
Updated the specs to match the behaviour of ruby/ruby#15888. This might not be the final version, ruby/ruby#15894 has a proposed change to return US-ASCII if everything is ASCII compatible, and BINARY otherwise. |
|
Updated again to match the new behaviour of ruby/ruby#15894 |
It the whole string is valid ASCII, it returns US-ASCII encoding. Otherwise, it is BINARY. The current specs pass with the latest upstream version of Ruby 4.1 (commit 78b7646bdb)
0912e58 to
f737668
Compare
| end | ||
| end | ||
|
|
||
| it "returns the same frozen strings for different encodings" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description/test is confusing because the encoding is ignored by that function, it only receives char* and length.
| end | ||
| end | ||
|
|
||
| it "returns the same frozen strings for different encodings" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
eregon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, just 2 comments
Keeping this as a draft for now. The comment in the MRI source for
rb_interned_strreads as follows:But the encoding of the result is always
Encoding::US_ASCII, which leads to the following spec:I will create an issue for Ruby to get clarification about the desired behaviour.