[Ironruby-core] More fixes for core/array specs (again)

Daniele Alessandri suppakilla at gmail.com
Tue Apr 21 14:42:49 EDT 2009


Just pushed my changes (with the added comment to IListOps.Difference
as suggested by Shri).

http://github.com/nrk/ironruby/commit/722dd4d2dc233458a49f154b926ea666d2cdb324

Thanks,
Daniele


On Tue, Apr 21, 2009 at 13:43, Daniele Alessandri <suppakilla at gmail.com> wrote:
> Right now I don't have access to my local repository, I think to be
> able to push on github by 9:00 PM CEST (UTC+2, that's my timezone, it
> should be 12:00 PM PDT).
>
> On Tue, Apr 21, 2009 at 08:39, Jim Deville <jdeville at microsoft.com> wrote:
>> Daniele,
>>
>> If you push this out tonight, I'll pull it in tomorrow.
>>
>> JD
>>
>>> -----Original Message-----
>>> From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-
>>> bounces at rubyforge.org] On Behalf Of Shri Borde
>>> Sent: Monday, April 20, 2009 9:58 PM
>>> To: ironruby-core at rubyforge.org
>>> Subject: Re: [Ironruby-core] More fixes for core/array specs (again)
>>>
>>> Looks great!
>>>
>>> Could you copy the comment "# MRI follows hashing semantics here, so
>>> doesn't actually call eql?/hash for Fixnum/Symbol" from
>>> core\array\minus_spec.rb to IListOps.Difference as its not obvious why
>>> the code in IListOps.Difference does the complicated checks.
>>>
>>> Yes, it seems OK that you can call all the Array methods on
>>> System::Collections::ArrayList (and any System::Collectinos::IList
>>> object in general). We are exposing Ruby String methods on
>>> System::String, and it would be consistent with that approach.
>>>
>>> About List<T>.Reverse, you could argue that if you use Ruby casing of
>>> "reverse", IListOps should get precedence. Could you open a bug for
>>> this please? If you use "Reverse", then the CLR method should get
>>> precedence, and there would be a possibility of confusion, but as you
>>> say, there is not much that can be done. It does not seem worth
>>> special-casing.
>>>
>>> Thanks,
>>> Shri
>>>
>>>
>>> -----Original Message-----
>>> From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-
>>> bounces at rubyforge.org] On Behalf Of Daniele Alessandri
>>> Sent: Monday, April 20, 2009 12:46 PM
>>> To: ironruby-core at rubyforge.org
>>> Subject: [Ironruby-core] More fixes for core/array specs (again)
>>>
>>> Hi,
>>> I have attached a diff with new fixes for a whole bunch of failing
>>> expectations in the core/array specs.
>>>
>>> * Slightly modified IListOps.UniqueSelf as suggested upon review.
>>> * Fixed IListOps.ReverseIndex as Array#rindex should not fail if
>>> elements are removed from the array during the iteration over its
>>> elements.
>>> * Changed IListOps.Reverse to return IList instances of the same type
>>> of the given self argument (this change also fixes the following
>>> failing spec: "Array#reverse returns subclass instance on Array
>>> subclasses")
>>> * Changed IListOps.SetElement to make it try to invoke #to_ary on its
>>> argument for multi-element sets.
>>> * Changed one overload of IListOps.Equals to make it try to call
>>> #to_ary on the object argument of Array#== and use the resulting array
>>> as the actual argument for the equality check.
>>> * Cleaning up tags removing expectations that do not fail anymore.
>>> * Added ArrayOps.ToAry as Array#to_a and Array#to_ary behave
>>> differently on subclasses of arrays.
>>> * Various changes to IListOps.Join to clear all of the remaining tags
>>> for the specs of Array#join. The tags marked as critical in
>>> join_tags.txt are not related to pending bugs for Array#join.
>>> * Changed IListOps.Repetition to return IList instances of the same
>>> type of the given self argument (this fixes also "Array#* with an
>>> integer returns subclass instance with Array subclasses")
>>> * Modified IListOps.RecursiveJoin to make it flag the resulting string
>>> as tainted if the given array, at least one of its elements or the
>>> separator string are tainted.
>>> * IListOps.Difference now uses Object#hash and Object#eql? to check for
>>> object equality, this fixes the failing spec "Array#- acts as if using
>>> an intermediate hash to collect values"
>>>
>>> These changes directly clear the following failing expectations:
>>>
>>> Array#[]= calls to_ary on its rhs argument for multi-element sets
>>> Array#== calls to_ary on its argument Array#join tries to convert the
>>> passed seperator to a String using #to_str Array#join checks whether
>>> the passed seperator responds to #to_str Array#join handles recursive
>>> arrays Array#join does not consider taint of either the array or the
>>> separator when the array is empty Array#join returns a string which
>>> would be infected with taint of the array, its elements or the
>>> separator when the array is not empty Array#join does not process the
>>> separator if the array is empty Array#join raises a TypeError if the
>>> passed separator is not a string and does not respond to #to_str
>>> Array#- acts as if using an  intermediate hash to collect values
>>> Array#* with an integer returns subclass instance with Array subclasses
>>> Array#* with an integer copies the taint status of the original array
>>> even if the array is empty
>>> Array#* with an integer copies the taint status of the original array
>>> if the passed count is not 0
>>> Array#* with a string returns a string which would be infected with
>>> taint of the array, its elements or the separator when the array is not
>>> empty Array#reverse returns subclass instance on Array subclasses
>>> Array#rindex does not fail when removing elements from block
>>>
>>> I have only a question: I've made changes to IListOps.Reverse and
>>> IListOps.Repetition in order to make them return IList instead of
>>> RubyArray. In ruby, Array#reverse and Array#* maintain the same type of
>>> self for the returning object (that is, if self is a RubyArray.Subclass
>>> then it is returned a new object of the same type).
>>> With these changes we would actually be able to do something like
>>> this:
>>>
>>> >>> arraylist = System::Collections::ArrayList.new([1,2,3,4,5])
>>> => [1, 2, 3, 4, 5]
>>> >>> multiplied_arraylist = arraylist * 2
>>> => [1, 2, 3, 4, 5, 1, 2, 3, 4, 5]
>>> >>> multiplied_arraylist.class
>>> => System::Collections::ArrayList
>>>
>>> Would it be a consistent behaviour?
>>>
>>> On a (partially) unrelated note, I've just noticed that List<T>.Reverse
>>> is more like Array#reverse! but it actually take the precedence over
>>> IListOps.Reverse. I'm just thinking if this could lead to some sort of
>>> confusion (ohan I'm not sure if anything can be done about this.
>>>
>>> Thanks,
>>> Daniele
>>>
>>>
>>> PS: I'm waiting to push these changes on my remote repository, Jim told
>>> he is going to pull today but I would like to wait for a review first
>>> (and yeah, today I'm too lazy to actually start off a new branch :-)).
>>>
>>> --
>>> Daniele Alessandri
>>> http://www.clorophilla.net/blog/
>>> http://twitter.com/JoL1hAHN
>>> _______________________________________________
>>> Ironruby-core mailing list
>>> Ironruby-core at rubyforge.org
>>> http://rubyforge.org/mailman/listinfo/ironruby-core
>>
>> _______________________________________________
>> Ironruby-core mailing list
>> Ironruby-core at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/ironruby-core
>>
>
>
>
> --
> Daniele Alessandri
> http://www.clorophilla.net/blog/
> http://twitter.com/JoL1hAHN
>



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


More information about the Ironruby-core mailing list