PropertyChanged.Fody + MvvmCross 6.2 = crash
Closed this issue · 6 comments
Combining PropertyChanged.Fody with MvvmCross 6.2-beta results in crashing with System.InvalidProgramException: Invalid IL code in MyApp.Core.ViewModels.LoginViewModel:set_RememberMe (bool): IL_0020: ret
see: MvvmCross/MvvmCross#3016
Probable Solution: Properly handle Task-returning as well as void-returning RaisePropertyChanged()
.
@borbmizzet yep looks like a bug. happy to accept a PR that fixes this
@SimonCropp Unfortunately I have no idea where to even begin with this. I have close to zero experience with reflection, and none at all with .NET IL codes.
Update: I've spent the last few hours pouring through the source code and I'm no closer to figuring out how to fix this, aside from potentially requiring the addition of Task to TypeSystem.
start by adding your scenario to https://github.com/Fody/PropertyChanged/tree/master/TestAssemblies/AssemblyToProcess eg similar to https://github.com/Fody/PropertyChanged/tree/master/TestAssemblies/AssemblyToProcess/ClassWithIndirectImplementation.cs
and then add your expected outcome to a test eg https://github.com/Fody/PropertyChanged/blob/master/Tests/AssemblyToProcessTests.cs#L70
you should then be able to debug the test and step through the code to derive the next step. i suspect it will require a change in here https://github.com/Fody/PropertyChanged/blob/master/PropertyChanged.Fody/OnChangedWalker.cs
Thanks. I'll do that.
I'd say you need to modify this method so it returns IEnumerable<Instruction>
instead of Instruction
(fix the call sites, obviously), and if the method returns something other than System.Void
, then just add a pop
instruction after the callvirt
.
This would simply ignore any return value (even if not a Task
), which I think is the right behavior.
@ltrzesniewski That worked perfectly. Thanks!