Runtime: System.Runtime.CompilerServices.Unsafe can't be used in corefx shared framework implementation

[stephentoub]

The Unsafe class can be used in corelib, and it can be used in packages in corefx that sit above netstandard. But libraries in the netcoreapp implementation can't use it, because although we have a netcoreapp build (https://github.com/dotnet/corefx/blob/master/src/System.Runtime.CompilerServices.Unsafe/src/Configurations.props#L11), System.Runtime.CompilerServices.Unsafe.dll is not currently included in-box in netcoreapp.

[jkotas]

Some of the workarounds we have because of this:

• System.Text.Json: https://github.com/dotnet/corefx/pull/33216#discussion_r231576744
• System.Runtime.Serialization.Formatters: https://github.com/dotnet/corefx/pull/33466#discussion_r233172933

[ahsonkhan]

System.Runtime.CompilerServices.Unsafe.dll is not currently included in-box in netcoreapp.

As far as I am aware, putting it in-box would freeze the API surface. Are we OK with that?

It looks like we still need to add APIs to it:
https://github.com/dotnet/corefx/commits/master/src/System.Runtime.CompilerServices.Unsafe
https://github.com/dotnet/corefx/issues?q=is%3Aopen+is%3Aissue+label%3Aarea-System.Runtime.CompilerServices+label%3Aapi-needs-work

cc @ericstj, @joperezr, @safern, @GrabYourPitchforks

[jkotas]

As far as I am aware, putting it in-box would freeze the API surface. Are we OK with that?

It would not freeze the API surface as long as it depends on netcoreapp only and avoids taking direct dependency on CoreLib, etc.

System.Reflection.Immutable or System.Reflection.Metadata are on the same plan.

[safern]

Yes we could have a netcoreapp and a netstandard reference assembly. The netstandard one would be frozen because it has to match the first netcoreapp version that contains the implementation as inbox and the netcoreapp and uap refs could keep growing as long as we have one reference assembly for them. System.Data.SqlClient does the same thing.

[jkotas]

System.Reflection.Immutable or System.Reflection.Metadata are included inbox and their netstandard surface is not frozen. What prevents us from doing the same thing here?

[safern]

System.Reflection.Immutable or System.Reflection.Metadata are included inbox and their netstandard surface is not frozen. What prevents us from doing the same thing here?

That seems broken, no? What if someone has a library they built for netstandard with a new API that was added after we shipped .netcoreapp2.1 and reference one of those packages, using the new API. Then a 2.1 console app references this netstandard library, it will fail at runtime because it will not find the implementation for that guy in the netcoreapp2.1 inbox assembly, am I missing something?

[ericstj]

Correct, the only thing that will freeze this is if we put it inbox and remove the applicable implementation from the package.

I think when I had talked to @ahsonkhan about this he was mentioning that the logical thing to do when putting it inbox was to have it forward to core lib, in which case we wouldn't want to keep that in the package. If folks are ok having it inbox with a different implementation than core lib then we can do that and treat it like SRM and SCI.

[ericstj]

am I missing something?

Yeah, if the package is newer it will have an implementation assembly that applies to netcoreapp2.1 that will win over the inbox version.

[safern]

Yeah, if the package is newer it will have an implementation assembly that applies to netcoreapp2.1 that will win over the inbox version.

Oh right, :smile:

[jkotas]

having it inbox with a different implementation than core lib then we can do that and treat it like SRM and SCI

Yes, having it inbox with a different implementation than corelib is just fine.

[ericstj]

I'll pick this up. Multiple folks need this inbox and we don't want them building against corelib to make that happen. I'll also cleanup the existing projects that took a corelib dependency just for this.

[stephentoub]

Thank you, Eric.

[vancem]

Logging the fact that System.Diagnotics.DiagnosticSource want to use this. See
https://github.com/dotnet/corefx/issues/34828#issuecomment-466467573
for more.

[ahsonkhan]

Fixed in https://github.com/dotnet/corefx/pull/35526, thanks @ericstj