Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Add RuntimeHelpers.GetUserDefinedOperatorMethods for trimming-safe access to user-defined operators #97709

Closed
simonrozsival opened this issue Jan 30, 2024 · 16 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Reflection discussion enhancement Product code improvement that does NOT require public API changes/additions linkable-framework Issues associated with delivering a linker friendly framework
Milestone

Comments

@simonrozsival
Copy link
Member

simonrozsival commented Jan 30, 2024

Background and motivation

Libraries such as System.Linq.Expressions or MAUI often require the invocation of user-defined operators through reflection. Unfortunately, achieving trimming-safe code by utilizing annotations with Type.GetMethod or Type.GetMethods is not possible in certain scenarios.

In the context of System.Linq.Expressions, there is a necessity to resolve operators corresponding to specific expressions. Although the trimmer currently includes a hard-coded special case for System.Linq.Expressions to preserve user-defined operators, attempts to implement a similar workaround for AOT weren't successful (see #79016). Introducing a new API could effectively decouple the trimmer from the runtime, explicitly defining this currently hidden contract.

In the case of .NET MAUI, invoking implicit operators through reflection is essential for converting values in data bindings and XAML property setters (see dotnet/maui#19922). Unfortunately, a reliable trimming-safe method for implementing this feature is currently lacking, posing challenges to making MAUI trimmable without introducing breaking changes for our customers.

API Proposal

namespace System.Runtime.CompilerServices
{
  public partial class RuntimeHelpers
  {
    public static MethodInfo[] GetUserDefinedOperatorMethods(Type type, string name, BindingFlags flags, Type[] types);
  }
}

The method will behave the same way as Type.GetMethod(string name, BindingFlags flags, Type[] types) but instead of throwing AmbiguousMatchException in case of multiple matches, the method returns all matches in an array.

The method must throw when the name is not one of the operator method names (op_Implicit, op_Addition, ...) and filter out any matching methods which don't have a special name. This API is not meant to allow trimming-safe access to arbitrary methods via name. This functionality could be easily misused by developers and unintentionally hamper trimming of their apps.

Trimming semantics

  • The method will be recognized by ILLink and ILC
  • When type is annotated with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] the trimmer doesn't need to do any additional work.
  • When name isn't a constant string, the trimmer should produce a trimming warning.
  • When name is a constant string, the trimmer should preserve all operators with the given name on all preserved types and it should not produce any warning.
  • When name isn't a valid operator name, the method should throw an InvalidOperationException and the trimmer should produce a compile-time trimming warning.

API Usage

For example, the new API could be used in place of Type.GetMethods(BindingFlags) in the MAUI codebase:

public static bool TryCast(ref object value, Type targetType)
{
    var cast = GetCastMethod(value.GetType(), from: value.GetType(), to: targetType)
      ?? GetCastMethod(targetType, from: value.GetType(), to: targetType);
    if (cast is not null)
    {
        value = cast.Invoke(null, [value])!;
        return true;
    }
    
    return false;

    static MethodInfo? GetCastMethod(Type type, Type from, Type to)
    {
        const BindingFlags flags = BindingFlags.Static | BindingFlags.Public | BindingFlags.FlattenHierarchy;
        return RuntimeHelpers.GetUserDefinedOperatorMethods(type, "op_Implicit", flags, [from])
            .FirstOrDefault(m => to.IsAssignableFrom(m.ReturnType));
    }
}

Alternative Designs

  1. Instead of passing method names as string, we could introduce specific variants of the method:
// Conversion operators
public static MethodInfo[] GetImplicitOperatorMethods(Type type, Type parameter);
public static MethodInfo[] GetImplicitOperatorMethods(Type type, Type parameter, BindingFlags flags);
public static MethodInfo[] GetExplicitOperatorMethods(Type type, Type parameter);
public static MethodInfo[] GetExplicitOperatorMethods(Type type, Type parameter, BindingFlags flags);

// Binary operators
public static MethodInfo? GetAdditionOperatorMethod(Type type, Type left, Type right);
public static MethodInfo? GetAdditionOperatorMethod(Type type, Type left, Type right, BindingFlags flags);
public static MethodInfo? GetSubtractionOperatorMethod(Type type, Type left, Type right);
public static MethodInfo? GetSubtractionOperatorMethod(Type type, Type left, Type right, BindingFlags flags);
// + Multiply, Division, Modulus, LeftShift, RightShift, LessThan, GreaterThan, LessThanOrEqual,
// GreaterThanOrEqual, Equality, Inequality, BitwiseAnd, ExclusiveOr, BitwiseOr, LogicalNot

// Unary operators
public static MethodInfo? GetUnaryNegationOperatorMethod(Type type);
public static MethodInfo? GetUnaryNegationOperatorMethod(Type type, BindingFlags flags);
// + UnaryPlus, OnesComplement, True, False, Increment, Decrement
  1. Follow the pattern used in System.Linq.Expressions:
// Accepted names: "op_Implicit", "op_Explicit"
public static MethodInfo? GetConversionOperator(Type type, string name, Type convertFrom, Type convertTo);
public static MethodInfo? GetConversionOperator(Type type, string name, Type convertFrom, Type convertTo, BindingFlags flags);

// Accepted names: "op_Addition", "op_Subtraction", "op_Multiplication", ...
public static MethodInfo? GetBinaryOperator(Type type, string name, Type left, Type right);
public static MethodInfo? GetBinaryOperator(Type type, string name, Type left, Type right, BindingFlags flags);

// Accepted names: "op_True", "op_False", "op_UnaryNegation", ...
public static UnaryExpression? GetUnaryOperator(Type type, string name, Type parameter);
public static UnaryExpression? GetUnaryOperator(Type type, string name, Type parameter, BindingFlags flags);
  1. Do nothing
    System.Linq.Expressions has a hard-coded special case in the trimmer and .NET MAUI could implement a workaround.

Risks

Low risk of misuse. The new API doesn't replace Type.GetMethod and Type.GetMethods, it is deliberately limited to user-defined operators.

@simonrozsival simonrozsival added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 30, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 30, 2024
@ghost
Copy link

ghost commented Jan 30, 2024

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Libraries such as System.Linq.Expressions or MAUI often require the invocation of user-defined operators through reflection. Unfortunately, achieving trimming-safe code by utilizing annotations with Type.GetMethod or Type.GetMethods is not possible in certain scenarios.

In the context of System.Linq.Expressions, there is a necessity to resolve operators corresponding to specific expressions. Although the trimmer currently includes a hard-coded special case for System.Linq.Expressions to preserve user-defined operators, attempts to implement a similar workaround for AOT weren't successful (see #79016). Introducing a new API could effectively decouple the trimmer from the runtime, explicitly defining this currently hidden contract.

In the case of .NET MAUI, invoking implicit operators through reflection is essential for converting values in data bindings and XAML property setters (see dotnet/maui#19922). Unfortunately, a reliable trimming-safe method for implementing this feature is currently lacking, posing challenges to making MAUI trimmable without introducing breaking changes for our customers.

API Proposal

namespace System.Runtime.CompilerServices
{
  public partial class RuntimeHelpers
  {
    public static MethodInfo[] GetUserDefinedOperatorMethods(Type type, string name, BindingFlags flags, Type[] types);
  }
}

The method will behave the same way as Type.GetMethod(string name, BindingFlags flags, Type[] types) but instead of throwing AmbiguousMatchException in case of multiple matches, the method returns all matches in an array.

The method must throw when the name is not one of the operator method names (op_Implicit, op_Addition, ...) and filter out any matching methods which don't have a special name. This API is not meant to allow trimming-safe access to arbitrary methods via name. This functionality could be easily misused by developers and unintentionally hamper trimming of their apps.

Trimming semantics:

  • When type is annotated with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] the trimmer doesn't need to do any additional work.
  • When name isn't a constant string, the trimmer should produce a trimming warning.
  • When name is a constant string, the trimmer should preserve all operators with the given name on all preserved types and it should not produce any warning.
  • When name isn't a valid operator name, the method should throw an InvalidOperationException and the trimmer should produce a compile-time trimming warning.

