Skip to content

Commit

Permalink
Start building a universal System.Linq.Expressions library (#61952)
Browse files Browse the repository at this point in the history
System.Linq.Expressions currently offers multiple build time definitions to build different flavors of the library:

* `FEATURE_COMPILE` (defined everywhere except iOS/tvOS/Catalyst, and NativeAOT) - enables Reflection.Emit-based expression tree compiler.
* `FEATURE_INTERPRET` (always defined and not actually possible to build without) - enables the expression interpreter
* `FEATURE_DLG_INVOKE` (defined everywhere except NativeAOT, but we likely need to be able to run without it on iOS too because there's uninvestigated bugs around it ActiveIssue'd in #54970) - in the interpreter, use a delegate to invoke `CallInstructions` instead of `MethodInfo.Invoke`. The delegate might have to be Reflection.Emitted if it's not supportable with `Func`/`Action` (that's the uninvestigated iOS/tvOS/Catalyst bug).

For #61231, we need to be able to build a single System.Linq.Expression library that can switch between these build-time configurations _at publish time_ since we don't want to build a separate S.L.Expressions library for NativeAOT. There are advantages in having this setup for non-NativeAOT scenarios too. This pull request accomplishes that by mechanically changing the `#define`s into feature switches.

The feature switch is placed in the last proposed location for #17973. I expect we'll want such API to be public at some point; now that Xamarin and NativeAOT use this formerly .NET Native-only thing the API request became relevant again.

This pull request is focused on the mechanical replacement of `#defines` with feature switches and it's already a lot bigger than I'm comfortable with.

There's some obvious "`!FEATURE_COMPILE` means this is .NET Native with everything what that meant" that I did not touch because this is meant to be a mechanical change. Some cleanup will be needed at some point. Right now this just mostly means we're running fewer tests than we could.

Validation:
* Verified that we're still running the same number of tests with CoreCLR as we previously were and they're all passing.
* Verified we're getting mostly the same size of the S.L.Expressions library on iOS (433 kB grew to 436 kB, the diffs are expected).
* Verified things work on the NativeAOT side as well.
  • Loading branch information
MichalStrehovsky authored Dec 1, 2021
1 parent 6af8b24 commit 09cd9c2
Show file tree
Hide file tree
Showing 44 changed files with 357 additions and 377 deletions.
2 changes: 2 additions & 0 deletions eng/illink.targets
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@
<ILLinkArgs Condition="'$(ILLinkRewritePDBs)' == 'true' and Exists('$(ILLinkTrimAssemblySymbols)')">$(ILLinkArgs) -b true</ILLinkArgs>
<!-- pass the non-embedded descriptors xml file on the command line -->
<ILLinkArgs Condition="'$(ILLinkDescriptorsLibraryBuildXml)' != ''">$(ILLinkArgs) -x "$(ILLinkDescriptorsLibraryBuildXml)"</ILLinkArgs>
<ILLinkArgs Condition="'$(ILLinkSubstitutionsLibraryBuildXml)' != ''">$(ILLinkArgs) --substitutions "$(ILLinkSubstitutionsLibraryBuildXml)"</ILLinkArgs>
<!-- suppress warnings with the following codes:
IL2008: Could not find type A specified in resource B
IL2009: Could not find method A in type B specified in resource C
Expand All @@ -254,6 +255,7 @@
<!-- IL2067-IL2091: Unsatisfied DynamicallyAccessedMembers requirements -->
<LinkerNoWarn>$(LinkerNoWarn);IL2067;IL2068;IL2069;IL2070;IL2071;IL2072;IL2073;IL2074;IL2075;IL2076;IL2077;IL2078;IL2079;IL2080;IL2081;IL2082;IL2083;IL2084;IL2085;IL2086;IL2087;IL2088;IL2089;IL2090;IL2091</LinkerNoWarn>
<ILLinkArgs>$(ILLinkArgs) --nowarn $(LinkerNoWarn)</ILLinkArgs>
<ILLinkArgs Condition="'$(ILLinkDisableIPConstProp)' == 'true'">$(ILLinkArgs) --disable-opt ipconstprop</ILLinkArgs>
</PropertyGroup>

<MakeDir Directories="$(ILLinkTrimInputPath)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,7 @@ public static partial class PlatformDetection
private static readonly Lazy<bool> s_LinqExpressionsBuiltWithIsInterpretingOnly = new Lazy<bool>(GetLinqExpressionsBuiltWithIsInterpretingOnly);
private static bool GetLinqExpressionsBuiltWithIsInterpretingOnly()
{
Type type = typeof(LambdaExpression);
if (type != null)
{
// The "Accept" method is under FEATURE_COMPILE conditional so it should not exist
MethodInfo methodInfo = type.GetMethod("Accept", BindingFlags.NonPublic | BindingFlags.Static);
return methodInfo == null;
}

return false;
return !(bool)typeof(LambdaExpression).GetMethod("get_CanCompileToIL").Invoke(null, Array.Empty<object>());
}

// Please make sure that you have the libgdiplus dependency installed.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<linker>
<assembly fullname="System.Linq.Expressions">
<type fullname="System.Linq.Expressions.LambdaExpression">
<method signature="System.Boolean get_CanCompileToIL()" body="stub" value="false" />
</type>
</assembly>
</linker>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<linker>
<assembly fullname="System.Linq.Expressions">
<type fullname="System.Linq.Expressions.LambdaExpression">
<method signature="System.Boolean get_CanCompileToIL()" feature="System.Linq.Expressions.CanCompileToIL" featurevalue="false" body="stub" value="false" />
</type>
<type fullname="System.Dynamic.Utils.DelegateHelpers">
<method signature="System.Boolean get_CanEmitObjectArrayDelegate()" feature="System.Linq.Expressions.CanEmitObjectArrayDelegate" featurevalue="false" body="stub" value="false" />
</type>
<type fullname="System.Linq.Expressions.Interpreter.CallInstruction">
<method signature="System.Boolean get_CanCreateArbitraryDelegates()" feature="System.Linq.Expressions.CanCreateArbitraryDelegates" featurevalue="false" body="stub" value="false" />
</type>
</assembly>
</linker>
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ TypesMustExist : Type 'System.Linq.Expressions.Interpreter.LightLambda' does not
TypesMustExist : Type 'System.Runtime.CompilerServices.CallSiteOps' does not exist in the reference but it does exist in the implementation.
TypesMustExist : Type 'System.Runtime.CompilerServices.Closure' does not exist in the reference but it does exist in the implementation.
TypesMustExist : Type 'System.Runtime.CompilerServices.RuntimeOps' does not exist in the reference but it does exist in the implementation.
Total Issues: 16
MembersMustExist : Member 'public System.Boolean System.Linq.Expressions.LambdaExpression.CanCompileToIL.get()' does not exist in the reference but it does exist in the implementation.
MembersMustExist : Member 'public System.Boolean System.Linq.Expressions.LambdaExpression.CanInterpret.get()' does not exist in the reference but it does exist in the implementation.
Total Issues: 18
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<FeatureInterpret>true</FeatureInterpret>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppCurrent)-iOS;$(NetCoreAppCurrent)-tvOS;$(NetCoreAppCurrent)-MacCatalyst</TargetFrameworks>
<Nullable>enable</Nullable>
<IsInterpreting>false</IsInterpreting>
<IsInterpreting Condition="'$(TargetsiOS)' == 'true' or '$(TargetstvOS)' == 'true' or '$(TargetsMacCatalyst)' == 'true'">true</IsInterpreting>
<DefineConstants> $(DefineConstants);FEATURE_DLG_INVOKE;FEATURE_FAST_CREATE</DefineConstants>
<DefineConstants Condition=" '$(IsInterpreting)' != 'true' ">$(DefineConstants);FEATURE_COMPILE</DefineConstants>
<DefineConstants Condition=" '$(FeatureInterpret)' == 'true' ">$(DefineConstants);FEATURE_INTERPRET</DefineConstants>
<ILLinkSubstitutionsLibraryBuildXml Condition="'$(IsInterpreting)' == 'true'">ILLink\ILLink.Substitutions.IsInterpreting.LibraryBuild.xml</ILLinkSubstitutionsLibraryBuildXml>
<DefineConstants> $(DefineConstants);FEATURE_FAST_CREATE</DefineConstants>

<!--
Disable constant propagation so that methods referenced from ILLink.Substitutions.xml don't get inlined
with a wrong value at library build time and the substitution can still be selected at publish time.
For iOS/tvOS/Catalyst we prefer smaller size by default, so keep constprop enabled to get rid of the expression compiler.
-->
<ILLinkDisableIPConstProp Condition="'$(TargetsiOS)' != 'true' and '$(TargetstvOS)' != 'true' and '$(TargetsMacCatalyst)' != 'true'">true</ILLinkDisableIPConstProp>
</PropertyGroup>
<ItemGroup>
<ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.xml" />
</ItemGroup>
<ItemGroup>
<Compile Include="$(CommonPath)System\Obsoletions.cs"
Link="Common\System\Obsoletions.cs" />
Expand Down Expand Up @@ -125,7 +133,7 @@
<Compile Include="System\Dynamic\UnaryOperationBinder.cs" />
<Compile Include="System\Dynamic\IInvokeOnGetBinder.cs" />
</ItemGroup>
<ItemGroup Condition=" '$(IsInterpreting)' != 'true'">
<ItemGroup>
<Compile Include="System\Linq\Expressions\Compiler\AssemblyGen.cs" />
<Compile Include="System\Dynamic\Utils\Helpers.cs" />
<Compile Include="System\Linq\Expressions\Compiler\AnalyzedTree.cs" />
Expand Down Expand Up @@ -157,7 +165,7 @@
<Compile Include="System\Runtime\CompilerServices\RuntimeOps.ExpressionQuoter.cs" />
<Compile Include="System\Runtime\CompilerServices\RuntimeOps.RuntimeVariableList.cs" />
</ItemGroup>
<ItemGroup Condition="'$(IsInterpreting)' == 'true' Or '$(FeatureInterpret)' == 'true'">
<ItemGroup>
<Compile Include="System\Dynamic\Utils\DelegateHelpers.cs" />
<Compile Include="System\Linq\Expressions\Interpreter\AddInstruction.cs" />
<Compile Include="System\Linq\Expressions\Interpreter\AndInstruction.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ namespace System.Dynamic
{
internal static partial class UpdateDelegates
{
#if FEATURE_COMPILE
[Obsolete("pregenerated CallSite<T>.Update delegate", error: true)]
internal static TRet UpdateAndExecute1<T0, TRet>(CallSite site, T0 arg0)
{
Expand Down Expand Up @@ -2895,6 +2894,5 @@ internal static void NoMatchVoid10<T0, T1, T2, T3, T4, T5, T6, T7, T8, T9>(CallS
site._match = false;
return;
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ namespace System.Dynamic
{
internal static partial class UpdateDelegates
{
#if FEATURE_COMPILE
<#
for (int i = 1; i <= 10; i++)
{
Expand Down Expand Up @@ -330,6 +329,5 @@ for (int i = 1; i <= 10; i++)
}
}
#>
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,43 @@

using System.Diagnostics.CodeAnalysis;
using System.Reflection;

#if !FEATURE_DYNAMIC_DELEGATE
using System.Reflection.Emit;
using System.Text;
using System.Threading;
#endif

namespace System.Dynamic.Utils
{
internal static class DelegateHelpers
{
internal static Delegate CreateObjectArrayDelegate(Type delegateType, Func<object?[], object?> handler)
// This can be flipped to true using feature switches at publishing time
internal static bool CanEmitObjectArrayDelegate => true;

// Separate class so that the it can be trimmed away and doesn't get conflated
// with the Reflection.Emit statics below.
private static class DynamicDelegateLightup
{
#if !FEATURE_DYNAMIC_DELEGATE
return CreateObjectArrayDelegateRefEmit(delegateType, handler);
#else
return Internal.Runtime.Augments.DynamicDelegateAugments.CreateObjectArrayDelegate(delegateType, handler);
#endif
public static Func<Type, Func<object?[], object?>, Delegate> CreateObjectArrayDelegate { get; }
= CreateObjectArrayDelegateInternal();

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "Works around https://github.com/dotnet/linker/issues/2392")]
private static Func<Type, Func<object?[], object?>, Delegate> CreateObjectArrayDelegateInternal()
=> Type.GetType("Internal.Runtime.Augments.DynamicDelegateAugments")!
.GetMethod("CreateObjectArrayDelegate")!
.CreateDelegate<Func<Type, Func<object?[], object?>, Delegate>>();
}


#if !FEATURE_DYNAMIC_DELEGATE
internal static Delegate CreateObjectArrayDelegate(Type delegateType, Func<object?[], object?> handler)
{
if (CanEmitObjectArrayDelegate)
{
return CreateObjectArrayDelegateRefEmit(delegateType, handler);
}
else
{
return DynamicDelegateLightup.CreateObjectArrayDelegate(delegateType, handler);
}
}

private static readonly CacheDict<Type, MethodInfo> s_thunks = new CacheDict<Type, MethodInfo>(256);
private static readonly MethodInfo s_FuncInvoke = typeof(Func<object?[], object?>).GetMethod("Invoke")!;
Expand Down Expand Up @@ -290,7 +305,5 @@ private static Type ConvertToBoxableType(Type t)
{
return (t.IsPointer) ? typeof(IntPtr) : t;
}

#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ internal static ParameterInfo[] GetParametersCached(this MethodBase method)
return pis;
}

#if FEATURE_COMPILE
// Expression trees/compiler just use IsByRef, why do we need this?
// (see LambdaCompiler.EmitArguments for usage in the compiler)
internal static bool IsByRefParameter(this ParameterInfo pi)
Expand All @@ -91,6 +90,5 @@ internal static bool IsByRefParameter(this ParameterInfo pi)

return (pi.Attributes & ParameterAttributes.Out) == ParameterAttributes.Out;
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -911,8 +911,6 @@ public static MethodInfo GetInvokeMethod(this Type delegateType)
return delegateType.GetMethod("Invoke", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)!;
}

#if FEATURE_COMPILE

internal static bool IsUnsigned(this Type type) => IsUnsigned(GetNonNullableType(type).GetTypeCode());

internal static bool IsUnsigned(this TypeCode typeCode)
Expand Down Expand Up @@ -946,8 +944,6 @@ internal static bool IsFloatingPoint(this TypeCode typeCode)
}
}

#endif

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
Justification = "The Array 'Get' method is dynamically constructed and is not included in IL. It is not subject to trimming.")]
public static MethodInfo GetArrayGetMethod(Type arrayType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ public static FieldInfo DateTime_MinValue
(s_Math_Pow_Double_Double = typeof(Math).GetMethod(nameof(Math.Pow), new[] { typeof(double), typeof(double) })!);

// Closure and RuntimeOps helpers are used only in the compiler.
#if FEATURE_COMPILE
private static ConstructorInfo? s_Closure_ObjectArray_ObjectArray;
public static ConstructorInfo Closure_ObjectArray_ObjectArray =>
s_Closure_ObjectArray_ObjectArray ??
Expand Down Expand Up @@ -194,6 +193,5 @@ public static FieldInfo DateTime_MinValue
public static MethodInfo RuntimeOps_Quote =>
s_RuntimeOps_Quote ??
(s_RuntimeOps_Quote = typeof(RuntimeOps).GetMethod(nameof(RuntimeOps.Quote))!);
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ private static TypeInfo NextTypeInfo(Type initialArg, TypeInfo curTypeInfo)
return nextTypeInfo;
}

