none
Private Property Setter Writable??? RRS feed

  • Question

  • Hello!

     

    I don't know if this is a design decision, but with the advent of accessor-modifier the behavior of the methods "RuntimePropertyInfo::SetValue(object obj, object value, object[] index)" and "RuntimePropertyInfo::GetValue(object obj, object[] index)" should be changed, no?

     

    The "RuntimePropertyInfo::SetValue" and the "RuntimePropertyInfo::GetValue" uses the "BindingFlags.NonPublic" to invoke the corresponding method. This is diferent from the behavior of "FieldInfo::SetValue", for example. Note that I'm not passing the "BindingFlags" to the method, so I expect the behavior as "BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance" or "BindingFlags.Default".

     

    Is this a bug or a design decision? If this is a design decision, why?

     

    Thanks...

    Sunday, May 4, 2008 3:11 PM

Answers

  • PropertyInfo is an abstract class, you'd always work with a RuntimePropertyInfo instance.  PropertyInfo.SetValue() is just a default implementation, you can never call it.  The BindingFlags were relevant when you got an instance of RuntimePropertyInfo with Type.GetProperty().  And it observed the accessibility of the property, not the getter and setter.  RuntimePropertyInfo.SetValue() now makes sure that the PropertyInfo you got will work, regardless of the accessibility of the setter method.

    So it is the accessibility of the property that counts.  That's the way it worked in .NET 1.1 and that's the way it still works.  The compiler enforces property getter/setter accessibility, Reflection doesn't.  I don't see a problem.  Since the SetValue(object, object, object[]) overload always works, there isn't any reason to pass BindingFlags at all.
    Sunday, May 4, 2008 6:52 PM
    Moderator
  • This is starting to go around in circles.  There is an unavoidable friction between the group that designs the feature and makes it works vs the group that designs the API to use the feature.  The property feature was a tough one and gave the C# group many headaches.  They are still tinkering with it, C# V3.0 has yet another property syntax feature.

    Back at .NET 1.0 and C# 1.0, there wasn't a real big problem.  C# access modifiers were only available on the property, not at the getter/setter level.  Which gave the API designers a great opportunity to make PropertyInfo behave like FieldInfo, MethodInfo etc.  Not sure who spec-ed it first.  From what I see, I'd say the API group was first, the C# group followed suit.

    No doubt by overwhelming demand from customers, they decided that perhaps that wasn't a good idea.  It wasn't.  So C# 2.0 acquired access modifiers at the getter/setter level.  But the API was already set in stone.  And they came up with an API implementation that is logically consistent, but not "coherent" with the actual implementation.

    I'd have to say: "job well done".  It is logically consistent to me.  I don't see how it prevents me from doing anything using Reflection, other than giving me slight pause at what BindingFlags to pass.  You can post to the Connect site to express your dismay but I can guarantee you they'll close the issue right away with "by design".  To quote the old commercial: "where's the beef?"
    Sunday, May 4, 2008 10:43 PM
    Moderator

