Bug #12540: test failures when SHARABLE_MIDDLE_SUBSTRING=1

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.4.0dev (2016-07-02 trunk 55562) [x86_64-linux]
• Backport: 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN
• Tags: core

[znz (Kazuhiro NISHIYAMA)]

SHARABLE_MIDDLE_SUBSTRING=1 でビルドすると Test_StringCStr#test_wchar_lstrip!,Test_StringCStr#test_wchar_rstrip! が失敗します。

[16604/16757] Test_StringCStr#test_wchar_lstrip! = 0.00 s
  3) Failure:
Test_StringCStr#test_wchar_lstrip! [/home/vagrant/ruby/test/-ext-/string/test_cstr.rb:81]:
Expected {#<Encoding:UTF-16BE>=>"a",
 #<Encoding:UTF-16LE>=>"a",
 #<Encoding:UTF-32BE>=>"a",
 #<Encoding:UTF-32LE>=>"a"} to be empty.

[16606/16757] Test_StringCStr#test_wchar_rstrip! = 0.00 s
  4) Failure:
Test_StringCStr#test_wchar_rstrip! [/home/vagrant/ruby/test/-ext-/string/test_cstr.rb:85]:
Expected {#<Encoding:UTF-16BE>=>" ",
 #<Encoding:UTF-16LE>=>" ",
 #<Encoding:UTF-32BE>=>" ",
 #<Encoding:UTF-32LE>=>" "} to be empty.

原因を調べてみたところ、lstrip!,rstrip! だけ !SHARABLE_MIDDLE_SUBSTRING で TERM_FILL をくくっているからのようでした。

調べていて思ったのですが、個別に TERM_FILL を呼ぶかどうかを分岐するよりも、 SHARABLE_MIDDLE_SUBSTRING の値によって TERM_FILL 自体の定義を変更するようにした方が良いのではないでしょうか?

[jeremyevans0 (Jeremy Evans)]

I think this was caused by a707ab4bc8a29241440f56696098efa2f7f3ff45, which removed the term filling if SHARABLE_MIDDLE_SUBSTRING was enabled. I submitted a pull request that basically reverts that commit: https://github.com/ruby/ruby/pull/4731. I think the term filling can still be done safely, because the places where it was used already call str_modify_keep_cr, which should ensure they are dealing with a unique memory buffer.

• Status changed from Open to Closed

Applied in changeset git|84bf4d2ce55e16a4fb51b407a8c9c79e583596b3.

---

Term fill in String#{,l,r}strip! even when SHARABLE_MIDDLE_SUBSTRING

Each of these methods calls str_modify_keep_cr before
term filling, which should ensure the backing string
uses private memory, and therefore term filling should
not affect other strings.

Skipping the term filling was added in
a707ab4bc8a29241440f56696098efa2f7f3ff45.

Fixes [Bug #12540]