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.

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.


Ok, cool. Sounds good.

Are you thinking of a method in MutableString like IncrementVersion()?
Or something?

The version updates would be internal detail of MutableString. You would
be able to ask for the current version from other classes, but not
change it. The operations on MutableString will do.