Thanks for the patch!

Jim, could you look at that?

[Inline]

-----Original Message-----
From: ironruby-core-boun...@rubyforge.org 
[mailto:ironruby-core-boun...@rubyforge.org] On Behalf Of Daniele Alessandri
Sent: Friday, August 20, 2010 12:00 PM
To: ironruby-core@rubyforge.org
Subject: [Ironruby-core] Review: miscellaneous fixes

Hi,
today I've finally started fixing some failing specs in the core libraries of 
IronRuby (well actually I spent most of the time reviewing the smaller changes 
between MRI 1.8 and 1.9 and reading some parts of the source code of IronRuby 
since it's been quite some time since the last time I contributed). I still 
have to push my changes on a remote repository but in the meantime you can find 
them in the diff attached to this email.

304459b Implement various fixes to Kernel and Array related to the trust status 
of Object instances.

[Better fix would be:
return stream.String.TaintBy(format);

This would take care of these two failures as well:
fails:Array#pack returns a tainted string when the format is tainted
fails:Array#pack returns a tainted string when the format is tainted even if 
the given format is empty

Note that context.TaintObjectBy should only be used on objects that are not 
statically typed to a type that implements IRubyObjectState. MutableString does.
]

e77abb5 Fix: Kernel#taint and Kernel#untaint raise an exception when trying to 
modify the taint status of a frozen object.

[This is better done internally. See your question below ☺]

3f0760b Fix: Array#pop raises a RuntimeError on a frozen array.
9e4defb Implement Array#pop(n).
1ba0628 Fix: Object#=~ returns nil matching any object.
19e8250 Update the unsigned version of App.config to pick the right library 
paths for 1.9.

A few question / notes:
- If nothing has changed since the last time, I guess you can't still merge 
changes coming from the community that don't apply to parts of IronRuby outside 
Libraries.LCA_RESTRICTED, right?

[Nothing changed yet.]

- I noticed that the contents of .gitignore have been changed in the master 
branch and now it contains only one line. I wonder if this change is 
intentional, now I get a lot of garbage (e.g. the output of the compilation) in 
the list of unstaged changes.

[Jim?]

- Many specs of Array, Hash and String fail because MRI 1.9 now raises a 
RuntimeError instead of a TypeError when trying to modify a frozen object. The 
change in
http://github.com/ironruby/ironruby/blob/d0fcc0a132c1ca4f07a11369293d1c5801a6a0d3/Languages/Ruby/Ruby/Runtime/RubyExceptions.cs#L49
is trivial though.

[Fixed]

- The checks I added when modiying taintness or trustiness on frozen objects 
should be moved inside RubyContext, respectively to
SetObjectTaint() SetObjectTrustiness().

[Fixed as well.]

- It would be nice to have a TrustObjectBy<T>(RubyContext, Object) method in 
RubyContext.

[I'm not sure this is needed. Instead I modified TaintObjectBy to copy 
trustiness as well - it seems that most of the time "trust" and "taint" are 
both copied.
This makes the changes in KernelOps.cs unnecessary (Clone, Trust, Untrust, 
Taint, Untaint). And also the change in IListOps.Repetition and Compact.
]

- If you are wondering why I haven't implemented the last three items myself, 
see my first question :-)

Thanks,
Daniele

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

_______________________________________________
Ironruby-core mailing list
Ironruby-core@rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core

Reply via email to