All replies

  • Not quite sure what you're driving at.  The C# V2.0 ability to use an access modifier on the getter and setter in addition to the one on the property itself is a convenience feature that is not directly supported by the CLS, just the compiler.  The PropertyInfo class was probably the sticky problem, it can only represent the accessibility of the property.  MSFT's decision not to modify the PropertyInfo and RuntimePropertyInfo classes is, arguably, a reasonable one, it would break too much existing code.  And be non-CLSCompliant.
    Sunday, May 4, 2008 4:50 PM
    Moderator
  • @nobugz, I think you're right that the problem lies with modifying the PropertyInfo class.  However, the property accessor with the different accessibility modifier is emitted into the IL, it's not entirely a compiler-enforced feature.  So if it's not part of CLS, then emitting a property that has e.g. a private set_Prop method but a public get_Prop method is already non-compliant, isn't it?
    Sunday, May 4, 2008 5:18 PM
  • They dodged that bullet because accessibility is not enforced by the CLR, only compilers can enforce it.  A CLS compliant language compiler would use PropertyInfo to discover the accessibility of the property.  I imagine the V2.0 C# compiler uses MethodInfo on the getter and setter methods as well.
    Sunday, May 4, 2008 6:04 PM
    Moderator
  • I know that changing things at this time will break many existing apps... I guess that the people at Microsoft forgot about that... take the following code:

     

    public class A

    {

    private string __someProp;

    private string SomeProp

    {

    get

    {

    return (this.__someProp);

    }

    set

    {

    this.__someProp = value;

    }

    }

    }

     

    The code above will compile fine since .NET 1.0...

     

    Now make the following test...

     

    A a = new A();

    Type t = a.GetType();

    PropertyInfo pi = t.GetProperty("SomeProp");

    pi.SetValue(a, "Testing", null);

     

    You will not aquire a reference to the PropertyInfo and the code will fail...

     

    Now take a look at the metadata definition and IL code of SomeProp...

     

    .property instance string SomeProp()

    {

    .set instance void LinqTests.A:Tongue Tiedet_SomeProp(string)

    .get instance string LinqTests.A::get_SomeProp()

    }

     

    .method private hidebysig specialname instance void

    set_SomeProp(string 'value') cil managed

    {

    .maxstack 8

    IL_0000: nop

    IL_0001: ldarg.0

    IL_0002: ldarg.1

    IL_0003: stfld string LinqTests.A::__someProp

    IL_0008: ret

    }

     

    .method private hidebysig specialname instance string

    get_SomeProp() cil managed

    {

    .maxstack 1

    .locals init ([0] string CS$1$0000)

    IL_0000: nop

    IL_0001: ldarg.0

    IL_0002: ldfld string LinqTests.A::__someProp

    IL_0007: stloc.0

    IL_0008: br.s IL_000a

    IL_000a: ldloc.0

    IL_000b: ret

    }

     

    Take a very close look at the metadata definition of SomeProp... The private modifier is not defined there, the private modifier is defined at the getter and setter methods...

     

    Type.GetProperty uses the following BindingFlags: BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance to aquire a PropertyInfo reference...

     

    Ok, this is correct since I didn't specified the BindingFlags...

     

    Now take a look at the following code

     

    public class A

    {

    private string __someProp;

    public string SomeProp

    {

    get

    {

    return (this.__someProp);

    }

    private set

    {

    this.__someProp = value;

    }

    }

    }

     

    Now the setter is private...

     

    Let's test...

     

    A a = new A();

    Type t = a.GetType();

    PropertyInfo pi = t.GetProperty("SomeProp");

    pi.SetValue(a, "Testing", null);

    Console.WriteLine("SomeProp = {0}\nCanRead = {1}\nCanWrite = {2}\n", a.SomeProp, pi.CanRead, pi.CanWrite);

     

    In the above code I can get a reference to the PropertyInfo, because the getter method is public, but the setter method is not! The code above will run smoothly...

     

    The following code will not even compile...

     

    A b = new A();

    b.SomeProp = "Testing"; // Fail

     

    Take a look at the metadata definition and IL code of SomeProp now...

     

    .property instance string SomeProp()

    {

    .set instance void LinqTests.A:Tongue Tiedet_SomeProp(string)

    .get instance string LinqTests.A::get_SomeProp()

    }

     

    .method private hidebysig specialname instance void

    set_SomeProp(string 'value') cil managed

    {

    .maxstack 8

    IL_0000: nop

    IL_0001: ldarg.0

    IL_0002: ldarg.1

    IL_0003: stfld string LinqTests.A::__someProp

    IL_0008: ret

    }

     

    .method public hidebysig specialname instance string

    get_SomeProp() cil managed

    {

    .maxstack 1

    .locals init (string V_0)

    IL_0000: nop

    IL_0001: ldarg.0

    IL_0002: ldfld string LinqTests.A::__someProp

    IL_0007: stloc.0

    IL_0008: br.s IL_000a

    IL_000a: ldloc.0

    IL_000b: ret

    }

     

    This is a unexpected behavior... Getting a look at the PropertyInfo:Tongue TiedetValue(object, object, object[]) I found this:

     

    [DebuggerHidden, DebuggerStepThrough]
    public virtual void SetValue(object obj, object value, object[] index)
    {
        this.SetValue(obj, value, BindingFlags.Default, null, index, null);
    }


     

    Ok this is good! BindingFlags.Default... Non public access... but looking into RuntimePropertyInfo:Tongue TiedetValue(object, object, object[])  I found this:

     

    [DebuggerStepThrough, DebuggerHidden]
    public override void SetValue(object obj, object value, object[] index)
    {
        this.SetValue(obj, value, BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance, null, index, null);
    }

    Oops...
    As you can see RuntimePropertyInfo uses a diferent BindingFlags... Why? What is the purpose for that since RuntimePropertyInfo has a SetValue(object, object, BindingFlags, Binder, object[], CultureInfo) that can be used for this purpose?
    Thanks
    Sunday, May 4, 2008 6:18 PM
  • PropertyInfo is an abstract class, you'd always work with a RuntimePropertyInfo instance.  PropertyInfo.SetValue() is just a default implementation, you can never call it.  The BindingFlags were relevant when you got an instance of RuntimePropertyInfo with Type.GetProperty().  And it observed the accessibility of the property, not the getter and setter.  RuntimePropertyInfo.SetValue() now makes sure that the PropertyInfo you got will work, regardless of the accessibility of the setter method.

    So it is the accessibility of the property that counts.  That's the way it worked in .NET 1.1 and that's the way it still works.  The compiler enforces property getter/setter accessibility, Reflection doesn't.  I don't see a problem.  Since the SetValue(object, object, object[]) overload always works, there isn't any reason to pass BindingFlags at all.
    Sunday, May 4, 2008 6:52 PM
    Moderator
  • "The BindingFlags were relevant when you got an instance of RuntimePropertyInfo with Type.GetProperty().  And it observed the accessibility of the property, not the getter and setter."

     

    This is the point... The property itself does not have an accessibility modifier.. take a look at the IL level... Who has the accessibility modifier is the setter and getter methods... That makes RuntimeType.GetPropertyImpl look for some accessor using the specified BindingFlags (in this case BindingFlags.Default). If it finds one then a reference to a PropertyInfo will be returned. When looking for a property the BindingFlags.NonPublic is not considered, but when executing one of it's accessors it is? Oops!

     

    The default behavior of RuntimePropertyInfo.SetValue(object, object, object[]) should be BindingFlags.Default. I know that I can use the method signature that takes this into account, but if you compare this behavior to the others on the System.Reflection namespace you will see that this behavior is different. And is this difference that I'm curious to know about. Is this a bug or what decision was strong enough to the development of this behavior? The compiler team has nothing to do with that. This is code produced by the Class Library Team...

     

    Can you understand me? Can you see the whole thing? Where is the simmetry?

     

    "Since the SetValue(object, object, object[]) overload always works, there isn't any reason to pass BindingFlags at all."

     

    In fact "the working method" SetValue is not coherent with the others... If I didn't pass a BindingFlag then the default behavior must be BindingFlags.Default... Look for the other methods in the class library... Can you understand that?

    Sunday, May 4, 2008 8:44 PM
  • This is starting to go around in circles.  There is an unavoidable friction between the group that designs the feature and makes it works vs the group that designs the API to use the feature.  The property feature was a tough one and gave the C# group many headaches.  They are still tinkering with it, C# V3.0 has yet another property syntax feature.

    Back at .NET 1.0 and C# 1.0, there wasn't a real big problem.  C# access modifiers were only available on the property, not at the getter/setter level.  Which gave the API designers a great opportunity to make PropertyInfo behave like FieldInfo, MethodInfo etc.  Not sure who spec-ed it first.  From what I see, I'd say the API group was first, the C# group followed suit.

    No doubt by overwhelming demand from customers, they decided that perhaps that wasn't a good idea.  It wasn't.  So C# 2.0 acquired access modifiers at the getter/setter level.  But the API was already set in stone.  And they came up with an API implementation that is logically consistent, but not "coherent" with the actual implementation.

    I'd have to say: "job well done".  It is logically consistent to me.  I don't see how it prevents me from doing anything using Reflection, other than giving me slight pause at what BindingFlags to pass.  You can post to the Connect site to express your dismay but I can guarantee you they'll close the issue right away with "by design".  To quote the old commercial: "where's the beef?"
    Sunday, May 4, 2008 10:43 PM
    Moderator