Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HELP - BREAKS VS DEBUGGER #14

Open
Planet99 opened this issue Mar 28, 2019 · 10 comments
Open

HELP - BREAKS VS DEBUGGER #14

Planet99 opened this issue Mar 28, 2019 · 10 comments

Comments

@Planet99
Copy link

Been using this package for a couple years now. This has been plaguing me for the last few weeks and unfortunately it is mist. Set a breakpoint at either sb.Append below. When you get there, a hover over sb will say it is null - it is not. Duplicated variable names in different scopes is what is causing this issue - different names work. Comment out the one property - and it works. We have 10 years of code (not by me and not close to mvvm) which is being re-purposed and I have been porting mist into the new derivatives. The debugger is near useless to try to understand old code when you can't trust it.

Note: don't know why this isn't formatting correctly.

` [Notifier(NotificationMode.Implicit)]
public partial class MainWindow : Window, INotifyPropertyChanged
{
public MainWindow()
{
InitializeComponent();
int x = 3;
if (x > 12)
{
var sb = new StringBuilder();
sb.Append($"{x}");
sb.ToString();
}
else
{
var sb = new StringBuilder();
sb.Append($"{x}");
sb.ToString();
}
}
// comment out this property and debugger works
public bool Checked { get; set; }

    public event PropertyChangedEventHandler PropertyChanged;
    [NotifyTarget]
    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) => PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
}

`

This is absolutely killing me. I am the only person at this company who is not afraid of extensions or nuget. Every time I have an issue they say "well what extension do you have that is causing it because it doesn't happen to me". If I tell them I have to rip this out - they will simply stop me from using anything but vanilla everything.

I love this thing and am using it against the caution from my boss and a couple of my coworkers hate it. (Good guys, mechanical engineers who write code, but very conservative when it comes to SW)

@Planet99
Copy link
Author

Planet99 commented Apr 2, 2019

FYI. I tried Fody's PropChange and it does not exhibit this issue. I have to admit I like what you have here. I have a custom VSIX to add your dll, ref and task to selected csproj so you'r pretty integrated in my environment.

Just a note. The immediate window doesn't work for anything but locals when Mist in project. Fody had the same issue and still might. So I'm guessing Mist is causing that too.

Thanks

@aquamoth
Copy link
Contributor

aquamoth commented Apr 3, 2019

This is actually a bug in the old version of Mono.Cecil we are using.
I have successfully upgraded to Cecil 0.10.3 on my dev.machine and passed all tests.
Will clean up the code and push later.

@Mart-Bogdan Can you create a new nuget when I'm done. I don't have nuget push access on this repo.

@aquamoth
Copy link
Contributor

aquamoth commented Apr 3, 2019

Actually turns out I dont have push access any more either, so Ill just send a PR I guess...

@Planet99
Copy link
Author

Planet99 commented Apr 3, 2019

Thanks @aquamoth .
I just built with cecil 11 and the issues still seem to occur. Getting an error (method not found loading cecil project) when trying with 10.3. I'll try to find some time to play with it some more and await the release.

@aquamoth
Copy link
Contributor

aquamoth commented Apr 4, 2019

I've created a prerelease branch in my fork here: https://github.com/aquamoth/MIST/releases/tag/v2.5.2%2Bissue-14

I attached a compiled version of the mist builder for .net 4.7 for convenience.

Next I will make a formal PR and see this gets integrated in the official master

@aquamoth
Copy link
Contributor

aquamoth commented Apr 5, 2019

@Planet99 Can you please confirm if the posted fix solved your problem or not?
If not, please give us some more details.

@Planet99
Copy link
Author

Planet99 commented Apr 5, 2019

Still busted. I have a simpler rerpo. BP on the 2nd write will show i=13 in the debugger hover, pinned
tooltip and watch as you step through. It IS CORRECT in the locals window.

    void MistBug()
    {
        for (int i = 10; i < 13; i++)
            Console.WriteLine(i);
        for (int i = 10; i < 13; i++)
            Console.WriteLine(i);
    }
    // comment out this property and debugger works
    public bool Checked { get; set; }

Here is the good IL. The bad IL is identical except for 1 place (and 2 in the comments) shown between ** and **. One little (seemingly unimportant) underscore. Hmm. Very strange.

Hope it helps.

Thanks
`
.method private hidebysig instance void
MistBug() cil managed
{
.maxstack 2
.locals init (
[0] int32 i,
[1] bool V_1,
[2] int32 i_V_2,
[3] bool V_3 ** ONLY FUNCTIONAL DIFFERENCE [3] bool _V_3 **
)

// [36 9 - 36 10]
IL_0000: nop

// [37 18 - 37 28]
IL_0001: ldc.i4.s     10 // 0x0a
IL_0003: stloc.0      // i

IL_0004: br.s         IL_0011
// start of loop, entry point: IL_0011

  // [38 17 - 38 38]
  IL_0006: ldloc.0      // i
  IL_0007: call         void [mscorlib]System.Console::WriteLine(int32)
  IL_000c: nop

  // [37 38 - 37 41]
  IL_000d: ldloc.0      // i
  IL_000e: ldc.i4.1
  IL_000f: add
  IL_0010: stloc.0      // i

  // [37 30 - 37 36]
  IL_0011: ldloc.0      // i
  IL_0012: ldc.i4.s     13 // 0x0d
  IL_0014: clt
  IL_0016: stloc.1      // V_1

  IL_0017: ldloc.1      // V_1
  IL_0018: brtrue.s     IL_0006
// end of loop

// [39 18 - 39 28]
IL_001a: ldc.i4.s     10 // 0x0a
IL_001c: stloc.2      // i_V_2

IL_001d: br.s         IL_002a
// start of loop, entry point: IL_002a

  // [40 17 - 40 38]
  IL_001f: ldloc.2      // i_V_2
  IL_0020: call         void [mscorlib]System.Console::WriteLine(int32)
  IL_0025: nop

  // [39 38 - 39 41]
  IL_0026: ldloc.2      // i_V_2
  IL_0027: ldc.i4.1
  IL_0028: add
  IL_0029: stloc.2      // i_V_2

  // [39 30 - 39 36]
  IL_002a: ldloc.2      // i_V_2
  IL_002b: ldc.i4.s     13 // 0x0d
  IL_002d: clt
  IL_002f: stloc.3      // V_3                **  IL_002f: stloc.3      // _V_3  **

  IL_0030: ldloc.3      // V_3              ** IL_0030: ldloc.3      // _V_3 **
  IL_0031: brtrue.s     IL_001f
// end of loop

// [41 9 - 41 10]
IL_0033: ret

} // end of method MainWindow::MistBug
`

@Planet99 Planet99 closed this as completed Apr 5, 2019
@Planet99
Copy link
Author

Planet99 commented Apr 5, 2019

Oops. Sorry

@Planet99 Planet99 reopened this Apr 5, 2019
@Planet99
Copy link
Author

Hey All, I spent a bit of time looking at this but decided to go with Fody/PropertyChanged. I understand this was not the kind of bug that one would have tested for at first. I really liked using it but Fody also opens the door to other weavers. Thanks for taking the time.

@mathtone
Copy link
Owner

I do plan to push another version out shortly which I hope will resolve this behavior, I'm sorry I wasn't able to get to the issue at the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants