Project

General

Profile

Actions

Bug #12540

closed

test failures when SHARABLE_MIDDLE_SUBSTRING=1

Added by znz (Kazuhiro NISHIYAMA) over 7 years ago. Updated over 2 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.4.0dev (2016-07-02 trunk 55562) [x86_64-linux]
[ruby-dev:49704]
Tags:

Description

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 自体の定義を変更するようにした方が良いのではないでしょうか?

Updated by jeremyevans0 (Jeremy Evans) over 2 years ago

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.

Actions #2

Updated by jeremyevans (Jeremy Evans) over 2 years ago

  • 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]

Actions

Also available in: Atom PDF

Like0
Like0Like0