Skip to content
This repository has been archived by the owner on Dec 13, 2021. It is now read-only.

Fix proxy generation #225

Merged
merged 3 commits into from
Sep 5, 2017
Merged

Fix proxy generation #225

merged 3 commits into from
Sep 5, 2017

Conversation

JimBobSquarePants
Copy link

So it turns out setting properties for proxied instances was broken for value types.

We got away with it because we actually had two instances of the mapped type (one proxied and one not) on the go at one time. (obviously not efficient) and were setting the property on the hidden copy.

This PR fixes that and also changes the interceptor to only attempt to intercept the lazy properties (as removing the copy would create an infinite recursion of interception)

We also don't now attempt to create a proxy from a non-null instance as that would simply replace the instance.

@JimBobSquarePants JimBobSquarePants requested review from mattbrailsford and leekelleher and removed request for mattbrailsford September 4, 2017 12:43
Copy link
Collaborator

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

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

All looks good to me.

I admit that I look over the PropertyEmitter amends and think "I'm trust @JimBobSquarePants knows what's going on in here" 😎

The rest of the C#6 amends are cool with me 👍

@JimBobSquarePants
Copy link
Author

Reflection.Emit is hard, very hard but I understand it enough to write something simple-ish like this. Anything more complicated though would require a lot of swearing!

Protip: If you ever end up giving this kind of code a go Teleriks Just Decompile is leagues ahead of Jetbrains DotPeek. Gives you tons to work with viewing output and the Peverify.exe tool in the Windows SDK tells you exactly what is wrong when you mess up.

I added tests also with a demo very similar to what the reflection emit code actually produces so we know it works also.

@leekelleher leekelleher merged commit ebad9ee into develop Sep 5, 2017
@leekelleher leekelleher deleted the fix-proxy branch September 5, 2017 09:23
@leekelleher
Copy link
Collaborator

Merged in, thanks @JimBobSquarePants! 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants