Skip to content

Commit

Permalink
Fix String#casecmp? for empty strings of different encodings
Browse files Browse the repository at this point in the history
* Fixes #2826
  • Loading branch information
eregon committed Jan 12, 2023
1 parent e8875c4 commit 7a96f2c
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ Compatibility:
* Fix `Array#unshift` to not depend on `Array#[]=` and allow overriding `#[]=` in a subclass (#2772, @andrykonchin).
* Fix syntactic check for `void value expression` (#2821, @eregon).
* Fix `Range#step` with no block and non-`Numeric` values (#2824, @eregon).
* Fix `String#casecmp?` for empty strings of different encodings (#2826, @eregon).

Performance:

Expand Down
10 changes: 10 additions & 0 deletions spec/ruby/core/string/casecmp_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@
"B".casecmp(a).should == 1
end
end

it "returns 0 for empty strings in different encodings" do
''.b.casecmp('').should == 0
''.b.casecmp(''.encode("UTF-32LE")).should == 0
end
end

describe 'String#casecmp? independent of case' do
Expand Down Expand Up @@ -191,4 +196,9 @@
it "returns nil if other can't be converted to a string" do
"abc".casecmp?(mock('abc')).should be_nil
end

it "returns true for empty strings in different encodings" do
''.b.should.casecmp?('')
''.b.should.casecmp?(''.encode("UTF-32LE"))
end
end
8 changes: 8 additions & 0 deletions spec/ruby/core/symbol/casecmp_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@
lower_a_tilde.casecmp(upper_a_tilde).should == 1
lower_a_umlaut.casecmp(upper_a_umlaut).should == 1
end

it "returns 0 for empty strings in different encodings" do
''.to_sym.casecmp(''.encode("UTF-32LE").to_sym).should == 0
end
end

describe "Symbol#casecmp" do
Expand Down Expand Up @@ -141,4 +145,8 @@
upper_a_tilde.casecmp?(lower_a_tilde).should == nil
lower_a_tilde.casecmp?(upper_a_tilde).should == nil
end

it "returns true for empty symbols in different encodings" do
''.to_sym.should.casecmp?(''.encode("UTF-32LE").to_sym)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ protected Object isCompatible(Object first, Object second,
}

// encoding_compatible? but only accepting Strings for better footprint
// Like Primitive.encoding_ensure_compatible_str but returns nil if incompatible
@Primitive(name = "strings_compatible?")
public abstract static class AreStringsCompatibleNode extends PrimitiveArrayArgumentsNode {
public static AreStringsCompatibleNode create() {
Expand Down Expand Up @@ -748,6 +749,7 @@ protected int getIndex(RubyEncoding encoding) {
}

// MRI: rb_enc_check_str / rb_encoding_check (with Ruby String arguments)
// Like strings_compatible? but raises if incompatible
@Primitive(name = "encoding_ensure_compatible_str")
public abstract static class CheckStringEncodingPrimitiveNode extends PrimitiveArrayArgumentsNode {
@Specialization(guards = { "libFirst.isRubyString(first)", "libSecond.isRubyString(second)", }, limit = "1")
Expand All @@ -760,11 +762,7 @@ protected RubyEncoding checkEncodingStringString(Object first, Object second,
final RubyEncoding secondEncoding = libSecond.getEncoding(second);

final RubyEncoding negotiatedEncoding = negotiateCompatibleStringEncodingNode
.execute(
libFirst.getTString(first),
firstEncoding,
libSecond.getTString(second),
secondEncoding);
.execute(libFirst.getTString(first), firstEncoding, libSecond.getTString(second), secondEncoding);

if (negotiatedEncoding == null) {
errorProfile.enter();
Expand Down
2 changes: 1 addition & 1 deletion src/main/ruby/truffleruby/core/string.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,7 @@ def casecmp?(other)
other = Truffle::Type.rb_check_convert_type(other , String, :to_str)
return nil if Primitive.nil? other

enc = Primitive.encoding_compatible?(encoding, other.encoding)
enc = Primitive.strings_compatible?(self, other)
if Primitive.nil? enc
return nil
end
Expand Down

0 comments on commit 7a96f2c

Please sign in to comment.