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

Implement BindingFlags.DoNotWrapExceptions on Project N #4437

Merged
merged 1 commit into from Aug 31, 2017

Conversation

Projects
None yet
2 participants
@AtsushiKan
Contributor

AtsushiKan commented Aug 31, 2017

This was approved here.

dotnet/corefx#22866

Ok, this one actually makes the feature work. Turned
out not to be too hard.

There are a couple of drive-by items being done here:

  • Since the other Invoke overload on MethodInvoker
    was only for its use, downgraded its visibility.

  • Moved the catch in InvokeUtilites before the finally
    that copies back arguments. We don't want to wrap
    any exceptions out of the argument post-processing
    steps.

Implement BindingFlags.DoNotWrapExceptions on Project N
This was approved here.

dotnet/corefx#22866

Ok, this one actually makes the feature work. Turned
out not to be too hard.

There are a couple of drive-by items being done here:

- Since the other Invoke overload on MethodInvoker
  was only for its use, downgraded its visibility.

- Moved the catch in InvokeUtilites before the finally
  that copies back arguments. We don't want to wrap
  any exceptions out of the argument post-processing
  steps.

@AtsushiKan AtsushiKan merged commit 13ea6e4 into dotnet:nmirror Aug 31, 2017

@AtsushiKan AtsushiKan deleted the AtsushiKan:bfimpl branch Aug 31, 2017

akoeplinger added a commit to mono/mono that referenced this pull request Mar 29, 2018

Add BindingFlags.DoNotWrapExceptions (#7863)
The original design review is here: dotnet/corefx#22866 . To summarize, this adds a new flag to `BindingFlags` called `DoNotWrapExceptions`. When this flag is used to invoke a method using reflection, any exceptions thrown are not wrapped with a `TargetInvocationException`.

It has already been implemented in some other version of .NET:
- dotnet/corert#4437
- dotnet/coreclr#13767

I would be delighted if this handy feature was also available in Mono.

I have in part based my changes on the CoreCLR implementation. There all already tests for this feature in corefx, so I have added those to the corlib_xtest.dll.sources file.

I have a couple of concerns about my implementation:
- I notice that `DynamicMethod.Invoke` was ignoring the BindingFlags and other arguments. I changed this method to pass along all the arguments. For what it's worth, it appears that [CoreCLR respects](https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Reflection/Emit/DynamicMethod.cs#L488) these arguments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment