[Ironruby-core] Review: misc fixes to MutableStringOps.cs

Daniele Alessandri suppakilla at gmail.com
Thu Jul 16 16:22:33 EDT 2009


Hi,
I pushed a bunch of fixes to my repository. In the end I had to hold
back a couple of more commits staged in a local branch because I'm
still not convinced about them, I'll push them in the next few days
for a future review.

http://github.com/nrk/ironruby/commit/74c47076ab82f357e58ca604331701cae40a9b6a

>From the commit message:

- Implemented 'V' and 'v' directives for String#unpack.
- Implemented 'B' and 'b' directives for String#unpack.
- Cleared all the failing expectations in core/string related to an
unthrown TypeError exception when calling in-place methods on frozen
strings.

The absence of 'V', 'v', 'B' and 'b' directives in String#unpack was
preventing me to use the wmainfo-rb gem.

See also the attached diff.

Thanks,
Daniele

-- 
Daniele Alessandri
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN
-------------- next part --------------
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/capitalize_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/capitalize_tags.txt
deleted file mode 100644
index c2f8309..0000000
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/capitalize_tags.txt
+++ /dev/null
@@ -1 +0,0 @@
-fails:String#capitalize! raises a TypeError when self is frozen
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/delete_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/delete_tags.txt
deleted file mode 100644
index 1e7d568..0000000
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/delete_tags.txt
+++ /dev/null
@@ -1 +0,0 @@
-fails:String#delete! raises a TypeError when self is frozen
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/downcase_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/downcase_tags.txt
deleted file mode 100644
index b16d1d8..0000000
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/downcase_tags.txt
+++ /dev/null
@@ -1 +0,0 @@
-fails:String#downcase! raises a TypeError when self is frozen
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/next_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/next_tags.txt
deleted file mode 100644
index 1f3bca2..0000000
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/next_tags.txt
+++ /dev/null
@@ -1 +0,0 @@
-fails:String#next! raises a TypeError if self is frozen
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/split_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/split_tags.txt
deleted file mode 100644
index e7bf2f6..0000000
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/split_tags.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-fails:String#split with Regexp splits between characters when regexp matches a zero-length string
-fails:String#split with Regexp includes all captures in the result array
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/succ_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/succ_tags.txt
deleted file mode 100644
index d10eac2..0000000
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/succ_tags.txt
+++ /dev/null
@@ -1 +0,0 @@
-fails:String#succ! raises a TypeError if self is frozen
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/swapcase_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/swapcase_tags.txt
deleted file mode 100644
index 729201b..0000000
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/swapcase_tags.txt
+++ /dev/null
@@ -1 +0,0 @@
-fails:String#swapcase! raises a TypeError when self is frozen
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/unpack_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/unpack_tags.txt
index 2eef04f..d0ab5e9 100644
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/unpack_tags.txt
+++ b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/unpack_tags.txt
@@ -1,16 +1,14 @@
 fails:String#unpack returns an array by decoding self according to the format string
 fails:String#unpack with 'Z' directive returns an array by decoding self according to the format string
 fails:String#unpack with 'N' and 'n' directives returns an array by decoding self according to the format string
-fails:String#unpack with 'V' and 'v' directives returns an array by decoding self according to the format string
 fails:String#unpack with 'Q' and 'q' directives returns an array in little-endian (native format) order by decoding self according to the format string
 fails:String#unpack with 'Q' and 'q' directives returns Bignums for big numeric values on little-endian platforms
 fails:String#unpack with 'Q' and 'q' directives returns Fixnums for small numeric values
 fails:String#unpack with 'a', 'X' and 'x' directives returns an array by decoding self according to the format string
 fails:String#unpack with 'DdEeFfGg' directives returns an array by decoding self according to the format string
 fails:String#unpack with 'DdEeFfGg' directives returns an array by decoding self in little-endian order according to the format string
-fails:String#unpack with 'B' and 'b' directives returns an array by decoding self according to the format string
 fails:String#unpack with 'U' directive returns an array by decoding self according to the format string
 fails:String#unpack with '@' directive returns an array by decoding self according to the format string
 fails:String#unpack with 'M' directive returns an array by decoding self according to the format string
 fails:String#unpack with 'm' directive returns an array by decoding self according to the format string
 fails:String#unpack with 'w' directive produces a BER-compressed integer
