[Ironruby-core] Code Review: StringGsubVersion

Tomas Matousek Tomas.Matousek at microsoft.com
Thu Jul 10 21:46:24 EDT 2008


I think it would be better to increment the version inside MutableString class (not in -Ops). I can do that transition when I'll revisit frozen/tainted flags on MutableString. So go ahead with your change.

Tomas

-----Original Message-----
From: Dave Remy
Sent: Thursday, July 10, 2008 6:21 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: Code Review: StringGsubVersion

tfpt review "/shelveset:StringGsubVersion;REDMOND\dremy"
Comment  :
  This change fixes all of the gsub, each, and each_line specs.  Added aditional tests to gsub spec.  The excluded gsub tests are only failing on unimplemented string methods (lstrip!, rstrip!)  Implemented simple mutablestring version that is used to assure that mutation hasn't occurred during an iteration.

  tring#gsub with pattern and block sets $~ for access from the block
  String#gsub with pattern and block restores $~ after leaving the block
  String#gsub with pattern and block sets $~ to MatchData of last match and nil when there's none for access from outside
  String#gsub with pattern and block raises a RuntimeError if the string is modified while substituting

This is a revision on a previous shelfset I submitted for these changes.  Based on feedback I did a few things differently.  I didn't do anything special with version it is just a simple property implementation at this point.  Shri's points about potential multi-threaded issues and overflow are well taken but it sounds like we will pick this up as requirements dictate.  I did revise the way version gets incremented.  Rather than creating a single function that both requires frozen and increments (didn't someone say sometime that function should only do one thing ;))  I think this was a good thing, I ended up putting the Version++ in the logical place close to where a mutation actually occurs.  This combined with putting tests for each mutation method caught some scenarios that would have failed.


More information about the Ironruby-core mailing list