#if !FEATURE_COMPILE

public delegate object VBCallSiteDelegate0<T>(T callSite, object instance);
public delegate object VBCallSiteDelegate1<T>(T callSite, object instance, ref object arg1);
public delegate object VBCallSiteDelegate2<T>(T callSite, object instance, ref object arg1, ref object arg2);
Expand All @@ -95,7 +93,6 @@ private static TypeInfo NextTypeInfo(Type initialArg, TypeInfo curTypeInfo)
public delegate object VBCallSiteDelegate6<T>(T callSite, object instance, ref object arg1, ref object arg2, ref object arg3, ref object arg4, ref object arg5, ref object arg6);
public delegate object VBCallSiteDelegate7<T>(T callSite, object instance, ref object arg1, ref object arg2, ref object arg3, ref object arg4, ref object arg5, ref object arg6, ref object arg7);


private static Type TryMakeVBStyledCallSite(Type[] types)
{
// Shape of VB CallSiteDelegates is CallSite * (instance : obj) * [arg-n : byref obj] -> obj
Expand Down Expand Up @@ -128,7 +125,6 @@ private static Type TryMakeVBStyledCallSite(Type[] types)
default: return null;
}
}
#endif

/// <summary>
/// Creates a new delegate, or uses a func/action
Expand Down Expand Up @@ -163,11 +159,14 @@ internal static Type MakeNewDelegate(Type[] types)

