Feature #22238: String#tr to take a Hash for multi-character replacements

• Status: Closed
• Assignee: byroot (Jean Boussier)
• Target version: -

[byroot (Jean Boussier)]

This is @matz's counter proposal to [Feature #22229] from the last dev-meeting

matz: I will counter-propose String#tr-with-hash style.

'Hello </script>'.tr(">" => '\u003e', "<" => '\u003c', "&" => '\u0026') # matz: OK

'Hello </script>'.tr("abc" => 'ABC') # should raise an exception
'Hello </script>'.tr("abc" => 'ABC', "ab" => "XY") # should raise an exception

"fée".tr("é" => "€") # it should work

"Hello".tr("l" => "ABC", "o" => "XYZ") #=> "HeABCABCXYZ"

"Hello".gsub(/./m) { hash[$&] || $& } # gsub equivalent

Notes:

• Hash keys MUST be single characters (but can be multi-byte).
• Values can be multiple characters.

[byroot (Jean Boussier)]

• Related to Feature #22229: Allow GCI.escapeHTML to take a custom escape table added

[nobu (Nobuyoshi Nakada)]

byroot (Jean Boussier) wrote:

• Hash keys MUST be single characters (but can be multi-byte).

I think it would be worth to clarify: "character" means "codepoint" here not including combined characters, as well as String#tr does now.

• Status changed from Open to Closed

Applied in changeset git|9144c914a1a51176d461c06e11bc78723c2c24cb.

---

Fix GC compaction re-embedding for arrays and strings

Don't re-embed after compaction for these types if they are pinned because if these objects
aren't embedded we want to keep their xmalloc's backing storage intact so that RARRAY_PTR
and RSTRING_PTR don't point to freed memory across a compaction event. If the object is pinned
we want to be able to trust that these pointers can't go stale across an allocation.

Fixes [Bug #22238]

[luke-gru (Luke Gruber)]

• Status changed from Closed to Open

The commit in the previous message has the wrong Issue ID, it is unrelated.

[sowieso (So Wieso)]

Can you explain what

"Hello".gsub(/./m) { hash[$&] || $& } # gsub equivalent

is supposed to do? I can't wrap my head around this code.

[byroot (Jean Boussier)]

It's a per character replacement:

hash = { "e" => "€", "l" => "EL", "o" => "Ø" }
str = "Hello".gsub(/./m) { hash[$&] || $& } # gsub equivalent
puts str

H€ELELØ

[sowieso (So Wieso)]

Ah, of course. Thank you! I thought hash was Kernel#hash🤦

[byroot (Jean Boussier)]

• Status changed from Open to Closed

Applied in changeset git|522f6ea22744b8ae92a50cee26785ddd5b64b7e7.

---

String#tr: take a Hash for multi-character replacements

[Feature #22238]

[matz (Yukihiro Matsumoto)]

• Status changed from Closed to Open
• Assignee set to byroot (Jean Boussier)

Reopened. I am satisfied with the spec, but the current implementation (master 3742091) has bugs.

"hello".tr("x" => "y")   #=> nil (should be "hello")
"hello".tr({})           #=> nil (should be "hello")

s = +"hello"
s.tr!("x" => "y")        #=> nil
s                        #=> "" (the receiver is emptied)

"0123456789abcdefghij".tr("h" => "H", "2" => "@")
#=> "0123456789abcdefgHij" ("2" is not replaced)

• When nothing is replaced, tr returns nil, and tr! empties the receiver. The spec for tr! only checks the return value, so it passes.
• On strings of 16 bytes or longer, the SIMD search only finds the first key. The loop that merges the matches of other keys starts with for (i = i; ...) and never runs. With more than 16 keys, the SSE2 version finds nothing.

Also:

• The call-seq and document of tr do not mention the Hash form.
• A broken string raises ArgumentError in non-fastpath encodings, but not in UTF-8/US-ASCII/BINARY. It should be consistent (the 2-argument form raises).

Please add tests for these cases as well.

Matz.

[byroot (Jean Boussier)]

• Status changed from Open to Closed

Applied in changeset git|0adfd0ec77e4acda94b7ec3643b3998c621f71df.

---

string.c: Fix String#tr edge cases and improve coverage

[Feature #22238]

"hello".tr("x" => "y")   #=> nil (should be "hello")
"hello".tr({})           #=> nil (should be "hello")

s = +"hello"
s.tr!("x" => "y")        #=> nil
s                        #=> "" (the receiver is emptied)

"0123456789abcdefghij".tr("h" => "H", "2" => "@")

[byroot (Jean Boussier)]

Thanks for catching that. I believe I addressed all points.