DuplicatesBody sometimes picks up patched method body

[awgil]

The following pattern doesn't always work as expected:

        [NewMember]
        [DuplicatesBody("foo")]
        public void ori_foo() { }

        [ModifiesMember("foo")]
        public void mod_foo()
        {
            ori_foo();
        }

This seems to be caused by AssemblyPatcher::UpdateMethods: the order in which it iterates over lists of NewMember & ModifiesMember is undefined. If it processes NewMember attributes first, everything works correctly. Otherwise it would duplicate mod_foo()'s body into ori_foo() (and cause infinite recursion).

Now, this is a bit strange - the comment in GetBodySource seems to imply that it should always get the unmodified body, regardless of the patching order. At least for me, it doesn't work like that - and looking at the code, I don't really understand why should it work differently.

A simple fix would be to always process NewMembers before ModifiesMembers by writing two loops.

[GregRos]

Interesting. The reason it was supposed to work is described in GetBodySource, though it's a bit arcane. The short reason is that I was trying to be clever.

The patching assembly has a reference to an unmodified copy of the target assembly, so resolving a reference to a thing from the patched assembly using the patching assembly should give you the definition of the thing from the original target assembly. So it shouldn't matter what modification is applied first, since we're not trying to get the body from the copy of the assembly being modified.

The process apparently doesn't work like I'd thought it did though, or maybe something else is going on.

It worked when I was worked on IE Mod and in some of my tests, but maybe I was just lucky.

I thought about doing things in a different order like you say, but it still introduced the possibility that the original method would be somehow changed by the steps before it, which might affect its body or something else.

---

More to the point, you can change the ordering by going to this file and ordering the thing being iterated over according to the attribute. You can use bool or int as the key. Something like this:

methodActions[typeof (ModifiesMemberAttribute), typeof (NewMemberAttribute)].OrderBy(x => KEY_OF(x))

I'm not sure if I'll be able to work on this library anytime soon. I don't even have a development environment set up for .NET nowadays. It's possible though, if I really feel like it. I'd love it if you made a pull request though.

[awgil]

If I understood your proposal correctly, it is to have manually specified "priority" field in the attribute? While this is indeed more flexible, it would mean that one has to explicitly specify the priority in each case where method needs to be copied + modified, which is easy to forget. On the other hand, simple ordering "new always before modified" should cover the vast majority of required cases.

Anyway, I've created pull request with a simpler approach.