Skip to content

Conversation

serhiiz
Copy link

@serhiiz serhiiz commented Mar 20, 2024

It seems the documentation on System.Reflection.MethodBase.Invoke in regard to the invokeAttr parameter is outdated.
According to the documentation:

invokeAttr BindingFlags
A bitmask that is a combination of 0 or more bit flags from BindingFlags. If binder is null, this parameter is assigned the value Default; thus, whatever you pass in is ignored.

the invokeAttrs parameter is ignored when the binder parameter is null. However, that doesn't seem to be accurate. According to the implementations (System.Reflection.RuntimeMethodInfo.cs, System.Reflection.Emit.DynamicMethod.CoreCLR.cs, System.Reflection.Emit.DynamicMethod.CoreCLR.cs), the invokeAttrs is never ignored. Unless I'm missing something.

Summary

The suggestion is to take out the statement about invokeAttr being ignored when the binder is null.

…out invokeAttrs being ignored when binder was null
@serhiiz serhiiz requested a review from a team as a code owner March 20, 2024 03:34
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 20, 2024
Copy link

Learn Build status updates of commit 5cd4e9d:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Reflection/MethodBase.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@steveharter
Copy link
Contributor

LGTM; the only flag that would be used without a binder is BindingFlags.DoNotWrapExceptions but I don't think calling that out is necessary.

@steveharter steveharter self-requested a review March 20, 2024 20:45
@steveharter steveharter merged commit ba8b070 into dotnet:main Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Reflection 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.

4 participants