API Usage

For example, the new API could be used in place of Type.GetMethods(BindingFlags) in the MAUI codebase:

public static bool TryCast(ref object value, Type targetType)
{
    var cast = GetCastMethod(value.GetType(), from: value.GetType(), to: targetType)
      ?? GetCastMethod(targetType, from: value.GetType(), to: targetType);
    if (cast is not null)
    {
        value = cast.Invoke(null, [value])!;
        return true;
    }
    
    return false;

    static MethodInfo? GetCastMethod(Type type, Type from, Type to)
    {
        const BindingFlags flags = BindingFlags.Static | BindingFlags.Public | BindingFlags.FlattenHierarchy;
        return RuntimeHelpers.GetUserDefinedOperatorMethods(type, "op_Implicit", flags, [from])
            .FirstOrDefault(m => to.IsAssignableFrom(m.ReturnType));
    }
}

Alternative Designs

  1. Instead of passing method names as string, we could introduce specific variants of the method:
// Conversion operators
public static MethodInfo[] GetImplicitOperatorMethods(Type type, Type parameter);
public static MethodInfo[] GetImplicitOperatorMethods(Type type, Type parameter, BindingFlags flags);
public static MethodInfo[] GetExplicitOperatorMethods(Type type, Type parameter);
public static MethodInfo[] GetExplicitOperatorMethods(Type type, Type parameter, BindingFlags flags);

// Binary operators
public static MethodInfo? GetAdditionOperatorMethod(Type type, Type left, Type right);
public static MethodInfo? GetAdditionOperatorMethod(Type type, Type left, Type right, BindingFlags flags);
public static MethodInfo? GetSubtractionOperatorMethod(Type type, Type left, Type right);
public static MethodInfo? GetSubtractionOperatorMethod(Type type, Type left, Type right, BindingFlags flags);
// + Multiply, Division, Modulus, LeftShift, RightShift, LessThan, GreaterThan, LessThanOrEqual,
// GreaterThanOrEqual, Equality, Inequality, BitwiseAnd, ExclusiveOr, BitwiseOr, LogicalNot

// Unary operators
public static MethodInfo? GetUnaryNegationOperatorMethod(Type type);
public static MethodInfo? GetUnaryNegationOperatorMethod(Type type, BindingFlags flags);
// + UnaryPlus, OnesComplement, True, False, Increment, Decrement
  1. Follow the pattern used in System.Linq.Expressions:
// Accepted names: "op_Implicit", "op_Explicit"
public static MethodInfo? GetConversionOperator(Type type, string name, Type convertFrom, Type convertTo);
public static MethodInfo? GetConversionOperator(Type type, string name, Type convertFrom, Type convertTo, BindingFlags flags);

// Accepted names: "op_Addition", "op_Subtraction", "op_Multiplication", ...
public static MethodInfo? GetBinaryOperator(Type type, string name, Type left, Type right);
public static MethodInfo? GetBinaryOperator(Type type, string name, Type left, Type right, BindingFlags flags);

// Accepted names: "op_True", "op_False", "op_UnaryNegation", ...
public static UnaryExpression? GetUnaryOperator(Type type, string name, Type parameter);
public static UnaryExpression? GetUnaryOperator(Type type, string name, Type parameter, BindingFlags flags);
  1. Do nothing
    System.Linq.Expressions has a hard-coded special case in the trimmer and .NET MAUI could implement a workaround.

Risks

Low risk of misuse. The new API doesn't replace Type.GetMethod and Type.GetMethods, it is deliberately limited to user-defined operators.

Author: simonrozsival
Assignees: -
Labels:

api-suggestion, area-System.Reflection, untriaged

Milestone: -

@simonrozsival
Copy link
Member Author

@eerhardt
Copy link
Member

Why is this method defined on System.Runtime.CompilerServices.RuntimeHelpers and not on System.Type?