if (needCustom)
{
#if FEATURE_COMPILE
return MakeNewCustomDelegate(types);
#else
return TryMakeVBStyledCallSite(types) ?? MakeNewCustomDelegate(types);
#endif
if (LambdaExpression.CanCompileToIL)
{
return MakeNewCustomDelegate(types);
}
else
{
return TryMakeVBStyledCallSite(types) ?? MakeNewCustomDelegate(types);
}
}

Type result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,26 +107,27 @@ private static bool IsByRef(DynamicMetaObject mo)
return mo.Expression is ParameterExpression pe && pe.IsByRef;
}

#if FEATURE_COMPILE
private const MethodAttributes CtorAttributes = MethodAttributes.RTSpecialName | MethodAttributes.HideBySig | MethodAttributes.Public;
private const MethodImplAttributes ImplAttributes = MethodImplAttributes.Runtime | MethodImplAttributes.Managed;
private const MethodAttributes InvokeAttributes = MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.NewSlot | MethodAttributes.Virtual;
private static readonly Type[] s_delegateCtorSignature = { typeof(object), typeof(IntPtr) };
#endif

private static Type MakeNewCustomDelegate(Type[] types)
{
#if FEATURE_COMPILE
Type returnType = types[types.Length - 1];
Type[] parameters = types.RemoveLast();

TypeBuilder builder = AssemblyGen.DefineDelegateType("Delegate" + types.Length);
builder.DefineConstructor(CtorAttributes, CallingConventions.Standard, s_delegateCtorSignature).SetImplementationFlags(ImplAttributes);
builder.DefineMethod("Invoke", InvokeAttributes, returnType, parameters).SetImplementationFlags(ImplAttributes);
return builder.CreateTypeInfo()!;
#else
throw new PlatformNotSupportedException();
#endif
if (LambdaExpression.CanCompileToIL)
{
Type returnType = types[types.Length - 1];
Type[] parameters = types.RemoveLast();
Type[] delegateCtorSignature = { typeof(object), typeof(IntPtr) };

const MethodAttributes ctorAttributes = MethodAttributes.RTSpecialName | MethodAttributes.HideBySig | MethodAttributes.Public;
const MethodImplAttributes implAttributes = MethodImplAttributes.Runtime | MethodImplAttributes.Managed;
const MethodAttributes invokeAttributes = MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.NewSlot | MethodAttributes.Virtual;

TypeBuilder builder = AssemblyGen.DefineDelegateType("Delegate" + types.Length);
builder.DefineConstructor(ctorAttributes, CallingConventions.Standard, delegateCtorSignature).SetImplementationFlags(implAttributes);
builder.DefineMethod("Invoke", invokeAttributes, returnType, parameters).SetImplementationFlags(implAttributes);
return builder.CreateTypeInfo()!;
}
else
{
throw new PlatformNotSupportedException();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ namespace System.Linq.Expressions.Interpreter
{
internal partial class CallInstruction
{
#if FEATURE_DLG_INVOKE
private const int MaxHelpers = 5;
#endif

#if FEATURE_FAST_CREATE
private const int MaxArgs = 3;
Expand Down Expand Up @@ -158,7 +156,6 @@ private static CallInstruction FastCreate<T0, T1>(MethodInfo target, ParameterIn
}
#endif

#if FEATURE_DLG_INVOKE
[return: DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes.PublicConstructors)]
private static Type GetHelperType(MethodInfo info, Type[] arrTypes)
{
Expand Down Expand Up @@ -211,10 +208,8 @@ private static Type GetHelperType(MethodInfo info, Type[] arrTypes)
}
return t;
}
#endif
}

#if FEATURE_DLG_INVOKE
internal sealed class ActionCallInstruction : CallInstruction
{
private readonly Action _target;
Expand Down Expand Up @@ -577,6 +572,4 @@ public override int Run(InterpretedFrame frame)

public override string ToString() => "Call(" + _target.Method + ")";
}

#endif
}
Loading

0 comments on commit 09cd9c2

Please sign in to comment.