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

ActivatorUtilities: Do not wrap Delegate.Invoke in another delegate #105814

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

huoyaoyuan
Copy link
Member

#104731 (comment)

There's no conversion between delegates with same signature. Using Invoke method for another type will create another level of indirection.

Here the delegates can be created directly into ObjectFactory type.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 1, 2024
Copy link
Contributor

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

@steveharter
Copy link
Member

Thanks huoyaoyuan but I'm going to mark this "No Merge" until the main branch targets v10.0 which will happen soon.

This doesn't meet the bar to get it into v9.0.

@steveharter steveharter added this to the 10.0.0 milestone Aug 8, 2024
@steveharter steveharter added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 8, 2024
factoryExpressionBody, provider, argumentArray);

Func<IServiceProvider, object?[]?, object>? result = factoryLambda.Compile();
return result.Invoke;
ObjectFactory? result = factoryLambda.Compile();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for other reviewers: ObjectFactory delegate is declared here:

public delegate object ObjectFactory(IServiceProvider serviceProvider, object?[]? arguments);

@steveharter steveharter removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 21, 2024
@@ -324,11 +324,11 @@ public static ObjectFactory<T>
#endif
CreateFactoryInternal(typeof(T), argumentTypes, out ParameterExpression provider, out ParameterExpression argumentArray, out Expression factoryExpressionBody);

var factoryLambda = Expression.Lambda<Func<IServiceProvider, object?[]?, T>>(
var factoryLambda = Expression.Lambda<ObjectFactory<T>>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steveharter steveharter merged commit ba8e006 into dotnet:main Aug 21, 2024
81 of 84 checks passed
@huoyaoyuan huoyaoyuan deleted the delegate-invoke branch August 21, 2024 21:33
@github-actions github-actions bot locked and limited conversation to collaborators Sep 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-DependencyInjection community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants