Add RuntimeFeature

[terrajobst]

This adds the RuntimeFeature class which enables compilers to statically discover which features the runtime is guaranteed to support. The compiler checks for individual features by looking for the presence of a static field by a well-known name. By convention, these fields are of type string.

Please note that for .NET Standard this means that adding a field to this type requires all implementers to provide this feature in their runtimes. The only feature we currently have is portable PDBs, which (AFAIK) all runtimes already support.

@dotnet/nsboard

[joshpeterson]

What does it mean for a runtime to support portable PDBs? I'm worried that IL2CPP might not support this now, so I'd like more details about what is required.

[marek-safar]

I don't think this has runtime impact

[weshaggard]

I don't think this has runtime impact

From the coreclr perspective we needed to plumb in support to be able to read portable pdbs for diagnostics purposes to get stack traces. We actually do this in a light-up fashion and call into System.Reflection.Metadata, see dotnet/corefx#8670.

@jaredpar what kind of decisions does the compiler make if this feature is present?

[terrajobst]

@marek-safar

I don't think this has runtime impact.

By convention, by adding this type we sign up all current (ad future) runtimes to supports portable PDBs. In that sense it has runtime impact.

@jaredpar, could answer @weshaggard question? Are you blocking emitting portable PDBs if the reference assemblies don't contain this field?

joshpeterson

What does it mean for a runtime to support portable PDBs? I'm worried that IL2CPP might not support this now, so I'd like more details about what is required.

Have you had a chance looking into this?

[joshpeterson]

@terrajobst

What does it mean for a runtime to support portable PDBs? I'm worried that IL2CPP might not support this now, so I'd like more details about what is required.

Have you had a chance looking into this?

Yes, at the moment IL2CPP does not support portable PDBs in the runtime (we do for managed code debugging via VSTU though). I think we are fine with returning false here until (or if) we get support for this.

[terrajobst]

@joshpeterson

Unfortunately the presence of this field indicates that a runtime does have support for portable PDBs. If you guys can't support that, I'm inclined to not take this API then.

@jaredpar

How are you guys using this particular field?

[joshpeterson]

@terrajobst:

Could we do something like IsSupported("PortablePdb") instead? I don't think this is something we can easily implement though, and I don't see a need for it from our users.

[jkotas]

Unfortunately the presence of this field indicates that a runtime does have support for portable PDBs.

How do you define that runtime supports portable PDBs (or any PDBs), for full AOT runtime like Unity IL2CPP?

I think this feature does not really make sense in full AOT runtimes like Unity IL2CPP.

[joncham]

Unfortunately the presence of this field indicates that a runtime does have support for portable PDBs. I am confused. I would have expected this to have been used in the form

