hazzik / DelegateDecompiler

A library which is able to decompile a delegate or a method body to its lambda representation
MIT License
526 stars 62 forks source link

Fix calls to base implementation decompilation #124

Closed magicmoux closed 5 years ago

magicmoux commented 6 years ago

@hazzik,

Here is an up-to-date PR that should solve the calls to base method/properties implementations issue I talked about in #121

I annotated the relevant changes so you don't get confused by all the code cleaning changes (sorry about these but I use CodeMaid on save) Max.

hazzik commented 6 years ago

@magicmoux could you please explain what is the desired behaviour? I'm confused. Better with examples.

magicmoux commented 6 years ago

@hazzik what is the confusion about? The examples should be clear, the previous implementation should raise stackoverflow exceptions when calling base.xxx on computed membres. This pr should solve the problem and décompile the base implementations recursively instead.

Any way to contact you directly?

hazzik commented 6 years ago

@magicmoux I’ve made some changes on develop branch. Could you check if these changes work in your scenarios?

magicmoux commented 6 years ago

Hi @hazzik,

I've checked your implementation and I found out that it works only in one direction, ie. when you invoke the virtual call on a super-class. When called directly on a subclass, calls to base won't be processed since there are not added to the baseCalls dictionary beforehand.

You can try with the following test :

        Expression<Func<D, string>> e = @this => "D is child of " + ("C is child of " + "A".ToString()).ToString();
        Test(e, typeof(D).GetMethod(nameof(D.M6)));

If I've understood your intent correctly, I have to add the following code in MethodBodyDecompiler.cs @ before line 67 for my tests to succeed:

        var baseCalls = AppDomain.CurrentDomain.GetAssemblies()
            .Where(a => !a.IsDynamic)
            .SelectMany(a => SafeGetTypes(a))
            .Where(t => t.IsAssignableFrom(declaringType) && t != declaringType)
            .Select(t => GetDeclaredMethod(t, method))
            .Where(m => m!=null && !m.IsAbstract)
            .ToDictionary(m => m, m => DecompileConcrete(m, args));

There is however a drawback to this approach : it will force concrete decompilation of base methods whether they are used or not, contrary to mine where only necessary base methods will be decompiled.

magicmoux commented 6 years ago

On afterthought, that should not cause much overhead in most scenarii, but i didn't really check how the main cache works, so many decompilations could have négative impact on real environments

magicmoux commented 6 years ago

Hi, @hazzik still no conclusion on this problem? I think this is one of the major issues left that need to be resolved to reach a fully functional version.