Add reflection emit

[terrajobst]

This adds reflection emit APIs. We should consider also adding a capability API. This fixes #712.

@dotnet/nsboard

[JonathanHUnity]

@joshpeterson Will this pose problems for Unity developers, where most platforms do not support Reflection.Emit?

[terrajobst]

That's a great question. Our thinking was that we should consider this as an API that might throw if the runtime environment doesn't support runtime code gen. And for this we should offer a capability APIs so that developers can write code like this:

if (IsRefEmitAvailable())
{
    EmitAndRun();    
}
else
{
    Interpret();
}

[joshpeterson]

@terrajobst Would IsRefEmitAvailable be implemented with a "runtime feature" as in #804?

[joshpeterson]

I have some concerns about adding types from System.Reflection.Emit to .NET Standard. At the moment, Unity can support all of .NET Standard 2.0 on all[1] of its platforms. With this change, that will no longer be the case.

Unity supports a number of AOT-only platforms, including iOS, consoles, some VR devices. For these platforms, System.Reflection.Emit cannot work. At the moment (Unity version 2018.2), Unity exposes two profiles (both on AOT-only and JIT/AOT platforms):

1. .NET 4.x (which matches .NET 4.7.1)
2. .NET Standard 2.0

We are providing the following guidance to Unity users about which profile to choose:

Library authors: Use .NET Standard 2.0, since all of it is supported on all Unity platforms. Your library code can run everywhere.

Game developers: You have two options, get errors about unsupported API usage at compile time or at run time.

• For compile time errors, use .NET Standard 2.0. If it compiles, it will work.
• For run time errors, use .NET 4.x. Any API usage will compile, but some might fail at run time on some platforms. If you have have run time checks to avoid the code you know is not implemented on a given platform, you should be fine.

This pull request would require Unity to throw a PlatformNotSupportedException for these APIs on AOT-only platforms, which would eliminate this nice compile time/run time choice users have[2]. Effectively, users could have no compile time guarantees that the APIs they use are supported.

So minimally, I would like to see something like IsRefEmitAvailable implemented, so that users can make a run time decision without relying on the need to catch an exception. But ideally I'd prefer not to merge this into .NET Standard.

[1] For this discussion, let's ignore Unity's WebGL platform (effectively IL -> C++ -> asm.js/Wasm), since it is pretty restricted and does not support threads or sockets.

[2] If I'm being entirely honest, users don't really have this nice distinction now, because even .NET Standard 2.0 on WebGL has unsupported APIs. Since this is a single platform with well-known restrictions which might be lifted in the future, I think it is acceptable to ignore it as an outlier that does not impact the general principle.

[weshaggard]

Few things:

1. You should note that this isn't all the Ref.Emit APIs, it is only the set in .NET Core and things like save to file aren't supported there currently.
2. If we do decide to add these APIs I think we should include a capability API to detect whether or not they will work.
3. I'm curious to get @jkotas take on adding these to the standard.

[jkotas]

I'm curious to get @jkotas take on adding these to the standard.

It reduces netstandard value for environments without JIT, as @joshpeterson explained. This is the first example of where adding API helps some, but hurts others.

I do not have a strong opinion either way. I agree that there has to be the capability API if this were to be added.

[davidfowl]

I personally think it helps more than it hurts. AOT has always had these warts and all that is needed to make this viable is a capability API that allows libraries to pick a different code generation strategy. Many systems have varying code generation techniques and .NET’s 15+ year legacy has reflection emit at the heart of a certain class of library (serialization etc).

Do an API scan of on nuget.org and see how many libraries use it transitively or non transitively.

[joshpeterson]

I agree, with a capability API, I think this will be fine.

[jskeet]

Fine in general - just one question that probably affects lots of PRs.

[jskeet]

ReturnParameter is virtual in MethodInfo - why do we need to specify here that it's overridden?

(This is just an example I happened upon due to being puzzled by the name. I suspect there are others.)

It feels reasonable to specify an override for abstract members (e.g. the Attributes property) but if it's just a virtual member and we're not specifying any additional attributes, does the fact that it's overridden need to be part of the standard? That feels like an implementation detail.

[jskeet]

(Unless this is to signify that any conformant implementation must override it because the specified behavior of the base implementation is different to the specified behavior of the derived implementation? Do we have a document listing rules for this sort of thing?)

[joshpeterson]

Approving assuming #832 will be addressed as well.

[terrajobst]

Given we have a commitment to do https://github.com/dotnet/corefx/issues/29258 and subsequently #832, we can remove the netstandard-api-needs-work label.