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

Cannot hit breakpoints in parameter mapped locations #43432

Open
NTaylorMullen opened this issue Apr 16, 2020 · 19 comments
Open

Cannot hit breakpoints in parameter mapped locations #43432

NTaylorMullen opened this issue Apr 16, 2020 · 19 comments

Comments

@NTaylorMullen
Copy link
Contributor

This ends up impacting Blazor scenarios where Blazor generates @currentCount as:

image

And then attempting to set a breakpoints results in a failure to bind or step over:
image

/cc @tmat

@CyrusNajmabadi
Copy link
Member

Does razor forward this call along to Roslyn?

@NTaylorMullen
Copy link
Contributor Author

Does razor forward this call along to Roslyn?

Sorry I don't understand the question, could you elaborate?

@jcouv
Copy link
Member

jcouv commented Apr 18, 2020

@NTaylorMullen Is this a new problem? I'm not sure what I would have expected, but probably nothing good. One issue is that breakpoints are set on statements (which the exception of switch expressions, just recently) but you're clicking on an expression and setting a breakpoint. I'm not sure which line map would count.

Tagging @tmat @ivanbasov @cston in case something to add.

@tmat
Copy link
Member

tmat commented Apr 18, 2020

I'll investigate.

@tmat tmat self-assigned this Apr 18, 2020
@NTaylorMullen
Copy link
Contributor Author

NTaylorMullen commented Apr 19, 2020

@NTaylorMullen Is this a new problem? I'm not sure what I would have expected, but probably nothing good. One issue is that breakpoints are set on statements (which the exception of switch expressions, just recently) but you're clicking on an expression and setting a breakpoint. I'm not sure which line map would count.

Nope not a new problem, it's always been there since Blazor's beginnings. Something we just haven't had more time to dig into until recently 😄

I'll investigate.

❤️ 😍 💪

@davidwengier
Copy link
Contributor

davidwengier commented Mar 25, 2021

@NTaylorMullen Can you confirm where this breakpoint is supposed to be in "real" C#? Your screenshot of the Counter.razor.g.cs file matches whats on my disk (default Blazor app template), but the tree we get from Razor looks like this:

// <auto-generated/>
#pragma warning disable 1591
namespace BlazorApp1.Pages
{
    #line hidden
    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Threading.Tasks;
    using Microsoft.AspNetCore.Components;
#nullable restore
#line 1 "e:/Scratch/BlazorApp1/_Imports.razor"
using System.Net.Http;

#line default
#line hidden
#nullable disable
#nullable restore
#line 2 "e:/Scratch/BlazorApp1/_Imports.razor"
using Microsoft.AspNetCore.Authorization;

#line default
#line hidden
#nullable disable
#nullable restore
#line 3 "e:/Scratch/BlazorApp1/_Imports.razor"
using Microsoft.AspNetCore.Components.Authorization;

#line default
#line hidden
#nullable disable
#nullable restore
#line 4 "e:/Scratch/BlazorApp1/_Imports.razor"
using Microsoft.AspNetCore.Components.Forms;

#line default
#line hidden
#nullable disable
#nullable restore
#line 5 "e:/Scratch/BlazorApp1/_Imports.razor"
using Microsoft.AspNetCore.Components.Routing;

#line default
#line hidden
#nullable disable
#nullable restore
#line 6 "e:/Scratch/BlazorApp1/_Imports.razor"
using Microsoft.AspNetCore.Components.Web;

#line default
#line hidden
#nullable disable
#nullable restore
#line 7 "e:/Scratch/BlazorApp1/_Imports.razor"
using Microsoft.JSInterop;

#line default
#line hidden
#nullable disable
#nullable restore
#line 8 "e:/Scratch/BlazorApp1/_Imports.razor"
using BlazorApp1;

#line default
#line hidden
#nullable disable
#nullable restore
#line 9 "e:/Scratch/BlazorApp1/_Imports.razor"
using BlazorApp1.Shared;

#line default
#line hidden
#nullable disable
    [Microsoft.AspNetCore.Components.RouteAttribute("/counter")]
    public partial class Counter : Microsoft.AspNetCore.Components.ComponentBase
    {
        #pragma warning disable 219
        private void __RazorDirectiveTokenHelpers__() {
        ((System.Action)(() => {
#nullable restore
#line 1 "e:/Scratch/BlazorApp1/Pages/Counter.razor"
global::System.Object __typeHelper = "/counter";

#line default
#line hidden
#nullable disable
        }
        ))();
        }
        #pragma warning restore 219
        #pragma warning disable 0414
        private static System.Object __o = null;
        #pragma warning restore 0414
        #pragma warning disable 1998
        protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder __builder)
        {
#nullable restore
#line 5 "e:/Scratch/BlazorApp1/Pages/Counter.razor"
             __o = currentCount;

#line default
#line hidden
#nullable disable
            __o = Microsoft.AspNetCore.Components.EventCallback.Factory.Create<Microsoft.AspNetCore.Components.Web.MouseEventArgs>(this, 
#nullable restore
#line 7 "e:/Scratch/BlazorApp1/Pages/Counter.razor"
                                          IncrementCount

#line default
#line hidden
#nullable disable
            );
        }
        #pragma warning restore 1998
#nullable restore
#line 9 "e:/Scratch/BlazorApp1/Pages/Counter.razor"
       
