[Approval needed] Re: [Mono-devel-list] Patch for Char. Pleasetest if it breaks your assembly!

Andreas Nahr ClassDevelopment at A-SoftTech.com
Mon May 17 13:21:58 EDT 2004


> On Sun, 2004-05-16 at 17:26, Miguel de Icaza wrote:
> > Hello,
> >
> > > But we need to do this change at some point in time...
> > > So if nobody objects I'll commit soon.
> >
> > I want Dick and Paolo to approve this patch before it goes into CVS.
>
> I don't know what the comment ("needs to call line below, but that would
> probably break a lot of things right now") is referring to.  The comment
> was written by Andreas when he wrote the Invariant optimisations.

I added this line when I made the optimizations because I found the bug that
we are not calling ToLower/ToUpper with the current culture but with
invariant culture.
I did not make the change back then because doing the change would have
broken code. Why? Because at the time nearly all of the corelib code in mono
depended on that bug (there was only one exception). I changed the possible
problematic areas in corelib. But as I saw that in corelib there were nearly
20 places where this would have produced problems and only 1 place where it
was correct I assumed that in other assemblies the situation might be the
same and decided to not do the patch back then.
All that was probably 1-2 months ago. Back then I wrote that people need to
check the assemblies for code that calls Char.ToLower/Upper and depends on
this being the INVARIANT version (which it shouldn't be).
It seemed to me that either nobody did check anything or there were no bugs,
that's why I wanted to commit now

> The spec says the current culture should be used, so this should be
> committed.

Thats the reason why this needs to be applied.

However if we have code like:
char CaseInsensitiveRegex = 'R';
if (Char.ToLower(CaseInsensitiveRegex) == 'r')

Then this will fail in turkish locale after the patch (at least I think the
R was one of those problematic chars) because the author probably wanted to
call:
if (Char.ToLower(CaseInsensitiveRegex, CultureInfo.InvariantCulture) == 'r')

And as I sayed: I found lots of code like this in corlib so I thougth it may
be likely that such bugs (depending on the Char.ToLower bug) may also exist
in other assemblies.

Andreas




More information about the Mono-devel-list mailing list