diff --git a/docs/Target Framework Strategy.md b/docs/Target Framework Strategy.md new file mode 100644 index 0000000000000..31a14019a4b1e --- /dev/null +++ b/docs/Target Framework Strategy.md @@ -0,0 +1,79 @@ +Target Framework Strategy +=== + +# Layers +The roslyn repository produces components for a number of different products that push varying ship and TFM constraints on us. A summary of some of our dependencies are : + +- Build Tools: requires us to ship compilers on `net472` +- .NET SDK: requires us to ship compilers on current servicing target framework (presently `net6.0`) +- Source build: requires us to ship `$(NetCurrent)` and `$(NetPrevious)` in workspaces and below (presently `net8.0` and `net7.0` respectively) +- Visual Studio: requires us to ship `net472` for base IDE components and `net6.0` for private runtime components. + +It is not reasonable for us to take the union of all TFM and multi-target every single project to them. That would add several hundred compilations to any build operation which would in turn negatively impact our developer throughput. Instead we attempt to use the TFM where needed. That keeps our builds smaller but increases complexity a bit as we end up shipping a mix of TFM for binaries across our layers. + +# Require consistent API across Target Frameworks +It is important that our shipping APIs maintain consistent API surface area across target frameworks. That is true whether the API is `public` or `internal`. + +The reason for `public` is standard design pattern. The reason for `internal` is a combination of the following problems: + +- Our repository makes use of `InternalsVisibleTo` which allows other assemblies to directly reference signatures of `internal` members. +- Our repository ships a mix of target frameworks. Typically workspaces and below will ship more recent TFMs than the layers above it. Compiler has to ship newer TFM for source build while IDE is constrained by Visual Studio's private runtime hence adopts newer TFM slower. +- Our repository invests in polyfill APIs to make compiling against multiple TFMs in the same project a seamless experience. + +Taken together though this means that our `internal` surface area in many cases is effectively `public` when it comes to binary compatibility. For example a consuming project can end up with `net7.0` binaries from workspaces layer and `net6.0` binaries from IDE layer. Because there is `InternalsVisibleTo` between these binaries the `internal` API surface area is effectively `public`. This requires us to have a consistent strategy for achieving binary compatibility across TFM combinations. + +Consider a specific example of what goes wrong when our `internal` APIs are not consistent across TFM: + +- Workspaces today targets `net6.0` and `net7.0` and it contains our `EnumerableExtensions.cs` which polyfills many extensions methods on `IEnumerable`. In `net7.0` the `Order` extension method is not needed because it was put into the .NET core libraries. +- Language Server Protocol targets `net6.0` and consumes the `Order` polyfill from Workspaces + +Let's assume for a second that we `#if` the `Order` method such that it's not present in `net7.0`. Locally this all builds because we compile the `net6.0` versions against each other so they're consistent. However if an external project which targets `net7.0` and consumes both Workspaces and Language Server Protocol then it will be in a broken state. The Protocol binary is expecting Workspaces to contain a polyfill method for `Order` but it does not since it's at `net7.0` and it was `#if` out. As a result this will fail at runtime with missing method exceptions. + +This problem primarily comes from our use of polyfill APIs. To avoid this we employ the following rule: + +> When there is a `#if` directive that matches the regex `#if !?NET.*` that declares a non-private member, there must be an `#else` that defines an equivalent binary compatible symbol + +This comes up in two forms: + +## Pattern for types +When creating a polyfill for a type use the `#if !NET...` to declare the type and in the `#else` use a `TypeForwardedTo` for the actual type. + +Example: + +```csharp +#if NET6_0_OR_GREATER + +using System.Runtime.CompilerServices; + +#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting forwarder for an internal polyfill API) +[assembly: TypeForwardedTo(typeof(IsExternalInit))] +#pragma warning restore RS0016 // Add public types and members to the declared API + +#else + +using System.ComponentModel; + +namespace System.Runtime.CompilerServices +{ + internal static class IsExternalInit + { + } +} + +#endif +``` + +## Pattern for extension methods +When creating a polyfill for an extension use the `#if NET...` to declare the extension method and the `#else` to declare the same method without `this`. That will put a method with the expected signature in the binary but avoids it appearing as an extension method within that target framework. + +```csharp +#if NET7_0_OR_GREATER + public static IOrderedEnumerable Order(IEnumerable source) where T : IComparable +#else + public static IOrderedEnumerable Order(this IEnumerable source) where T : IComparable +#endif +``` + + + + diff --git a/src/Compilers/Core/Portable/InternalUtilities/CompilerFeatureRequiredAttribute.cs b/src/Compilers/Core/Portable/InternalUtilities/CompilerFeatureRequiredAttribute.cs index f2853f1216e96..435972378b197 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/CompilerFeatureRequiredAttribute.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/CompilerFeatureRequiredAttribute.cs @@ -5,7 +5,15 @@ // Copied from: // https://github.com/dotnet/runtime/blob/fdd104ec5e1d0d2aa24a6723995a98d0124f724b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/CompilerFeatureRequiredAttribute.cs -#if !NET7_0_OR_GREATER +#if NET7_0_OR_GREATER + +using System.Runtime.CompilerServices; + +#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting forwarder for an internal polyfill API) +[assembly: TypeForwardedTo(typeof(CompilerFeatureRequiredAttribute))] +#pragma warning restore RS0016 // Add public types and members to the declared API + +#else namespace System.Runtime.CompilerServices { diff --git a/src/Compilers/Core/Portable/InternalUtilities/EnumerableExtensions.cs b/src/Compilers/Core/Portable/InternalUtilities/EnumerableExtensions.cs index c0f36f27557da..bceb437269c13 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/EnumerableExtensions.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/EnumerableExtensions.cs @@ -513,12 +513,14 @@ public static IOrderedEnumerable OrderByDescending(this IEnumerable sou return source.OrderByDescending(Comparer.Create(compare)); } -#if !NET7_0_OR_GREATER +#if NET7_0_OR_GREATER + public static IOrderedEnumerable Order(IEnumerable source) where T : IComparable +#else public static IOrderedEnumerable Order(this IEnumerable source) where T : IComparable +#endif { return source.OrderBy(Comparisons.Comparer); } -#endif public static IOrderedEnumerable ThenBy(this IOrderedEnumerable source, IComparer? comparer) { diff --git a/src/Compilers/Core/Portable/InternalUtilities/InterpolatedStringHandlerArgumentAttribute.cs b/src/Compilers/Core/Portable/InternalUtilities/InterpolatedStringHandlerArgumentAttribute.cs index de98ee7c33a83..77690b8c5140c 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/InterpolatedStringHandlerArgumentAttribute.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/InterpolatedStringHandlerArgumentAttribute.cs @@ -2,7 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#if !NET6_0_OR_GREATER +#if NET6_0_OR_GREATER + +using System.Runtime.CompilerServices; + +#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting forwarder for an internal polyfill API) +[assembly: TypeForwardedTo(typeof(InterpolatedStringHandlerArgumentAttribute))] +#pragma warning restore RS0016 // Add public types and members to the declared API + +#else namespace System.Runtime.CompilerServices { diff --git a/src/Compilers/Core/Portable/InternalUtilities/InterpolatedStringHandlerAttribute.cs b/src/Compilers/Core/Portable/InternalUtilities/InterpolatedStringHandlerAttribute.cs index f401f0728a03a..84f8216271823 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/InterpolatedStringHandlerAttribute.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/InterpolatedStringHandlerAttribute.cs @@ -2,7 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#if !NET6_0_OR_GREATER +#if NET6_0_OR_GREATER + +using System.Runtime.CompilerServices; + +#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting forwarder for an internal polyfill API) +[assembly: TypeForwardedTo(typeof(InterpolatedStringHandlerAttribute))] +#pragma warning restore RS0016 // Add public types and members to the declared API + +#else namespace System.Runtime.CompilerServices { diff --git a/src/Compilers/Core/Portable/InternalUtilities/IsExternalInit.cs b/src/Compilers/Core/Portable/InternalUtilities/IsExternalInit.cs index 344d6601587c5..2e63533ac4099 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/IsExternalInit.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/IsExternalInit.cs @@ -5,7 +5,15 @@ // Copied from: // https://github.com/dotnet/runtime/blob/218ef0f7776c2c20f6c594e3475b80f1fe2d7d08/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/IsExternalInit.cs -#if !NET6_0_OR_GREATER +#if NET6_0_OR_GREATER + +using System.Runtime.CompilerServices; + +#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting forwarder for an internal polyfill API) +[assembly: TypeForwardedTo(typeof(IsExternalInit))] +#pragma warning restore RS0016 // Add public types and members to the declared API + +#else using System.ComponentModel; @@ -21,12 +29,4 @@ internal static class IsExternalInit } } -#else - -using System.Runtime.CompilerServices; - -#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting fowarder for an internal polyfill API) -[assembly: TypeForwardedTo(typeof(IsExternalInit))] -#pragma warning restore RS0016 // Add public types and members to the declared API - #endif diff --git a/src/Compilers/Core/Portable/InternalUtilities/RequiredMemberAttribute.cs b/src/Compilers/Core/Portable/InternalUtilities/RequiredMemberAttribute.cs index 45fc7774d9850..fd59b3e51e499 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/RequiredMemberAttribute.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/RequiredMemberAttribute.cs @@ -5,7 +5,15 @@ // Copied from: // https://github.com/dotnet/runtime/blob/fdd104ec5e1d0d2aa24a6723995a98d0124f724b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RequiredMemberAttribute.cs -#if !NET7_0_OR_GREATER +#if NET7_0_OR_GREATER + +using System.Runtime.CompilerServices; + +#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting forwarder for an internal polyfill API) +[assembly: TypeForwardedTo(typeof(RequiredMemberAttribute))] +#pragma warning restore RS0016 // Add public types and members to the declared API + +#else namespace System.Runtime.CompilerServices { diff --git a/src/Compilers/Core/Portable/InternalUtilities/SetsRequiredMembersAttribute.cs b/src/Compilers/Core/Portable/InternalUtilities/SetsRequiredMembersAttribute.cs index 9ccfa2aa01cab..60536a6cc569b 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/SetsRequiredMembersAttribute.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/SetsRequiredMembersAttribute.cs @@ -5,7 +5,16 @@ // Copied from: // https://github.com/dotnet/runtime/blob/fdd104ec5e1d0d2aa24a6723995a98d0124f724b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis/SetsRequiredMembersAttribute.cs -#if !NET7_0_OR_GREATER +#if NET7_0_OR_GREATER + +using System.Runtime.CompilerServices; +using System.Diagnostics.CodeAnalysis; + +#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting forwarder for an internal polyfill API) +[assembly: TypeForwardedTo(typeof(SetsRequiredMembersAttribute))] +#pragma warning restore RS0016 // Add public types and members to the declared API + +#else namespace System.Diagnostics.CodeAnalysis { diff --git a/src/Compilers/Test/Core/ModuleInitializerAttribute.cs b/src/Compilers/Test/Core/ModuleInitializerAttribute.cs index b4609c681d25b..b21faf37adcf4 100644 --- a/src/Compilers/Test/Core/ModuleInitializerAttribute.cs +++ b/src/Compilers/Test/Core/ModuleInitializerAttribute.cs @@ -12,4 +12,12 @@ public sealed class ModuleInitializerAttribute : Attribute } } +#else + +using System.Runtime.CompilerServices; + +#pragma warning disable RS0016 // Add public types and members to the declared API (this is a supporting forwarder for an internal polyfill API) +[assembly: TypeForwardedTo(typeof(ModuleInitializerAttribute))] +#pragma warning restore RS0016 // Add public types and members to the declared API + #endif