My understanding is that System.Runtime.CompilerServices is meant for public APIs that we expect Roslyn (and other compilers) to use, but generally not used by developers.

@tannergooding
Copy link
Member

Its worth noting that generic math (https://learn.microsoft.com/en-us/dotnet/standard/generics/math) provides access to these in a trim safe way via constrained generics over the relevant interfaces.

It may be worthwhile to see if that can be utilized instead (either via new APIs, extensions, etc) as it is a much stronger and more robust pattern.

@simonrozsival
Copy link
Member Author

@eerhardt I see. The method is supposed to be a thin wrapper on top of the Type methods that is recognized by trimmer and implies some special trimmer behavior. I think it should be "hidden" from developers working on apps and that it should not pop-up in intellisense when working with common types and so I didn't want to add the method to Type.

@sbomer
Copy link
Member

sbomer commented Jan 30, 2024

It's not clear to me how this helps with System.Linq.Expressions. I think NativeAot would still need to implement the same heuristic as ILLink does (or a similar heuristic based on existing GetUserDefinedUnaryOperator), unless we change the code Roslyn emits to construct expression trees. But if we are open to changing that, we should consider instead introducing new trim-compatible APIs as discussed in dotnet/linker#1821 (comment).

It seems like the MAUI usage is not trim-safe because the type is not statically known due to object.GetType(), right? The proposal solves this for the case of custom operators by saying "if this method is called with a certain string argument, then keep that operator for every kept type". If we are going to introduce a new API for this, it would be better to make it more granular, so that it only keeps operators on types where they're required. The problem of a statically unknown type is a more general one, so for that I'd like to consider solutions that aren't specific to custom operators.

@simonrozsival
Copy link
Member Author

@sbomer Yes, I wasn't clear enough. ILC would need to recognize the method too and handle it the same way. I'll update the text to make this clear. My understanding is that adding new APIs to Expressions would be difficult so we should instead focus on the existing APIs and make them trimming-safe.

Yes, we don't know the types statically and we can't change the APIs to be generic (yet). I'm not sure how we would narrow down the set of types where preserving the operators is required. The problem is that either the parameter type or the return type usually isn't statically known. Maybe ILLink/ILC could narrow down the operators that it preserves when for example the return type or some of the parameters are known statically?

@MichalStrehovsky
Copy link
Member

attempts to implement a similar workaround for AOT weren't successful

I don't remember we even attempted this; we track the work in #69745. I don't think it would be particularly hard. We just didn't implement it because #79016 would be preferable and LINQ expressions have other issues.

This API is not meant to allow trimming-safe access to arbitrary methods via name. This functionality could be easily misused by developers and unintentionally hamper trimming of their apps.

The proposed API will quite literally hamper trimming of the app :). Conversion operators typically new up types of the conversion target, so it will bring in lots of new types that the app never used - it's not just about the operator method body. Introducing a new API restricted to operators feels like "rules for thee but not for me"

The problem with wanting to access a method with some name and signature but not knowing on what type is more general than just the operators. We could solve this by just allowing .GetMethod with a constant string and unknown owning type and have it mean "keep this on any type". It will absolutely make trimming a lot less efficient and have ripple effects across the entire app. We could have this generate a warning that people will need to suppress to acknowledge this. We'll need to think about the usability aspect of this a little so that people don't prefer "keep method with this name on all types" to "place DAM on the Type where this comes from".

@simonrozsival
Copy link
Member Author

The proposed API will quite literally hamper trimming of the app :)

Yes, it is of course a trade-off between keeping support for the existing feature and increasing app size. At least in the case of MAUI, it works around this issue by disabling trimming completely. If we allow trimming everything except for op_Implicit, we will already see a big improvement, there aren't that many implicit operators used in most apps.

We could solve this by just allowing .GetMethod with a constant string and unknown owning type and have it mean "keep this on any type". ... We could have this generate a warning that people will need to suppress to acknowledge this.