    private int currentCount = 0;

    private void IncrementCount()
    {

        currentCount++;
    }

#line default
#line hidden
#nullable disable
    }
}
#pragma warning restore 1591

I don't see that physical file anywhere, so I'm guessing this is the new source generator at work?

Given the above file though, it looks like Roslyn is doing the right thing. We're resolving the breakpoint for the statement __o = currentCount;, which comes from the right spot in the source razor file (ie, the line pragma is 5), so we return line 98, col 13..32. That then goes to the mapToDocumentRanges LSP method, which comes back with -1, -1.

I'm not exactly sure what is expected on the Razor side, and can't see too much in the debugger, but I did notice that in TryMapFromProjectedDocumentRangeStrict it calls codeDocument.GetCSharpDocument().GeneratedCode:
https://github.com/dotnet/aspnetcore-tooling/blob/067bd1bbfc26651af82be3b48d99e39799d456ba/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorDocumentMappingService.cs#L273

It then passes the mapped position from that to TryMapFromProjectedDocumentPosition which calls codeDocument.GetCSharpDocument() to get the mappings, but then doesn't find anything. Does GetCSharpDocument().SourceMappings and GetCSharpDocument().GeneratedCode refer to the same generated code? I can't see where they are defined is defined, so I'm not sure, I'm just guessing based on the fact that you pasted in the other generated code above. Is there supposed to be two?

@NTaylorMullen
Copy link
Contributor Author

@davidwengier We disabled this in the LSP editor so that you wouldn't even get the "red" breakpoint location. Try turning off the experimental Razor editor and do the same thing (it uses a different mechanism to map breakpoints)

As for what line it associates with that's:

#nullable restore
#line 5 "e:/Scratch/BlazorApp1/Pages/Counter.razor"
             __o = currentCount;

#line default
#line hidden

At design time and at runtime that __o goes away and is replaced with all sorts of __builder.Add... magic. Not sure which piece (runtime or designtime) matters

@tmat
Copy link
Member

tmat commented Mar 25, 2021

The IDE only sees design-time source when calculating break point locations when you place them.

@NTaylorMullen
Copy link
Contributor Author

The IDE only sees design-time source when calculating break point locations when you place them.

Oh totally get that part, I was more referring to what happens when the breakpoint is set to be "hit".

For instance, if you use the old editor you can set a breakpoint there where as in the new editor we restricted setting the breakpoint there because we didn't want to give false hope to folks.

Old Editor:
image

@tmat
Copy link
Member

tmat commented Mar 25, 2021

Breakpoints are likely to be quite broken, especially when you start editing the files in EnC. The fact that runtime does not match design-time will definitely throw it off.

@NTaylorMullen
Copy link
Contributor Author

Breakpoints are likely to be quite broken, especially when you start editing the files in EnC. The fact that runtime does not match design-time will definitely throw it off.

Sure but it works for all sorts of other things where it doesn't match 😄

@tmat
Copy link
Member

tmat commented Mar 25, 2021

Not sure what you mean. I'm just saying that we need to review EnC and see if its active statement mapping works for #line mapped spans. There might be issues there. Even then, once we fix those, it might turn out that supporting different runtime and compile time code is just not reasonable.

@davidwengier
Copy link
Contributor

davidwengier commented Mar 25, 2021

Alright, I'll turn off the LSP editor and see if that reveals any secrets.

It's worth nothing I wasn't doing any of this at runtime, this was just opening the razor file in the editor and hitting F9. Let me know if thats not the right repro.

@davidwengier
Copy link
Contributor

Okay, now I'm confused. With the LSP editor off, clicking to set a breakpoint on line 5 works. What am I missing here? Is this a problem when setting breakpoints, or at runtime?

RazorBreakpoint

@davidwengier
Copy link
Contributor

Alright, apologies for my confusion, the issue is at runtime the breakpoints aren't hit. The inability to create them in the LSP editor is a known thing that is irrelevant for this bug, for all intents and purposes.

@tmat
Copy link
Member

tmat commented Apr 5, 2021

This behavior in C# compiler is by design. The code that Razor emits can't work.

image

#line directives do not have an effect in the middle of a statement. They only affect the next sequence point. Sequence points can only be places on IL instruction where the evaluation stack is empty. Thus they can't be placed in the middle of statements and the #line directive emitted by Razor therefore has no effect.

@tmat
Copy link
Member

tmat commented Apr 5, 2021

To achieve the desired behavior we would need a new C# language feature:

#line <start line> <start column> <end line> <end column> <file name>

and then Razor could emit something like

#line 5 15 5 27 "Counter.razor"
__builder.AddContent(3, currentCount);
#line hidden

@tmat
Copy link
Member

tmat commented Apr 5, 2021

BTW, #line default immediately followed by #line hidden also has no effect since there is no sequence point in between these directives.

@NTaylorMullen
Copy link
Contributor Author

Ahhhh ok ya that sounds like a super awesome idea and also makes me think about #46526 too

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

No branches or pull requests

5 participants