if (RuntimeFeature.IsSupported(RuntimeFeature.PortablePdb)
{
// ... do stuff needing portable pdbs
}

Is that not the case?

[joncham]

How do you define that runtime supports portable PDBs (or any PDBs), for full AOT runtime like Unity IL2CPP?

I think this feature does not really make sense in full AOT runtimes like Unity IL2CPP.

We do consume portable PDBs during the AOT process, and can use portable PDBs when producing information for the managed debugger. But I am not sure that is what purpose this API serves.

[jkotas]

This API is meant to be used by dynamically generated code - see https://github.com/dotnet/corefx/issues/19788 for justification. If you are running Roslyn inside your process for scripting, generate code that you immediately load and want to work well on old runtimes that do not support Portable PDBs, you can use this API to tell whether to generate Portable PDBs or not.

If the runtime does not support dynamic loading or reflection emit, RuntimeFeature.PortablePdb is irrelevant.

[terrajobst]

Let's step back. The RuntimeFeature type serves two purposes:

1. Dynamic discovery of runtime features. This is done by calling RuntimeFeature.IsSupported("some literal"). Useful for libraries doing dynamic stuff and being able to light up on later runtimes with more features.
2. Static discovery of runtime features. The presence of a field with a particular name means the runtime will have support for this. Useful for the compilers to discover whether certain runtime features are guaranteed to be supported that cannot be keyed off by the presence of APIs, such as whether the runtime will allow default implementations of interfaces.

The means code like RuntimeFeature.IsSupported(RuntimeFeature.SomeField) is non-nonsensical as the implementation would be obligated to return true, otherwise it shouldn't have declared the field in the first place. So IsSupported should only ever be used with literals. Which ones? Well, the documentation page renders the union of all fields across all runtimes so it's conveniently discoverable too.

So if we don't believe all runtimes will support portable PDBs, we should not add the field to the standard. We should still add the RuntimeFeature type with the IsSupported method though.

[joncham]

So if we don't believe all runtimes will support portable PDBs, we should not add the field to the standard.

It seems like we should never add a field to the standard, if it implies all .NET Standard compatible runtimes would be required to support those features?

To me, this is distinct from the current contract of exposing APIs which may or may not be implemented.

In this specific case, it is unclear to me which APIs involving portable pdbs are expected to work and under what conditions if the field is present.

[terrajobst]

It seems like we should never add a field to the standard, if it implies all .NET Standard compatible runtimes would be required to support those features?

Depends. For example, we decided that all runtimes must support generics now. I suspect other features (e.g. default implementations of interfaces) will be in the same camp.

To me, this is distinct from the current contract of exposing APIs which may or may not be implemented.

Correct. The point of this type is to allow the compiler to guard against runtime failures due to use of unsupported features. Developers can guard against APIs that throw PlatformNotSupportedException but they can't guard against the runtime refusing to JIT methods or load assemblies due to missing type system features as the behavior is usually undefined.

In this specific case, it is unclear to me which APIs involving portable pdbs are expected to work and under what conditions if the field is present.

If the field is present, it would mean that the runtime would load a PDB in portable format and use the information for debugging or when rending line number information in the exception. If the field is missing, it means a compliant runtime may or may not support this scenario.

[gulshan]

I think there should be runtime/CLR standards like current dotnet(BCL) standards instead of runtime feature detection. And then different language features and BCL standards should be defined on top of a certain CLR standard. Quoting myself from another discussion #682 (comment) -

What I think regarding .net standard built on CLR capabilities is, there would be several standards of CLR, each being subset of the later, like current .net standard for BCL. There can be a tiny CLR with no reflection, ref counting gc and other reduced capabilities. There can be multiple CLR standards covering IoT devices, phones, desktop/laptops, servers/cloud, browser/wasm, bootstrapping etc scenarios. In compiler, language features then have to target a specific CLR standard (already kind of happening for default interface implementations). And BCL APIs also have to target a specific CLR standard(and available language features on that standard). Then new language features and BCL APIs can be developed targeting existing CLR standards and all these new codes using these new language/BCL features will run on existing CLR implementations without any problem. When some language feature will need new CLR feature, it will be coded in a new version of CLR standard. All the platform interfacing like file system, networking, UI will be based on CLR standards. Then coreCLR, mono and other CLR implementations will just implement CLR standards and same Roslyn and coreFX will work on top of them. When some feature like Span is not working on Xamarin or UWP, it's because the required CLR standard is not implemented there. Not because it is not yet implemented in their BCL.

[terrajobst]

I think there should be runtime/CLR standards

How would a library express what the minimum requirement would be? Currently, the managed compilers don't even know the target framework -- they just get passed a bunch of assemblies and the source you want to compile against it. It seems logical to me to model runtime features as APIs because it allows us to track it like any other feature in .NET Standard.

Also, runtimes are currently not something you can model as concentric circles. You could argue that they should but AOT and JIT cannot be expressed as subset/supersets.

[terrajobst]

@jkotas any thoughts on this? Would love to get this approved by you :-)

[jkotas]

RuntimeFeature.IsSupported is useful.

The usefulness of RuntimeFeature.PortablePdb run its course, and it is not very useful anymore since all runtimes that support RuntimeFeature API do support Portable PDBs as well. However, I think it does not hurt to add it for completeness and parity with .NET Core.

[terrajobst]

Sweet. Given your comment and @joncham comments above

We do consume portable PDBs during the AOT process, and can use portable PDBs when producing information for the managed debugger. But I am not sure that is what purpose this API serves.

It seems we can at least leave the field in as well for consistency.