Re: [Mono-dev] [Mono-patches] r107198 - trunk/mcs/class/corlib/System

2008-07-03 Thread Mirco Bauer
On Thu, 2008-07-03 at 18:23 -0400, Andreas Nahr
([EMAIL PROTECTED]) wrote:
> Author: andreas
> Date: 2008-07-03 18:23:54 -0400 (Thu, 03 Jul 2008)
> New Revision: 107198
> 
> Modified:
>trunk/mcs/class/corlib/System/ChangeLog
>trunk/mcs/class/corlib/System/IntPtr.cs
> Log:
> 2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
> 
>   * IntPtr: Fix parameter names, change internal name to accomodate for 
> parameter changes
> 
> Modified: trunk/mcs/class/corlib/System/ChangeLog
> ===
> --- trunk/mcs/class/corlib/System/ChangeLog   2008-07-03 22:22:43 UTC (rev 
> 107197)
> +++ trunk/mcs/class/corlib/System/ChangeLog   2008-07-03 22:23:54 UTC (rev 
> 107198)
> @@ -1,5 +1,9 @@
>  2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
>  
> + * IntPtr: Fix parameter names, change internal name to accomodate for 
> parameter changes
> +
> +2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
> +
>   * Predicate.cs:
>   * Object.cs:
>   * Nullable.cs
> 
> Modified: trunk/mcs/class/corlib/System/IntPtr.cs
> ===
> --- trunk/mcs/class/corlib/System/IntPtr.cs   2008-07-03 22:22:43 UTC (rev 
> 107197)
> +++ trunk/mcs/class/corlib/System/IntPtr.cs   2008-07-03 22:23:54 UTC (rev 
> 107198)
> @@ -57,44 +57,44 @@
>  #endif
>   public unsafe struct IntPtr : ISerializable
>   {
> - private void *value;
> + private void *m_value;

I am not sure, but doesn't this break binary serialization
compatibility?

>  
>   public static readonly IntPtr Zero;
>  
>  #if NET_2_0
>   [ReliabilityContract (Consistency.MayCorruptInstance, 
> Cer.MayFail)]
>  #endif
> - public IntPtr (int i32)
> + public IntPtr (int value)
>   {
> - value = (void *) i32;
> + m_value = (void *) value;

afaik the goal can also be archived using this.value = value;

thats all, thanks.

-- 
Regards,

Mirco 'meebey' Bauer

PGP-Key ID: 0xEEF946C8

FOSS Developer[EMAIL PROTECTED]  http://www.meebey.net/
PEAR Developer[EMAIL PROTECTED] http://pear.php.net/
Debian Developer  [EMAIL PROTECTED]  http://www.debian.org/

___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list


Re: [Mono-dev] [Mono-patches] r107198 - trunk/mcs/class/corlib/System

2008-07-03 Thread Andreas Nahr
In Text

> -Ursprüngliche Nachricht-
> Von: Mirco Bauer [mailto:[EMAIL PROTECTED]
> Gesendet: Freitag, 4. Juli 2008 01:52
> An: mono-devel-list@lists.ximian.com
> Cc: [EMAIL PROTECTED]
> Betreff: Re: [Mono-patches] r107198 - trunk/mcs/class/corlib/System
> 
> On Thu, 2008-07-03 at 18:23 -0400, Andreas Nahr
> ([EMAIL PROTECTED]) wrote:
> > Author: andreas
> > Date: 2008-07-03 18:23:54 -0400 (Thu, 03 Jul 2008)
> > New Revision: 107198
> >
> > Modified:
> >trunk/mcs/class/corlib/System/ChangeLog
> >trunk/mcs/class/corlib/System/IntPtr.cs
> > Log:
> > 2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
> >
> > * IntPtr: Fix parameter names, change internal name to accomodate
> for parameter changes
> >
> > Modified: trunk/mcs/class/corlib/System/ChangeLog
> > ===
> > --- trunk/mcs/class/corlib/System/ChangeLog 2008-07-03 22:22:43 UTC
> (rev 107197)
> > +++ trunk/mcs/class/corlib/System/ChangeLog 2008-07-03 22:23:54 UTC
> (rev 107198)
> > @@ -1,5 +1,9 @@
> >  2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
> >
> > +   * IntPtr: Fix parameter names, change internal name to accomodate
> for parameter changes
> > +
> > +2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
> > +
> > * Predicate.cs:
> > * Object.cs:
> > * Nullable.cs
> >
> > Modified: trunk/mcs/class/corlib/System/IntPtr.cs
> > ===
> > --- trunk/mcs/class/corlib/System/IntPtr.cs 2008-07-03 22:22:43 UTC
> (rev 107197)
> > +++ trunk/mcs/class/corlib/System/IntPtr.cs 2008-07-03 22:23:54 UTC
> (rev 107198)
> > @@ -57,44 +57,44 @@
> >  #endif
> > public unsafe struct IntPtr : ISerializable
> > {
> > -   private void *value;
> > +   private void *m_value;
> 
> I am not sure, but doesn't this break binary serialization
> compatibility?

It shouldn't. IntPtr has an explicit Serializer implementation that deals
with setting correct names. I did not change those.

> >
> > public static readonly IntPtr Zero;
> >
> >  #if NET_2_0
> > [ReliabilityContract (Consistency.MayCorruptInstance,
> Cer.MayFail)]
> >  #endif
> > -   public IntPtr (int i32)
> > +   public IntPtr (int value)
> > {
> > -   value = (void *) i32;
> > +   m_value = (void *) value;
> 
> afaik the goal can also be archived using this.value = value;

Did you try that? I actually had it this way and it refused to compile
because the value was deemed not initialized. I've got to admit that I
wasn't exactly sure why it didn't. Potential issue with the compiler maybe?

> thats all, thanks.
> 
> --
> Regards,
> 
> Mirco 'meebey' Bauer
> 
> PGP-Key ID: 0xEEF946C8
> 
> FOSS Developer[EMAIL PROTECTED]  http://www.meebey.net/
> PEAR Developer[EMAIL PROTECTED] http://pear.php.net/
> Debian Developer  [EMAIL PROTECTED]  http://www.debian.org/

___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list


Re: [Mono-dev] [Mono-patches] r107198 - trunk/mcs/class/corlib/System

2008-07-04 Thread Mirco Bauer
Hi Andreas,

my reply is inline, see below.

On Fri, 2008-07-04 at 08:59 +0200, Andreas Nahr wrote:
> In Text
> 
> > -Ursprüngliche Nachricht-
> > Von: Mirco Bauer [mailto:[EMAIL PROTECTED]
> > Gesendet: Freitag, 4. Juli 2008 01:52
> > An: mono-devel-list@lists.ximian.com
> > Cc: [EMAIL PROTECTED]
> > Betreff: Re: [Mono-patches] r107198 - trunk/mcs/class/corlib/System
> > 
> > On Thu, 2008-07-03 at 18:23 -0400, Andreas Nahr
> > ([EMAIL PROTECTED]) wrote:
> > > Author: andreas
> > > Date: 2008-07-03 18:23:54 -0400 (Thu, 03 Jul 2008)
> > > New Revision: 107198
> > >
> > > Modified:
> > >trunk/mcs/class/corlib/System/ChangeLog
> > >trunk/mcs/class/corlib/System/IntPtr.cs
> > > Log:
> > > 2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
> > >
> > >   * IntPtr: Fix parameter names, change internal name to accomodate
> > for parameter changes
> > >
> > > Modified: trunk/mcs/class/corlib/System/ChangeLog
> > > ===
> > > --- trunk/mcs/class/corlib/System/ChangeLog   2008-07-03 22:22:43 UTC
> > (rev 107197)
> > > +++ trunk/mcs/class/corlib/System/ChangeLog   2008-07-03 22:23:54 UTC
> > (rev 107198)
> > > @@ -1,5 +1,9 @@
> > >  2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
> > >
> > > + * IntPtr: Fix parameter names, change internal name to accomodate
> > for parameter changes
> > > +
> > > +2008-07-04  Andreas Nahr <[EMAIL PROTECTED]>
> > > +
> > >   * Predicate.cs:
> > >   * Object.cs:
> > >   * Nullable.cs
> > >
> > > Modified: trunk/mcs/class/corlib/System/IntPtr.cs
> > > ===
> > > --- trunk/mcs/class/corlib/System/IntPtr.cs   2008-07-03 22:22:43 UTC
> > (rev 107197)
> > > +++ trunk/mcs/class/corlib/System/IntPtr.cs   2008-07-03 22:23:54 UTC
> > (rev 107198)
> > > @@ -57,44 +57,44 @@
> > >  #endif
> > >   public unsafe struct IntPtr : ISerializable
> > >   {
> > > - private void *value;
> > > + private void *m_value;
> > 
> > I am not sure, but doesn't this break binary serialization
> > compatibility?
> 
> It shouldn't. IntPtr has an explicit Serializer implementation that deals
> with setting correct names. I did not change those.

Oh ok, didn't see that (from the patch).

> 
> > >
> > >   public static readonly IntPtr Zero;
> > >
> > >  #if NET_2_0
> > >   [ReliabilityContract (Consistency.MayCorruptInstance,
> > Cer.MayFail)]
> > >  #endif
> > > - public IntPtr (int i32)
> > > + public IntPtr (int value)
> > >   {
> > > - value = (void *) i32;
> > > + m_value = (void *) value;
> > 
> > afaik the goal can also be archived using this.value = value;
> 
> Did you try that? I actually had it this way and it refused to compile
> because the value was deemed not initialized. I've got to admit that I
> wasn't exactly sure why it didn't. Potential issue with the compiler maybe?

At least for me it works, using gmcs 1.9.1:

[EMAIL PROTECTED]:~$ cat test.cs
using System;

class Test
{
int value;

Test(int value)
{
this.value = value;
}

public static void Main ()
{
Test t = new Test(123);
Console.WriteLine(t.value);
}
}
[EMAIL PROTECTED]:~$ mono test.exe
123
[EMAIL PROTECTED]:~$ 

I just find a bit intrusive/overkill to rename class members because of
parameter names of some ctor or method... (especially when they come
from MS .NET) as it might have side-effects like serialization comp for
classes that don't have an explicit implementation.

Thanks for your effort.

-- 
Regards,

Mirco 'meebey' Bauer

PGP-Key ID: 0xEEF946C8

FOSS Developer[EMAIL PROTECTED]  http://www.meebey.net/
PEAR Developer[EMAIL PROTECTED] http://pear.php.net/
Debian Developer  [EMAIL PROTECTED]  http://www.debian.org/


signature.asc
Description: This is a digitally signed message part
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list


Re: [Mono-dev] [Mono-patches] r107198 - trunk/mcs/class/corlib/System

2008-07-04 Thread Andreas Nahr


> -Ursprüngliche Nachricht-
> Von: [EMAIL PROTECTED] [mailto:mono-devel-list-
> [EMAIL PROTECTED] Im Auftrag von Mirco Bauer
> Gesendet: Freitag, 4. Juli 2008 12:59
> An: Andreas Nahr
> Cc: mono-devel-list@lists.ximian.com
> Betreff: Re: [Mono-dev] [Mono-patches] r107198 -
> trunk/mcs/class/corlib/System
> > > >
> > > > public static readonly IntPtr Zero;
> > > >
> > > >  #if NET_2_0
> > > > [ReliabilityContract (Consistency.MayCorruptInstance,
> > > Cer.MayFail)]
> > > >  #endif
> > > > -   public IntPtr (int i32)
> > > > +   public IntPtr (int value)
> > > > {
> > > > -   value = (void *) i32;
> > > > +   m_value = (void *) value;
> > >
> > > afaik the goal can also be archived using this.value = value;
> >
> > Did you try that? I actually had it this way and it refused to
> compile
> > because the value was deemed not initialized. I've got to admit that
> I
> > wasn't exactly sure why it didn't. Potential issue with the compiler
> maybe?
> 
> At least for me it works, using gmcs 1.9.1:
> 
> [EMAIL PROTECTED]:~$ cat test.cs
> using System;
> 
> class Test
> {
>   int value;
> 
>   Test(int value)
>   {
>   this.value = value;
>   }
> 
>   public static void Main ()
>   {
>   Test t = new Test(123);
>   Console.WriteLine(t.value);
>   }
> }

Are you trying to make a joke? ;)
Of course this works with "normal" types. This is used thousands of times 
throughout the class libraries.
But in *THIS* case with void * it does not seem to work. So this was the 
easiest way to fix the problem.

Greetings Andreas

P.S. If you want to make further "tests" maybe mail me private and not through 
the list to keep the noise low.

___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list


Re: [Mono-dev] [Mono-patches] r107198 - trunk/mcs/class/corlib/System

2008-07-04 Thread Mirco Bauer
On Fri, 2008-07-04 at 16:53 +0200, Andreas Nahr wrote:
> 
> > -Ursprüngliche Nachricht-
> > Von: [EMAIL PROTECTED] [mailto:mono-devel-list-
> > [EMAIL PROTECTED] Im Auftrag von Mirco Bauer
> > Gesendet: Freitag, 4. Juli 2008 12:59
> > An: Andreas Nahr
> > Cc: mono-devel-list@lists.ximian.com
> > Betreff: Re: [Mono-dev] [Mono-patches] r107198 -
> > trunk/mcs/class/corlib/System
> > > > >
> > > > >   public static readonly IntPtr Zero;
> > > > >
> > > > >  #if NET_2_0
> > > > >   [ReliabilityContract (Consistency.MayCorruptInstance,
> > > > Cer.MayFail)]
> > > > >  #endif
> > > > > - public IntPtr (int i32)
> > > > > + public IntPtr (int value)
> > > > >   {
> > > > > - value = (void *) i32;
> > > > > + m_value = (void *) value;
> > > >
> > > > afaik the goal can also be archived using this.value = value;
> > >
> > > Did you try that? I actually had it this way and it refused to
> > compile
> > > because the value was deemed not initialized. I've got to admit that
> > I
> > > wasn't exactly sure why it didn't. Potential issue with the compiler
> > maybe?
> > 
> > At least for me it works, using gmcs 1.9.1:
> > 
> > [EMAIL PROTECTED]:~$ cat test.cs
> > using System;
> > 
> > class Test
> > {
> > int value;
> > 
> > Test(int value)
> > {
> > this.value = value;
> > }
> > 
> > public static void Main ()
> > {
> > Test t = new Test(123);
> > Console.WriteLine(t.value);
> > }
> > }
> 
> Are you trying to make a joke? ;)
> Of course this works with "normal" types.

I didn't test exactly the same situation, but I prooved the way it
should work like.

>  This is used thousands of times throughout the class libraries.
> But in *THIS* case with void * it does not seem to work. So this was the 
> easiest way to fix the problem.

Ok, I used a class and int as type. So here a test using an unsafe
struct and void* as member, just like IntPtr does:

[EMAIL PROTECTED]:~$ cat test.cs
using System;

unsafe class Test
{
int value;

Test(int value)
{
this.value = value;
}

public static void Main ()
{
Test t = new Test(123);
Console.WriteLine(t.value);

AnotherTest at = new AnotherTest(123);
Console.WriteLine((int) at.value);
}
}

public unsafe struct AnotherTest
{
internal void* value;

public AnotherTest(int value)
{
this.value = (void*) value;
}
}

[EMAIL PROTECTED]:~$ mono test.exe
123
123

> 
> Greetings Andreas
> 
> P.S. If you want to make further "tests" maybe mail me private and not 
> through the list to keep the noise low.

I don't think this is noise as you had issues with a possible compiler
bug, so allow other developers to join the discussion and comment on the
test-code.

-- 
Regards,

Mirco 'meebey' Bauer

PGP-Key ID: 0xEEF946C8

FOSS Developer[EMAIL PROTECTED]  http://www.meebey.net/
PEAR Developer[EMAIL PROTECTED] http://pear.php.net/
Debian Developer  [EMAIL PROTECTED]  http://www.debian.org/


signature.asc
Description: This is a digitally signed message part
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list


Re: [Mono-dev] [Mono-patches] r107198 - trunk/mcs/class/corlib/System

2008-07-04 Thread Andreas Nahr
> >  This is used thousands of times throughout the class libraries.
> > But in *THIS* case with void * it does not seem to work. So this was
> the easiest way to fix the problem.
> 
> Ok, I used a class and int as type. So here a test using an unsafe
> struct and void* as member, just like IntPtr does:
> 
> [EMAIL PROTECTED]:~$ cat test.cs
> using System;
> 
> unsafe class Test
> {
>   int value;
> 
>   Test(int value)
>   {
>   this.value = value;
>   }
> 
>   public static void Main ()
>   {
>   Test t = new Test(123);
>   Console.WriteLine(t.value);
> 
>   AnotherTest at = new AnotherTest(123);
>   Console.WriteLine((int) at.value);
>   }
> }
> 
> public unsafe struct AnotherTest
> {
>   internal void* value;
> 
>   public AnotherTest(int value)
>   {
>   this.value = (void*) value;
>   }
> }
> 
> [EMAIL PROTECTED]:~$ mono test.exe
> 123
> 123

Although it is interesting that this seems to work (by the way the problem was 
that I couldn't even compile it). I still do not see the use of showing a test 
that shows that there are situations in which it works. If you want to 
reproduce the problem then I suggest that you take the Version of IntPtr (it’s 
a small and simple type anyways) before my changes and then change the values 
accordingly and then you will hopefully see the problem.

Then reduce it to the smallest case possible.
Then you would have to analyze the reason for it.

That (which could easily take one or more days) then leads you to maybe fix a 
corner-case that probably nobody ever used (well exaggerating a little bit ;) 
except the IntPtr struct.

> >
> > Greetings Andreas
> >
> > P.S. If you want to make further "tests" maybe mail me private and
> not through the list to keep the noise low.
> 
> I don't think this is noise as you had issues with a possible compiler
> bug, so allow other developers to join the discussion and comment on
> the test-code.

I will not research any further here because imho it's just not worth my time 
and there are far more important things to fix. Feel free to do if you like.

___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list