I am not sure if this wouldn't be a breaking change for some customers. I can imagine scenarios where the type isn't statically known, but the developer only calls it with type where they make sure that the method is preserved using [DynamicDependency] for example. The proposed API is meant as an explicit opt-in for this behavior.

@steveharter steveharter added enhancement Product code improvement that does NOT require public API changes/additions linkable-framework Issues associated with delivering a linker friendly framework labels Jan 31, 2024
@ghost
Copy link

ghost commented Jan 31, 2024

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Libraries such as System.Linq.Expressions or MAUI often require the invocation of user-defined operators through reflection. Unfortunately, achieving trimming-safe code by utilizing annotations with Type.GetMethod or Type.GetMethods is not possible in certain scenarios.

In the context of System.Linq.Expressions, there is a necessity to resolve operators corresponding to specific expressions. Although the trimmer currently includes a hard-coded special case for System.Linq.Expressions to preserve user-defined operators, attempts to implement a similar workaround for AOT weren't successful (see #79016). Introducing a new API could effectively decouple the trimmer from the runtime, explicitly defining this currently hidden contract.

In the case of .NET MAUI, invoking implicit operators through reflection is essential for converting values in data bindings and XAML property setters (see dotnet/maui#19922). Unfortunately, a reliable trimming-safe method for implementing this feature is currently lacking, posing challenges to making MAUI trimmable without introducing breaking changes for our customers.

API Proposal

namespace System.Runtime.CompilerServices
{
  public partial class RuntimeHelpers
  {
    public static MethodInfo[] GetUserDefinedOperatorMethods(Type type, string name, BindingFlags flags, Type[] types);
  }
}

The method will behave the same way as Type.GetMethod(string name, BindingFlags flags, Type[] types) but instead of throwing AmbiguousMatchException in case of multiple matches, the method returns all matches in an array.

The method must throw when the name is not one of the operator method names (op_Implicit, op_Addition, ...) and filter out any matching methods which don't have a special name. This API is not meant to allow trimming-safe access to arbitrary methods via name. This functionality could be easily misused by developers and unintentionally hamper trimming of their apps.

Trimming semantics

  • The method will be recognized by ILLink and ILC
  • When type is annotated with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] the trimmer doesn't need to do any additional work.
  • When name isn't a constant string, the trimmer should produce a trimming warning.
  • When name is a constant string, the trimmer should preserve all operators with the given name on all preserved types and it should not produce any warning.
  • When name isn't a valid operator name, the method should throw an InvalidOperationException and the trimmer should produce a compile-time trimming warning.

API Usage

For example, the new API could be used in place of Type.GetMethods(BindingFlags) in the MAUI codebase:

public static bool TryCast(ref object value, Type targetType)
{
    var cast = GetCastMethod(value.GetType(), from: value.GetType(), to: targetType)
      ?? GetCastMethod(targetType, from: value.GetType(), to: targetType);
    if (cast is not null)
    {
        value = cast.Invoke(null, [value])!;
        return true;
    }
    
    return false;

    static MethodInfo? GetCastMethod(Type type, Type from, Type to)
    {
        const BindingFlags flags = BindingFlags.Static | BindingFlags.Public | BindingFlags.FlattenHierarchy;
        return RuntimeHelpers.GetUserDefinedOperatorMethods(type, "op_Implicit", flags, [from])
            .FirstOrDefault(m => to.IsAssignableFrom(m.ReturnType));
    }
}

Alternative Designs

  1. Instead of passing method names as string, we could introduce specific variants of the method:
// Conversion operators
public static MethodInfo[] GetImplicitOperatorMethods(Type type, Type parameter);
public static MethodInfo[] GetImplicitOperatorMethods(Type type, Type parameter, BindingFlags flags);
public static MethodInfo[] GetExplicitOperatorMethods(Type type, Type parameter);
public static MethodInfo[] GetExplicitOperatorMethods(Type type, Type parameter, BindingFlags flags);

