TypeInfo currently doesn't expose a parameterless constructor. We have several types in our reference assemblies which should be inheriting from TypeInfo but are unable to do so because there is no accessible constructor.
public abstract class TypeInfo
{
protected TypeInfo() {}
}
@weshaggard
This is needed to bring System.Reflection.Context up now as we have an ongoing PR blocked on this.
Also, the following items which we'd like to do in the future (and want to prototype in 2.2 timeframe) will be hampered by this:
https://github.com/dotnet/corefx/issues/4491 (System.Reflection.Emit.AssemblyBuilder.Save)
https://github.com/dotnet/corefx/issues/2800 (Assembly.ReflectionOnlyLoad replacement)
Marking as ready for review for @weshaggard and @terrajobst to take a look.
@joperezr can you please add the formal API on the top?
There isn't a formal API review yet as we have to explore options. Currently I think there are two options:
1) Give InternalsVisibleTo System.Reflection.Emit contracts. This is what we did before see https://github.com/dotnet/corefx/blob/master/src/System.Reflection/ref/System.Reflection.Manual.cs#L14.
2) Expose the default constructor as public.
However the trouble with either of those options is that it will not allow us to have a version of Reflection.Emit that builds on top of netstandard2.0 because either of those will exist in netstandard.dll so we are going to have to think about this some more and come up with some other creative/hacky solution.
In that case moving back to api-needs-work (@joperezr please do not move it to "ready-for-review", until there is solid API proposal you as owner reviewed and approved ;-))
Let's just do [1] (InternalsVisibleTo) for now, then.
@AtsushiKan System.Reflection.Context contains one type DelegatingType which inherits from TypeInfo and needs the public ctor. Any guidelines what to do here? Should we still use InternalsVisibleTo here or think again about exposing the ctor?
@jkotas I noticed you removed the InternalsVisibleTo attributes last month from System.Reflection. I would need to bring it back to access the internal TypeInfo constructor for S.R.Context which I try to port over. Any suggestions how to work around this?
I'd just as soon make this constructor protected and be done with it. For S.R.C specifically, NETFX can presumably continue to use its own version. But not having this constructor is going to hamper other Reflection work in the future.
I agree with Ati.
Also, I think S.R.C should be its own package in the compat pack. I do not think we would want the compat pack to be depending on netcoreapp internals.
I like making the constructor public as well but we cannot do it in the netstandard ref assembly because it isn't public in netfx. It will have to be netcoreapp specific which means that all the dependencies on it will also have to be netcorepp specific and cannot target netstandard. So we should see how much of a problem this is for the things that depend on it.
It will have to be netcoreapp specific which means that all the dependencies on it will also have to be >netcorepp specific and cannot target netstandard.
dotnet/runtime#15704 (System.Reflection.Emit.AssemblyBuilder.Save)
dotnet/runtime#15033 (Assembly.ReflectionOnlyLoad replacement)
These two items are things we'll likely be prototyping in CoreFxLab during the 2.2 timeframe but aren't expected to ship in 2.2 for now but in something afterward. There, I see two ways to move:
TypeInfo in them (just Type) and hope that people using them to convert old code won't mind us telling them to get rid of GetTypeInfo() after we told to them spend all that time inserting GetTypeInfo().Anyway, I don't see a way to make those things work on top of NETFX any other way.
what about exposing the TypeInfo ctor in netfx as well?
We could in a future version of netfx but that doesn't really help us much with shipping a library that needs to work on the existing version.
Let's start doing this in .NET Core. This doesn't help the RefEmit case for .NET Standard, but we gonna start somewhere :-)
Can we try to port this change to netfx vnext and mono vnext so that we can include it in netstandard vnext so that we can target that version for our OOB package, i.e. System.Reflection.Context?
We could in a future version of netfx but that doesn't really help us much with shipping a library that needs to work on the existing version.
I don't see the need to support netstandard2.0 for that particular package, vnext is just fine.