Ironruby core - Apr 2009 - [patch] More fixes for Array (IListOps.cs)

Hi,

I''ve committed more fixes for a few methods of the Array class that
were failing to pass the specs:

http://github.com/nrk/ironruby/commit/b754ddcd3bdd2c4513e456c9cf3933772e8541f9

These are the specs affected by this patch:

* core/array/compact
  - Array#compact keeps tainted status even if all elements are removed
* core/array/delete
  - Array#delete may be given a block that is executed if no element
matches object
* core/array/first
  - Array#first does not return subclass instance when passed count on
Array subclasses
* core/array/last
  - Array#last does not return subclass instance on Array subclasses
* core/array/flatten
  - Array#flatten returns subclass instance for Array subclasses
  - Array#flatten does not call flatten on elements
* core/array/fetch
  - Array#fetch passes the original index argument object to the
block, not the converted Integer

I hope I haven''t forgotten anything important this time, changes to
the tags files are included in the commit :-)

Regards,
Daniele

-- 
Daniele Alessandri
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN

On Thu, Apr 16, 2009 at 01:14, Shri Borde <Shri.Borde at microsoft.com>
wrote:
> In IListOps.Fetch, instead of doing the protocol conversion manually, you
> should calls Protocols. ConvertToInteger. Tomas, can you double-check this?
Hmm, right, and the code looks much more cleaner too.
I think I should just use Protocols.CastToFixnum then: we don''t really
care about bignums here, they can''t be used as indexes for arrays
anyway and in fact MRI throws a RangeError "bignum too big to convert
into `long''".
> In IListOps.First, you can get rid of the allocateStorage argument as its
> not being used anymore. (You can batch this fix with your next commit)
Same for IListOps.Last, it just slipped away. I removed allocateStorage on both.
> Btw, could you please increase the diff context size to 15-20 so more of
the
> surrounding lines are included? That makes it easier to understand what the
> fix is doing when using the url below, without having to map that back to
> the correct line number in the actual file.
I guess there is no way to increase the lines range of a diff viewed
via the github web interface, but I can generate and attach custom
diff patches to my mails if you think it could be useful, just like
the one attached to this mail and generated with "git diff HEAD~1
-U15
> patch.diff" (PS: I have not yet pushed this commit on my remote
repository).

-- 
Daniele Alessandri
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN
-------------- next part --------------
diff --git
a/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Extensions/IListOps.cs
b/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Extensions/IListOps.cs
index f6a6fd8..955836e 100644
--- a/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Extensions/IListOps.cs
+++ b/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Extensions/IListOps.cs
@@ -723,51 +723,45 @@ namespace IronRuby.Builtins {
                 if (block.Yield(i, out result)) {
                     return result;
                 }
                 i++;
             }
 
             return self;
         }
 
         #endregion
 
         #region fetch
 
         [RubyMethod("fetch")]
         public static object Fetch(
-            RespondToStorage/*!*/ respondToStorage, 
-            CallSiteStorage<Func<CallSite, object, int>>/*!*/
toIntStorage,
+            ConversionStorage<int>/*!*/ fixnumCast, 
             BlockParam outOfRangeValueProvider, 
             IList/*!*/ list, 
             object/*!*/ index, 
             [Optional]object defaultValue) {
 
-            if (!Protocols.RespondTo(respondToStorage, index,
"to_int")) {
-                throw
RubyExceptions.CannotConvertTypeToTargetType(respondToStorage.Context, index,
"Integer");
-            }
-
-            var toInt = toIntStorage.GetCallSite("to_int", 0);
-            int convertedIndex = toInt.Target(toInt, index);
+            int convertedIndex = Protocols.CastToFixnum(fixnumCast, index);
 
             if (InRangeNormalized(list, ref convertedIndex)) {
                 return list[convertedIndex];
             }
 
             if (outOfRangeValueProvider != null) {
                 if (defaultValue != Missing.Value) {
-                    respondToStorage.Context.ReportWarning("block
supersedes default value argument");
+                    fixnumCast.Context.ReportWarning("block supersedes
default value argument");
                 }
 
                 object result;
                 outOfRangeValueProvider.Yield(index, out result);
                 return result;
             }
 
             if (defaultValue == Missing.Value) {
                 throw RubyExceptions.CreateIndexError("index " +
convertedIndex + " out of array");
             }
             return defaultValue;
         }
 
         #endregion
 
@@ -875,48 +869,46 @@ namespace IronRuby.Builtins {
             int length = Math.Max(0, end - begin + (range.ExcludeEnd ? 0 : 1));
 
             return Fill(fixnumCast.Context, block, self, begin, length);
         }
 
         #endregion
 
         #region first, last
 
         [RubyMethod("first")]
         public static object First(IList/*!*/ self) {
             return self.Count == 0 ? null : self[0];
         }
 
         [RubyMethod("first")]
-        public static IList/*!*/ First(CallSiteStorage<Func<CallSite,
RubyClass, object>>/*!*/ allocateStorage,
-            IList/*!*/ self, [DefaultProtocol]int count) {
+        public static IList/*!*/ First(IList/*!*/ self, [DefaultProtocol]int
count) {
             if (count < 0) {
                 throw RubyExceptions.CreateArgumentError("negative array
size (or size too big)");
             }
 
             count = count > self.Count ? self.Count : count;
             return RubyArray.Create(self as IList<object>, 0, count);
         }
 
         [RubyMethod("last")]
         public static object Last(IList/*!*/ self) {
             return self.Count == 0 ? null : self[self.Count - 1];
         }
 
         [RubyMethod("last")]
-        public static IList/*!*/ Last(CallSiteStorage<Func<CallSite,
RubyClass, object>>/*!*/ allocateStorage,
-            IList/*!*/ self, [DefaultProtocol]int count) {
+        public static IList/*!*/ Last(IList/*!*/ self, [DefaultProtocol]int
count) {
             if (count < 0) {
                 throw RubyExceptions.CreateArgumentError("negative array
size (or size too big)");
             }
 
             count = count > self.Count ? self.Count : count;
             return RubyArray.Create(self as IList<object>, self.Count -
count, count);
         }
 
         #endregion
 
         #region flatten, flatten!
 
         [MultiRuntimeAware]
         private static RubyUtils.RecursionTracker _infiniteFlattenTracker = new
RubyUtils.RecursionTracker();
 
diff --git
a/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Initializers.Generated.cs
b/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Initializers.Generated.cs
index 213c2b2..0a6ad6b 100644
---
a/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Initializers.Generated.cs
+++
b/Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Initializers.Generated.cs
@@ -5059,47 +5059,47 @@ namespace IronRuby.Builtins {
             );
             
             module.DefineLibraryMethod("each_index", 0x51, 
                 new System.Func<IronRuby.Runtime.RubyContext,
IronRuby.Runtime.BlockParam, System.Collections.IList,
System.Object>(IronRuby.Builtins.IListOps.EachIndex)
             );
             
             module.DefineLibraryMethod("empty?", 0x51, 
                 new System.Func<System.Collections.IList,
System.Boolean>(IronRuby.Builtins.IListOps.Empty)
             );
             
             module.DefineLibraryMethod("eql?", 0x51, 
                 new System.Func<System.Collections.IList, System.Object,
System.Boolean>(IronRuby.Builtins.IListOps.HashEquals)
             );
             
             module.DefineLibraryMethod("fetch", 0x51, 
-                new System.Func<IronRuby.Runtime.RespondToStorage,
IronRuby.Runtime.CallSiteStorage<System.Func<System.Runtime.CompilerServices.CallSite,
System.Object, System.Int32>>, IronRuby.Runtime.BlockParam,
System.Collections.IList, System.Object, System.Object,
System.Object>(IronRuby.Builtins.IListOps.Fetch)
+                new
System.Func<IronRuby.Runtime.ConversionStorage<System.Int32>,
IronRuby.Runtime.BlockParam, System.Collections.IList, System.Object,
System.Object, System.Object>(IronRuby.Builtins.IListOps.Fetch)
             );
             
             module.DefineLibraryMethod("fill", 0x51, 
                 new System.Func<IronRuby.Runtime.RubyContext,
System.Collections.IList, System.Object, System.Int32,
System.Collections.IList>(IronRuby.Builtins.IListOps.Fill),
                 new System.Func<IronRuby.Runtime.RubyContext,
System.Collections.IList, System.Object, System.Int32, System.Int32,
System.Collections.IList>(IronRuby.Builtins.IListOps.Fill),
                 new
System.Func<IronRuby.Runtime.ConversionStorage<System.Int32>,
System.Collections.IList, System.Object, System.Object, System.Object,
System.Collections.IList>(IronRuby.Builtins.IListOps.Fill),
                 new
System.Func<IronRuby.Runtime.ConversionStorage<System.Int32>,
System.Collections.IList, System.Object, IronRuby.Builtins.Range,
System.Collections.IList>(IronRuby.Builtins.IListOps.Fill),
                 new System.Func<IronRuby.Runtime.RubyContext,
IronRuby.Runtime.BlockParam, System.Collections.IList, System.Int32,
System.Object>(IronRuby.Builtins.IListOps.Fill),
                 new System.Func<IronRuby.Runtime.RubyContext,
IronRuby.Runtime.BlockParam, System.Collections.IList, System.Int32,
System.Int32, System.Object>(IronRuby.Builtins.IListOps.Fill),
                 new
System.Func<IronRuby.Runtime.ConversionStorage<System.Int32>,
IronRuby.Runtime.BlockParam, System.Collections.IList, System.Object,
System.Object, System.Object>(IronRuby.Builtins.IListOps.Fill),
                 new
System.Func<IronRuby.Runtime.ConversionStorage<System.Int32>,
IronRuby.Runtime.BlockParam, System.Collections.IList, IronRuby.Builtins.Range,
System.Object>(IronRuby.Builtins.IListOps.Fill)
             );
             
             module.DefineLibraryMethod("first", 0x51, 
                 new System.Func<System.Collections.IList,
System.Object>(IronRuby.Builtins.IListOps.First),
-                new
System.Func<IronRuby.Runtime.CallSiteStorage<System.Func<System.Runtime.CompilerServices.CallSite,
IronRuby.Builtins.RubyClass, System.Object>>, System.Collections.IList,
System.Int32, System.Collections.IList>(IronRuby.Builtins.IListOps.First)
+                new System.Func<System.Collections.IList, System.Int32,
System.Collections.IList>(IronRuby.Builtins.IListOps.First)
             );
             
             module.DefineLibraryMethod("flatten", 0x51, 
                 new
System.Func<IronRuby.Runtime.CallSiteStorage<System.Func<System.Runtime.CompilerServices.CallSite,
IronRuby.Builtins.RubyClass, System.Object>>,
IronRuby.Runtime.ConversionStorage<System.Collections.IList>,
IronRuby.Runtime.RubyContext, System.Collections.IList,
System.Collections.IList>(IronRuby.Builtins.IListOps.Flatten)
             );
             
             module.DefineLibraryMethod("flatten!", 0x51, 
                 new
System.Func<IronRuby.Runtime.CallSiteStorage<System.Func<System.Runtime.CompilerServices.CallSite,
IronRuby.Builtins.RubyClass, System.Object>>,
IronRuby.Runtime.ConversionStorage<System.Collections.IList>,
IronRuby.Runtime.RubyContext, System.Collections.IList,
System.Collections.IList>(IronRuby.Builtins.IListOps.FlattenInPlace)
             );
             
             module.DefineLibraryMethod("hash", 0x51, 
                 new System.Func<System.Collections.IList,
System.Int32>(IronRuby.Builtins.IListOps.GetHashCode)
             );
             
             module.DefineLibraryMethod("include?", 0x51, 
@@ -5125,31 +5125,31 @@ namespace IronRuby.Builtins {
             module.DefineLibraryMethod("insert", 0x51, 
                 new System.Func<IronRuby.Runtime.RubyContext,
System.Collections.IList, System.Int32, System.Object[],
System.Collections.IList>(IronRuby.Builtins.IListOps.Insert)
             );
             
             module.DefineLibraryMethod("inspect", 0x51, 
                 new System.Func<IronRuby.Runtime.RubyContext,
System.Collections.IList,
IronRuby.Builtins.MutableString>(IronRuby.Builtins.IListOps.Inspect)
             );
             
             module.DefineLibraryMethod("join", 0x51, 
                 new
System.Func<IronRuby.Runtime.ConversionStorage<IronRuby.Builtins.MutableString>,
System.Collections.IList,
IronRuby.Builtins.MutableString>(IronRuby.Builtins.IListOps.Join),
                 new
System.Func<IronRuby.Runtime.ConversionStorage<IronRuby.Builtins.MutableString>,
System.Collections.IList, IronRuby.Builtins.MutableString,
IronRuby.Builtins.MutableString>(IronRuby.Builtins.IListOps.Join)
             );
             
             module.DefineLibraryMethod("last", 0x51, 
                 new System.Func<System.Collections.IList,
System.Object>(IronRuby.Builtins.IListOps.Last),
-                new
System.Func<IronRuby.Runtime.CallSiteStorage<System.Func<System.Runtime.CompilerServices.CallSite,
IronRuby.Builtins.RubyClass, System.Object>>, System.Collections.IList,
System.Int32, System.Collections.IList>(IronRuby.Builtins.IListOps.Last)
+                new System.Func<System.Collections.IList, System.Int32,
System.Collections.IList>(IronRuby.Builtins.IListOps.Last)
             );
             
             module.DefineLibraryMethod("length", 0x51, 
                 new System.Func<System.Collections.IList,
System.Int32>(IronRuby.Builtins.IListOps.Length)
             );
             
             module.DefineLibraryMethod("map!", 0x51, 
                 new System.Func<IronRuby.Runtime.RubyContext,
IronRuby.Runtime.BlockParam, System.Collections.IList,
System.Object>(IronRuby.Builtins.IListOps.CollectInPlace)
             );
             
             module.DefineLibraryMethod("nitems", 0x51, 
                 new System.Func<System.Collections.IList,
System.Int32>(IronRuby.Builtins.IListOps.NumberOfNonNilItems)
             );
             
             module.DefineLibraryMethod("pop", 0x51,

Result of "git diff HEAD~1 -U15" would be nice if you can remember to
include it, but don?t worry about it too much. I will try to deal with the
github.com web view and see if it bothers me too much.

-----Original Message-----
From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at
rubyforge.org] On Behalf Of Daniele Alessandri
Sent: Thursday, April 16, 2009 2:28 AM
To: ironruby-core at rubyforge.org
Subject: Re: [Ironruby-core] [patch] More fixes for Array (IListOps.cs)

On Thu, Apr 16, 2009 at 01:14, Shri Borde <Shri.Borde at microsoft.com>
wrote:
> In IListOps.Fetch, instead of doing the protocol conversion manually, 
> you should calls Protocols. ConvertToInteger. Tomas, can you double-check
this?
Hmm, right, and the code looks much more cleaner too.
I think I should just use Protocols.CastToFixnum then: we don''t really
care about bignums here, they can''t be used as indexes for arrays
anyway and in fact MRI throws a RangeError "bignum too big to convert into
`long''".
> In IListOps.First, you can get rid of the allocateStorage argument as 
> its not being used anymore. (You can batch this fix with your next 
> commit)
Same for IListOps.Last, it just slipped away. I removed allocateStorage on both.
> Btw, could you please increase the diff context size to 15-20 so more 
> of the surrounding lines are included? That makes it easier to 
> understand what the fix is doing when using the url below, without 
> having to map that back to the correct line number in the actual file.
I guess there is no way to increase the lines range of a diff viewed via the
github web interface, but I can generate and attach custom diff patches to my
mails if you think it could be useful, just like the one attached to this mail
and generated with "git diff HEAD~1 -U15
> patch.diff" (PS: I have not yet pushed this commit on my remote
repository).

--
Daniele Alessandri
http://www.clorophilla.net/blog/
http://twitter.com/JoL1hAHN