// Binary operators
public static MethodInfo? GetAdditionOperatorMethod(Type type, Type left, Type right);
public static MethodInfo? GetAdditionOperatorMethod(Type type, Type left, Type right, BindingFlags flags);
public static MethodInfo? GetSubtractionOperatorMethod(Type type, Type left, Type right);
public static MethodInfo? GetSubtractionOperatorMethod(Type type, Type left, Type right, BindingFlags flags);
// + Multiply, Division, Modulus, LeftShift, RightShift, LessThan, GreaterThan, LessThanOrEqual,
// GreaterThanOrEqual, Equality, Inequality, BitwiseAnd, ExclusiveOr, BitwiseOr, LogicalNot

// Unary operators
public static MethodInfo? GetUnaryNegationOperatorMethod(Type type);
public static MethodInfo? GetUnaryNegationOperatorMethod(Type type, BindingFlags flags);
// + UnaryPlus, OnesComplement, True, False, Increment, Decrement
  1. Follow the pattern used in System.Linq.Expressions:
// Accepted names: "op_Implicit", "op_Explicit"
public static MethodInfo? GetConversionOperator(Type type, string name, Type convertFrom, Type convertTo);
public static MethodInfo? GetConversionOperator(Type type, string name, Type convertFrom, Type convertTo, BindingFlags flags);

// Accepted names: "op_Addition", "op_Subtraction", "op_Multiplication", ...
public static MethodInfo? GetBinaryOperator(Type type, string name, Type left, Type right);
public static MethodInfo? GetBinaryOperator(Type type, string name, Type left, Type right, BindingFlags flags);

// Accepted names: "op_True", "op_False", "op_UnaryNegation", ...
public static UnaryExpression? GetUnaryOperator(Type type, string name, Type parameter);
public static UnaryExpression? GetUnaryOperator(Type type, string name, Type parameter, BindingFlags flags);
  1. Do nothing
    System.Linq.Expressions has a hard-coded special case in the trimmer and .NET MAUI could implement a workaround.

Risks

Low risk of misuse. The new API doesn't replace Type.GetMethod and Type.GetMethods, it is deliberately limited to user-defined operators.

Author: simonrozsival
Assignees: -
Labels:

enhancement, api-suggestion, area-System.Reflection, untriaged, linkable-framework

Milestone: -

@steveharter
Copy link
Member

Its worth noting that generic math (https://learn.microsoft.com/en-us/dotnet/standard/generics/math) provides access to these in a trim safe way via constrained generics over the relevant interfaces.

It may be worthwhile to see if that can be utilized instead (either via new APIs, extensions, etc) as it is a much stronger and more robust pattern.

I'm assuming that means an interface would need to be created for each user-defined operator, and then somehow registered or discoverable by LINQ, for example. That would be a breaking change for LINQ, which is not wanted here.

Thoughts @simonrozsival?

@steveharter steveharter added discussion and removed untriaged New issue has not been triaged by the area owner labels Jan 31, 2024
@simonrozsival
Copy link
Member Author

I am not sure how we could apply the interfaces from generic math to LINQ and MAUI. I also don't think we would be able to add similar new APIs to these libraries, especially to LINQ.

@MichalStrehovsky
Copy link
Member

The proposed API will quite literally hamper trimming of the app :)

Yes, it is of course a trade-off between keeping support for the existing feature and increasing app size. At least in the case of MAUI, it works around this issue by disabling trimming completely. If we allow trimming everything except for op_Implicit, we will already see a big improvement, there aren't that many implicit operators used in most apps.

I think this depends on how we set the goals. For example, there was a discussion of a trimming mode where types would always be kept in their entirety. So if one reflection-grabs a type, calling GetMethod/GetField/GetXXX on it will always work. The only trim-unsafe API would be the likes of Type.GetType, Assembly.Load, etc. It would be very compatible with existing .NET code - most people won't have to do anything, tons of unsafe pattern (including reflection serialization) will still work.

But it would not be possible to reach sizes competitive with non-.NET languages. If our goal is just to have sizes smaller than existing .NET, this would be a good addition. If our goal is to be competitive with non-.NET languages, additions like this make it harder to reach that goal.

The discussed behavior is closer to the "compatible, but we trim a lot less". It's a valid option if we consider it more important to not break existing code than to get the best trimming.

It would be good to collect some numbers on how much keeping all operators would cost us in size vs doing a breaking change in MAUI here and let them trim (e.g. we could make this so this code is only active for TrimMode=partial, but disabled in TrimMode=full, including F5 debugging). The comment at dotnet/maui#19922 (comment) seems to indicate this might not be very popular to begin with.

I am not sure if this wouldn't be a breaking change for some customers. I can imagine scenarios where the type isn't statically known, but the developer only calls it with type where they make sure that the method is preserved using [DynamicDependency] for example. The proposed API is meant as an explicit opt-in for this behavior.

Could you expand on the breaking change aspect? Is that about the warning suppression no longer suppressing the new warning? (It sounds solvable.)

@simonrozsival
Copy link
Member Author

If our goal is just to have sizes smaller than existing .NET, this would be a good addition. If our goal is to be competitive with non-.NET languages, additions like this make it harder to reach that goal.

The discussed behavior is closer to the "compatible, but we trim a lot less". It's a valid option if we consider it more important to not break existing code than to get the best trimming.

Yes, this proposal goes definitely in the "compatible, but we trim a lot less" direction. It seems like a reasonable solution to this problem to me, especially for LINQ, which already has this workaround in ILLink. I agree that especially in MAUI, where there's more flexibility in the API, we might want to be more ambitious and try to compete with the native solutions on both Android and iOS. I will measure the size difference of MAUI app with and without trimmed operators and I'll get back to you.

Could you expand on the breaking change aspect? Is that about the warning suppression no longer suppressing the new warning? (It sounds solvable.)

If I understood your previous idea that would increase the scope from operators to all methods correctly, calling obj.GetType().GetMethod("X") would preserve all methods called X across all types in an app. In the case where the developer already made the app trimming-safe using attributes, should this code start produce warnings? How would the developer opt out of the new trimming behavior of GetMethod?

class A { public void X() {} }
class B { public void X() {} } // previously B.X could be trimmed

[DynamicDependency("X", typeof(A))]
void Y(object value)
{
    var x = value.GetType().GetMethod("X");
    // ...
}

@MichalStrehovsky
Copy link
Member

If I understood your previous idea that would increase the scope from operators to all methods correctly, calling obj.GetType().GetMethod("X") would preserve all methods called X across all types in an app. In the case where the developer already made the app trimming-safe using attributes, should this code start produce warnings? How would the developer opt out of the new trimming behavior of GetMethod?

class A { public void X() {} }
class B { public void X() {} } // previously B.X could be trimmed

[DynamicDependency("X", typeof(A))]
void Y(object value)
{
    var x = value.GetType().GetMethod("X");
    // ...
}

They would not opt out. We always reserve the right to add intrinsic handling for cases that were previously not intrinsically handled; we don't create opt outs for those. The intrinsic handling is always correct with respect to the API and what is observable by trim safe code. It might end up more broad than what was done by a targeted suppression (that is no longer needed after the intrinsic handling). We sometimes keep broader set of things if there's not enough information.

@simonrozsival
Copy link
Member Author

@MichalStrehovsky Thanks for the clarification with Type.GetMethods and the general approach to intrinsics.

It would be good to collect some numbers on how much keeping all operators would cost us in size vs doing a breaking change in MAUI here and let them trim.

I collected the numbers for a basic MAUI app. I rooted all the implicit operators that are defined in the MAUI codebase using [DynamicDependency] and that changed the size of the final (compressed) bundle by 9.6 kB (0.17%), sizoscope shows difference in the code size of 19.6 kB. As Jonathan mentions in the issue, it's a niche feature.

@buyaa-n buyaa-n added this to the Future milestone Feb 7, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Reflection discussion enhancement Product code improvement that does NOT require public API changes/additions linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

No branches or pull requests

7 participants