Make DispatchProxy declares Property and EventInfos of the interface #9546
Conversation
|
Hi @yfakariya, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
|
@yfakariya, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
| foreach (MethodInfo mi in iface.GetRuntimeMethods()) | ||
| { | ||
| AddMethodImpl(mi); | ||
| var mdb = AddMethodImpl(mi); |
There was a problem hiding this comment.
Coding guidelines: "var" can only be used when the RHS unambiguously reveals the type ("new Foo()", "(IFoo)x").
|
Aside from the name suggestion, LGTM. Please squash into a single commit. |
|
cc @roncain |
yfakariya
force-pushed
the
dispatchproxy-emit-properties-and-events
branch
2 times, most recently
from
0d2309e to
935c847
Compare
|
I've re-push package with conflict resolution of |
|
cc @roncain Ron, are you planning any feedback on this? I don't have any further issues but I need a second signoff from within .dotnet. |
yfakariya
force-pushed
the
dispatchproxy-emit-properties-and-events
branch
from
935c847 to
a425ede
Compare
|
I've re-push package with conflict resolution of project.json of the test projects. No contents has been changed after 0d2309e without the project.json. If you are busy to review, please tell me. I hold back resolving the conflict if it is noisy for you. |
|
Looks like the repo updated project.json again so I'll need you to do one more conflict resolution and pre-push - then I'll merge the PR. Everyone has had days to give feedback so I'll take silence as consent. |
yfakariya
force-pushed
the
dispatchproxy-emit-properties-and-events
branch
from
a425ede to
5b4508a
Compare
This commit adds code to declare properties and events to generating proxy type as well. Note * Because declaring properties and events requires accessor methods, this commit changes AddMethodImpl to return created MethodBuilder. * This declares EqualityComparer for MethodInfo to avoid reference based comparison. * The test case now uses System.Reflection.Emit to generate the event with raise_ method which is not supported in C#.
yfakariya
force-pushed
the
dispatchproxy-emit-properties-and-events
branch
from
5b4508a to
e8f650d
Compare
|
Sure, I've re-pushed with conflict resolve. Thank you for gracious reviews and responses. |
ghost
merged commit 
| @@ -20,7 +22,6 @@ | |||
| "netstandard1.3": {} | |||
| }, | |||
| "supports": { | |||
| "coreFx.Test.netcore50": {}, | |||
There was a problem hiding this comment.
I suspect netcore50 needed to be removed because you took a dependency on Reflection.Emit. That will block these tests from running in .NET Native.
There was a problem hiding this comment.
Yes. It is required to test raise_Xxxbecause C# code cannot emit raise_ methods of events.
There was a problem hiding this comment.
OK at a future point we may need to split these tests into 2 projects to enable us to run the majority on both .NET Core and .NET Native.
There was a problem hiding this comment.
I see, thank you for follow-up.
This commit is implementation of issue #9220.
This adds code to declare properties and events to generating proxy type
as well as accessor methods.
Because declaring properties and events requires accessor methods,
this commit changes AddMethodImpl to return created MethodBuilder.