Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C# 9 "init-only" properties; unable to detect IsExternalInit via reflection API - and a pragmatic proposal #43088

Closed
mgravell opened this issue Oct 6, 2020 · 6 comments
Labels
area-System.Reflection untriaged New issue has not been triaged by the area owner

Comments

@mgravell
Copy link
Member

mgravell commented Oct 6, 2020

(note: if this is possible and I've just overlooked it; please do tell me!)

The current "init-only" implementation is nice in some ways, as existing libraries (e.g. serialization etc) will "just work", since SetValue (and the equivalent in IL) just work; however, one weakness is that it seems impossible for libraries that want to respect the "init-only"-ness to actually detect it; if we consider, for example:

class Foo
{
    public int A { get; init; }
}

or

record Bar(int B);

this gets compiled as:

.method public hidebysig specialname 
        instance void modreq(System.Runtime.CompilerServices.IsExternalInit) set_A (
            int32 'value'
        ) cil managed 
    {
        .custom instance void [System.Private.CoreLib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
            01 00 00 00
        )

i.e. with the modreq on the method.

The problem with this is that no existing reflection API allows you to detect this feature. PropertyInfo has GetOptionalCustomModifiers() / GetRequiredCustomModifiers(), but MethodInfo does not, and since the modreq is on the setter, it is not inspectable. All we can detect is attributes, i.e. [CompilerGenerated].

This makes it impossible for the type to be understood properly be reflection-based code.

I wonder if there are some pragmatic things that could be done here; for example, adding an attribute as well; obvious contenders might be [ReadOnly(true)] on the property (or perhaps type, in the record case?). There is a small problem here; the regular caller might also have added this attribute, since the following is valid:

[ReadOnly(true)]
class Foo
{
    [ReadOnly(true)]
    public int A { get; [ReadOnly(true)] init; }
}

but maybe something similar? Maybe a new attribute that we can check for, that is declared in tandem with IsExternalInit?


An alternative would be to add some new APIs; either:

  • PropertyInfo.IsExternalInit (very specific; also, naming is hard)
  • MethodInfo.GetOptionalCustomModifiers() / MethodInfo.GetRequiredCustomModifiers() (larger change, more reusable)

The downside of the "new API" approach is that it doesn't work on older TFMs that have hacked themselves an IsExternalInit

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Reflection untriaged New issue has not been triaged by the area owner labels Oct 6, 2020
@mgravell mgravell changed the title C# 9 "init-only" properties; unable to detect via reflection API - and a pragmatic proposal C# 9 "init-only" properties; unable to detect IsExternalInit via reflection API - and a pragmatic proposal Oct 6, 2020
@mgravell
Copy link
Member Author

mgravell commented Oct 6, 2020

Note: an advantage of [ReadOnly(true)] specifically is that it works with the TypeDescriptor model; ReflectPropertyDescriptor explicitly checks for this attribute on the property (not the accessor).

@mgravell
Copy link
Member Author

mgravell commented Oct 6, 2020

Additional workaround idea: the compiler could emit a modopt of IsExternalInit on the property (as well as the modreq on the setter); an API to check for modopt on already exists on PropertyInfo, and down-level compilers are allowed (indeed, recommended) to ignore a modopt that they don't understand. This requires zero additional APIs, but has more IL per property.

@jkotas
Copy link
Member

jkotas commented Oct 6, 2020

Try this:

using System;

public class Foo
{
    public int A { get; init; }
}

class Program
{
    static void Main(string[] args)
    {
        foreach (var m in typeof(Foo).GetMethod("set_A").ReturnParameter.GetRequiredCustomModifiers())
            Console.WriteLine(m);
    }
}

Prints "System.Runtime.CompilerServices.IsExternalInit"

@mgravell
Copy link
Member Author

mgravell commented Oct 6, 2020

@jkotas well ... fair enough; TIL - thanks; apparently we can detect it, and all is well with the world; not obvious, but it works

@mgravell mgravell closed this as completed Oct 6, 2020
@jkotas
Copy link
Member

jkotas commented Oct 6, 2020

Agree that it is not obvious. modreqs/modopts can be only attached to a type in the method signature. So the way to attach them to the method is to attach them to the method return type in the method signature instead.

@mgravell
Copy link
Member Author

mgravell commented Oct 6, 2020

makes sense now, thanks; have this detection working in a library branch now - minimal changes (protobuf-net now correctly handles unattributed positional record types, yay! protobuf-net/protobuf-net@1488599)

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Reflection untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

3 participants