[Ironruby-core] Review: miscellaneous fixes

Daniele Alessandri suppakilla at gmail.com
Sun Aug 22 05:44:36 EDT 2010


On Sun, Aug 22, 2010 at 02:43, Tomas Matousek
<Tomas.Matousek at microsoft.com> wrote:

I have rebased my local branch on top of your latest changes and
removed the unneeded commits, now everything is available on my remote
repository in a separate topic branch (see
http://github.com/nrk/ironruby/tree/corelib19). The three commits
"survived" are:

793effd Implement Array#pop(n).
503002f Fix: Object#=~ returns nil matching any object.
7a6275b Update the unsigned version of App.config to pick the right
library paths for 1.9.

See http://github.com/nrk/ironruby/compare/014e640ed27c28ff97fb...793effd4d75f39f36ff3
There are also new changes in that branch, but I'm going to post a new
message with a separate diff for those.


> 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);

Fix applied... and it actually lead me to notice a bug related to
operations on the trust status of MutableString instances (see below):

>>> "".untrust
=> ""
>>> "".untrust.untrust
(ir):1:in `untrust': can't modify frozen object (RuntimeError)
        from (ir):1
>>> "".untrust.trust
(ir):1:in `trust': can't modify frozen object (RuntimeError)
        from (ir):1
>>> "".trust.untrust
=> ""


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

Ah, right!


>> - 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 think I still need to find a decent workflow when working on stuff
that can't be merged back by you because of the current constraints,
maybe I'll just limit myself to do some experiments in a further
branch and report back just like I did in my previous mail.


> [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 trust and taint are both copied then yes, I agree that's the way to
go. I just wasn't sure about this behaviour so I thought that keeping
them separate (with maybe a new method in RubyContext such as
CopyFlags or anything along this line) might have been safer.

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


More information about the Ironruby-core mailing list