[Ironruby-core] Code review: String#unpack, Bignum and Array related stuff

Daniele Alessandri suppakilla at gmail.com
Thu Jul 23 15:48:42 EDT 2009


Thanks.
Rebased and pushed to remote, my master branch on github is ready for tomorrow.

Daniele


On Thu, Jul 23, 2009 at 21:25, Tomas
Matousek<Tomas.Matousek at microsoft.com> wrote:
> Looks good.
>
> Tomas
>
> -----Original Message-----
> From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Daniele Alessandri
> Sent: Thursday, July 23, 2009 11:38 AM
> To: ironruby-core at rubyforge.org
> Subject: Re: [Ironruby-core] Code review: String#unpack, Bignum and Array related stuff
>
> Hi,
>
> Cool I didn't know [DefaultProtocol] was valid for params arguments too.
>
> I fixed the code and pushed it to GitHub, here is the commit:
> http://github.com/nrk/ironruby/commit/00a9a22440ad243eabfd4795788261f2d7ce12cd
>
> If it's OK, I can rebase everything into my master branch to get it ready for tomorrow's pull.
>
> Thanks,
> Daniele
>
>
> On Thu, Jul 23, 2009 at 00:25, Tomas
> Matousek<Tomas.Matousek at microsoft.com> wrote:
>> Actually, wouldn't
>>
>> public static IList/*!*/ Zip(CallSiteStorage<EachSite>/*!*/ each,
>> ConversionStorage<IList>/*!*/ tryToAry, BlockParam block,  object
>> self, [DefaultProtocol, NotNull, NotNullItems]params IList[] args) {
>>
>> work? If [DefaultProtocol] is applied on a params array of IList it converts all parameters to IList via to_ary method.
>>
>> Also Enumerable.Zip should be split into 2 methods: one that performs to_a conversion on all object args and other (internal) strongly typed method that takes IList[] and performs no conversions.
>> IList.zip should then call the latter so that to_a conversions are not performed unnecessarily on already converted values.
>>
>> This is what I tested in MRI:
>>
>> class C
>>  def respond_to? name
>>    puts name
>>    true
>>  end
>>
>>  def to_ary
>>    [1]
>>  end
>>
>>  def to_a
>>    [2]
>>  end
>> end
>>
>> class D
>>  include Enumerable
>>
>>  def each
>>    1
>>  end
>> end
>>
>> [1,2,3].zip(C.new, C.new, C.new) #=> to_ary, to_ary, to_ary
>> D.new.zip(C.new, C.new, C.new) #=> to_a, to_a, to_a
>>
>> Tomas
>>
>> -----Original Message-----
>> From: ironruby-core-bounces at rubyforge.org
>> [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Shri Borde
>> Sent: Wednesday, July 22, 2009 2:52 PM
>> To: ironruby-core at rubyforge.org
>> Subject: Re: [Ironruby-core] Code review: String#unpack, Bignum and
>> Array related stuff
>>
>> Looks good. In Array#zip, you prefer casting to IList, and use Protocols.TryCastToArray as a fallback. Is that correct? If an IList object defines to_a, shouldn't to_a get called?
>>
>> The array argument to Array#zip is marked as [NotNull]. Should it also be marked as [NotNullItems]?
>>
>> Thanks,
>> Shri
>>
>>
>> -----Original Message-----
>> From: ironruby-core-bounces at rubyforge.org
>> [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Daniele
>> Alessandri
>> Sent: Wednesday, July 22, 2009 1:51 PM
>> To: ironruby-core at rubyforge.org
>> Subject: Re: [Ironruby-core] Code review: String#unpack, Bignum and
>> Array related stuff
>>
>> Just a quick note: updated with
>> http://github.com/nrk/ironruby/commit/82feca718fc17ad45747ca45a1c92250
>> abfcfce0
>>
>> On Sun, Jul 19, 2009 at 11:29, Daniele Alessandri<suppakilla at gmail.com> wrote:
>>> Hi
>>> first of all I am trying a different approach to submitting my
>>> patches: I created a code_review branch on my repository in which I
>>> am going to push all my fixes for review. Fixes that will pass review
>>> will be rebased (not merged) in my main branch (ready to get pulled
>>> into the ironruby repository), the others will remain staged in my
>>> code_review branch for further works until they are "approved". I am
>>> just experimenting a different git workflow :-)
>>>
>>> Anyway, let's move on to the new commits:
>>>
>>>
>>> *
>>> http://github.com/nrk/ironruby/commit/48a4a02b5b47d61f2f7a3f3887ea4bf
>>> 0
>>> 2d63edb4
>>>
>>> - More fixes for String#unpack:
>>>   o Fixed errors when unpacking data with 'Z' and '@' directives
>>>   o Implemented 'Q' and 'q' directives
>>>
>>>
>>> *
>>> http://github.com/nrk/ironruby/commit/17b5edab64967b0200370edf6657321
>>> a
>>> 0f79953d
>>>
>>> - Array related fixes:
>>>   o Array#== does not call #to_ary on its argument but it calls #to_a (it
>>>   basically performs a conversion of the argument rather than casting it to a
>>>   ruby array)
>>>   o Array#zip calls #to_ary to cast the argument to an Array, therefore it
>>>   overrides Enumerable#zip in which the argument is converted by calling
>>>   #to_a.
>>>
>>>
>>> *
>>> http://github.com/nrk/ironruby/commit/547d810c78b38afc34e728855ab7d0c
>>> 2
>>> 0f499719
>>>
>>> - Bignum related fixes:
>>>   o Changed the signature for ClrBigInteger.ToString(self,radix), now
>>>   Bignum#to_s works as expected raising an ArgumentException if base is less
>>>   than 2 or higher than 36.
>>>   o Fixed Bignum#divmod, Bignum#remainder, Bignum#% and Bignum#modulo to work
>>>   with Float values as argument.
>>>   o Fixed Bignum#/ and Bignum#div as they behave differently with each other
>>>   when Float values are passed as argument.
>>>
>>> Note: Now bignum does not fail any of the expectations in core/bignum.
>>>
>>> See also the attached diff.
>>>
>>> Thanks,
>>> Daniele
>>
>>
>> --
>> 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
>> _______________________________________________
>> 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
> _______________________________________________
> 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


More information about the Ironruby-core mailing list