diff --git a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/upcase_tags.txt b/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/upcase_tags.txt
deleted file mode 100644
index 93de75c..0000000
--- a/Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/string/upcase_tags.txt
+++ /dev/null
@@ -1 +0,0 @@
-fails:String#upcase! raises a TypeError when self is frozen
diff --git a/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/MutableStringOps.cs b/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/MutableStringOps.cs
index 3878e0f..31555e5 100644
--- a/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/MutableStringOps.cs
+++ b/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/MutableStringOps.cs
@@ -54,30 +54,36 @@ namespace IronRuby.Builtins {
         }
 
         private static bool InExclusiveRangeNormalized(MutableString/*!*/ str, ref int index) {
             if (index < 0) {
                 index = index + str.Length;
             }
             return index >= 0 && index < str.Length;
         }
 
         private static bool InInclusiveRangeNormalized(MutableString/*!*/ str, ref int index) {
             if (index < 0) {
                 index = index + str.Length;
             }
             return index >= 0 && index <= str.Length;
         }
+        
+        private static void RequiresNotFrozenString(MutableString/*!*/ self) {
+            if (self.IsFrozen) {
+                throw RubyExceptions.CreateTypeError("can't modify frozen string");
+            }
+        }
 
         // Parses interval strings that are of this form:
         //
         // abc         # abc
         // abc-efg-h   # abcdefgh
         // ^abc        # all characters in range 0-255 except abc
         // \x00-0xDD   # all characters in hex range
         public class IntervalParser {
             private readonly MutableString/*!*/ _range;
             private int _pos;
             private bool _rangeStarted;
             private int _startRange;
 
             public IntervalParser(MutableString/*!*/ range) {
                 _range = range;
@@ -838,66 +844,70 @@ namespace IronRuby.Builtins {
 
         [RubyMethod("casecmp")]
         public static int Casecmp(MutableString/*!*/ self, [DefaultProtocol, NotNull]MutableString/*!*/ other) {
             return Compare(DownCase(self), DownCase(other));
         }
 
         [RubyMethod("capitalize")]
         public static MutableString/*!*/ Capitalize(MutableString/*!*/ self) {
             MutableString result = self.Clone();
             CapitalizeMutableString(result);
             return result;
         }
 
         [RubyMethod("capitalize!")]
         public static MutableString CapitalizeInPlace(MutableString/*!*/ self) {
+            RequiresNotFrozenString(self);
             return CapitalizeMutableString(self) ? self : null;
         }
 
         [RubyMethod("downcase")]
         public static MutableString/*!*/ DownCase(MutableString/*!*/ self) {
             MutableString result = self.Clone();
             DownCaseMutableString(result);
             return result;
         }
 
         [RubyMethod("downcase!")]
         public static MutableString DownCaseInPlace(MutableString/*!*/ self) {
+            RequiresNotFrozenString(self);
             return DownCaseMutableString(self) ? self : null;
         }
 
         [RubyMethod("swapcase")]
         public static MutableString/*!*/ SwapCase(MutableString/*!*/ self) {
             MutableString result = self.Clone();
             SwapCaseMutableString(result);
             return result;
         }
 
         [RubyMethod("swapcase!")]
         public static MutableString SwapCaseInPlace(MutableString/*!*/ self) {
+            RequiresNotFrozenString(self);
             return SwapCaseMutableString(self) ? self : null;
         }
 
         [RubyMethod("upcase")]
         public static MutableString/*!*/ UpCase(MutableString/*!*/ self) {
             MutableString result = self.Clone();
             UpCaseMutableString(result);
             return result;
         }
 
         [RubyMethod("upcase!")]
         public static MutableString UpCaseInPlace(MutableString/*!*/ self) {
+            RequiresNotFrozenString(self);
             return UpCaseMutableString(self) ? self : null;
         }
 
         #endregion
 
 
         #region center
 
         private static readonly MutableString _DefaultPadding = MutableString.Create(" ").Freeze();
 
         [RubyMethod("center")]
         public static MutableString/*!*/ Center(MutableString/*!*/ self, 
             [DefaultProtocol]int length,
             [Optional, DefaultProtocol]MutableString padding) {
 
@@ -1736,30 +1746,32 @@ namespace IronRuby.Builtins {
             return self;
         }
 
         [RubyMethod("delete")]
         public static MutableString/*!*/ Delete(RubyContext/*!*/ context, MutableString/*!*/ self, 
             [DefaultProtocol, NotNull, NotNullItems]params MutableString/*!*/[]/*!*/ strs) {
             if (strs.Length == 0) {
                 throw RubyExceptions.CreateArgumentError("wrong number of arguments");
             }
             return InternalDelete(self, strs);
         }
 
         [RubyMethod("delete!")]
         public static MutableString/*!*/ DeleteInPlace(RubyContext/*!*/ context, MutableString/*!*/ self,
             [DefaultProtocol, NotNull, NotNullItems]params MutableString/*!*/[]/*!*/ strs) {
+            RequiresNotFrozenString(self);
+
             if (strs.Length == 0) {
                 throw RubyExceptions.CreateArgumentError("wrong number of arguments");
             }
             return InternalDeleteInPlace(self, strs);
         }
 
         #endregion
 
         #region count
         
         private static object InternalCount(MutableString/*!*/ self, MutableString[]/*!*/ ranges) {
             BitArray map = new RangeParser(ranges).Parse();
             int count = 0;
             for (int i = 0; i < self.Length; i++) {
                 if (map.Get(self.GetChar(i)))
@@ -1956,30 +1968,32 @@ namespace IronRuby.Builtins {
             if (c == 255) {
                 str.SetByte(index, 0);
                 if (index > 0) {
                     IncrementChar(str, index - 1);
                 } else {
                     str.Insert(0, 1);
                 }
             } else {
                 str.SetByte(index, unchecked((byte)(c + 1)));
             }
         }
 
         [RubyMethod("succ!")]
         [RubyMethod("next!")]
         public static MutableString/*!*/ SuccInPlace(MutableString/*!*/ self) {
+            RequiresNotFrozenString(self);
+
             if (self.IsEmpty) {
                 return self;
             }
 
             int index = GetIndexOfRightmostAlphaNumericCharacter(self, self.Length - 1);
             if (index == -1) {
                 IncrementChar(self, self.Length - 1);
             } else {
                 IncrementAlphaNumericChar(self, index);
             }
 
             return self;
         }
 
         [RubyMethod("succ")]
@@ -2391,30 +2405,35 @@ namespace IronRuby.Builtins {
             self.Append(other);
             return self.TaintBy(other);
         }
 
         [RubyMethod("reverse")]
         public static MutableString/*!*/ GetReversed(MutableString/*!*/ self) {
             return self.Clone().Reverse();
         }
 
         [RubyMethod("reverse!")]
         public static MutableString/*!*/ Reverse(MutableString/*!*/ self) {
             if (self.Encoding.IsKCoding) {
                 throw new NotImplementedException("TODO: KCODE");
             }
 
+            if (self.Length == 0) {
+                return self;
+            }
+            RequiresNotFrozenString(self);
+
             // TODO: MRI 1.9: allows invalid characters
             return self.Reverse();
         }
 
         #endregion
 
 
         #region tr, tr_s
 
         internal static MutableString/*!*/ Translate(MutableString/*!*/ src, MutableString/*!*/ from, MutableString/*!*/ to, 
             bool inplace, bool squeeze, out bool anyCharacterMaps) {
             Assert.NotNull(src, from, to);
 
             if (from.IsEmpty) {
                 anyCharacterMaps = false;
@@ -2448,49 +2467,51 @@ namespace IronRuby.Builtins {
         }
 
         // encoding aware, TODO: KCODE
         [RubyMethod("tr")]
         public static MutableString/*!*/ GetTranslated(MutableString/*!*/ self,
             [DefaultProtocol, NotNull]MutableString/*!*/ from, [DefaultProtocol, NotNull]MutableString/*!*/ to) {
             bool _;
             return Translate(self, from, to, false, false, out _);
         }
 
         // encoding aware, TODO: KCODE
         [RubyMethod("tr!")]
         public static MutableString Translate(MutableString/*!*/ self,
             [DefaultProtocol, NotNull]MutableString/*!*/ from, [DefaultProtocol, NotNull]MutableString/*!*/ to) {
 
+            RequiresNotFrozenString(self);
             bool anyCharacterMaps;
             self.RequireNotFrozen();
             Translate(self, from, to, true, false, out anyCharacterMaps);
             return anyCharacterMaps ? self : null;
         }
 
         // encoding aware, TODO: KCODE
         [RubyMethod("tr_s")]
         public static MutableString/*!*/ TrSqueeze(MutableString/*!*/ self,
             [DefaultProtocol, NotNull]MutableString/*!*/ from, [DefaultProtocol, NotNull]MutableString/*!*/ to) {
             bool _;
             return Translate(self, from, to, false, true, out _);
         }
 
         // encoding aware, TODO: KCODE
         [RubyMethod("tr_s!")]
         public static MutableString TrSqueezeInPlace(MutableString/*!*/ self,
             [DefaultProtocol, NotNull]MutableString/*!*/ from, [DefaultProtocol, NotNull]MutableString/*!*/ to) {
 
+            RequiresNotFrozenString(self);
             bool anyCharacterMaps;
             self.RequireNotFrozen();
             Translate(self, from, to, true, true, out anyCharacterMaps);
             return anyCharacterMaps ? self : null;
         }
 
         #endregion
 
         
         #region ljust
 
         [RubyMethod("ljust")]
         public static MutableString/*!*/ LeftJustify(MutableString/*!*/ self, [DefaultProtocol]int width) {
             // TODO: is this correct? Is it just a space or is this some configurable whitespace thing?
             return LeftJustify(self, width, _DefaultPadding);
@@ -2612,30 +2633,65 @@ namespace IronRuby.Builtins {
                             }
                             buffer = reader.ReadBytes(count);
                             str = MutableString.CreateBinary(buffer);
                             if (directive.Directive == 'A') {
                                 // TODO: efficiency?
                                 for (int pos = count - 1; pos >= 0; pos--) {
                                     if (buffer[pos] != 0 && buffer[pos] != 0x20) {
                                         break;
                                     }
                                     str.Remove(pos, 1);
                                 }
                             }
                             result.Add(str);
                             break;
 
+                        case 'B':
+                        case 'b':
+                            if (stream.Length - stream.Position != 0) {
+                                count = directive.Count.HasValue ? directive.Count.Value : (int)(stream.Length - stream.Position) * 8;
+                                buffer = reader.ReadBytes((int)Math.Ceiling((double)count / 8));
+                                if (count > buffer.Length * 8) {
+                                    count = buffer.Length * 8;
+                                }
+                                str = MutableString.CreateBinary(count);
+
+                                if ((directive.Directive == 'B' && BitConverter.IsLittleEndian) || (directive.Directive == 'b' && !BitConverter.IsLittleEndian)) {
+                                    for (int i = 0; i < buffer.Length; i++) {
+                                        byte b = buffer[i];
+                                        int r = (b >> 4) | ((b & 0x0F) << 4);
+                                        r = ((r & 0xCC) >> 2) | ((r & 0x33) << 2);
+                                        r = ((r & 0xAA) >> 1) | ((r & 0x55) << 1);
+                                        buffer[i] = (byte)r;
+                                    }
+                                }
+
+                                for (int b = 0, i = 0; b < count; b++) {
+                                    if (b == 8) {
+                                        i++;
+                                        b = 0;
+                                        count -= 8;
+                                    }
+                                    str.Append(((buffer[i] & (1 << b)) != 0 ? '1' : '0'));
+                                }
+                            }
+                            else {
+                                str = MutableString.CreateEmpty();
+                            }
+                            result.Add(str);
+                            break;
+
                         case 'Z':
                             maxCount = (int)(stream.Length - stream.Position);
                             count = directive.Count.HasValue ? directive.Count.Value : maxCount;
                             if (count > maxCount) {
                                 count = maxCount;
                             }
                             buffer = reader.ReadBytes(count);
                             str = MutableString.CreateBinary(buffer);
                             for (int pos = 0; pos < count; pos++) {
                                 if (buffer[pos] == 0) {
                                     str.Remove(pos, count - pos);
                                     break;
                                 }
                             }
                             result.Add(str);
@@ -2664,30 +2720,61 @@ namespace IronRuby.Builtins {
                             break;
 
                         case 'I':
                         case 'L':
                             count = CalculateCounts(stream, directive.Count, sizeof(uint), out nilCount);
                             for (int j = 0; j < count; j++) {
                                 uint value = reader.ReadUInt32();
                                 if (value <= Int32.MaxValue) {
                                     result.Add((int)value);
                                 } else {
                                     result.Add((BigInteger)value);
                                 }
                             }
                             break;
 
+                        case 'v':
+                            count = CalculateCounts(stream, directive.Count, sizeof(ushort), out nilCount);
+                            for (int j = 0; j < count; j++) {
+                                ushort value = reader.ReadUInt16();
+                                if (!BitConverter.IsLittleEndian) {
+                                    value = (ushort)(0x00FF & (value >> 8) |
+                                                     0xFF00 & (value << 8));
+                                }
+                                result.Add((int)value);
+                            }
+                            break;
+
+                        case 'V':
+                            count = CalculateCounts(stream, directive.Count, sizeof(uint), out nilCount);
+                            for (int j = 0; j < count; j++) {
+                                uint value = reader.ReadUInt32();
+                                if (!BitConverter.IsLittleEndian) {
+                                    value = (0x000000FF & (value >> 24) |
+                                             0x0000FF00 & (value >> 8) |
+                                             0x00FF0000 & (value << 8) |
+                                             0xFF000000 & (value << 24));
+                                }
+                                if (value <= Int32.MaxValue) {
+                                    result.Add((int)value);
+                                }
+                                else {
+                                    result.Add((BigInteger)value);
+                                }
+                            }
+                            break;
+
                         case 'm':
                             // TODO: Recognize "==" as end of base 64 encoding
                             int len = (int)(stream.Length - stream.Position);
                             char[] base64 = reader.ReadChars(len);
                             byte[] data = Convert.FromBase64CharArray(base64, 0, len);
                             result.Add(MutableString.CreateBinary(data));
                             break;
 
                         case 's':
                             count = CalculateCounts(stream, directive.Count, sizeof(short), out nilCount);
                             for (int j = 0; j < count; j++) {
                                 result.Add((int)reader.ReadInt16());
                             }
                             break;
 


More information about the Ironruby-core mailing list