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

JavaScript interop source generator should emit #line directives pointing to the originating declaration #107577

Open
tmat opened this issue Sep 9, 2024 · 7 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm
Milestone

Comments

@tmat
Copy link
Member

tmat commented Sep 9, 2024

Description

The source generator emits non-user code methods without #line directives. That's usually ok, since the code is not expected to be stepped into or compiler errors are not expected to occur in it. However, changing the declaration of a partial method stub annotated by JSImport attribute during EnC/Hot Reload might result in rude edits. Currently, these diagnostics are reported with locations pointing to the source-generated file.

Adding #line directives that map the locations back to the originating declaration would improve the experience. The IDE would then show the errors in the source file where the user is actually making the changes.

Reproduction Steps

  1. dotnet new blazorwasm -o testblazor01
  2. cd testblazor01
  3. In testblazor01.csproj add the following:
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
  1. In Program.cs, add the following at the end:
 public static partial class Test
 {
     [System.Runtime.InteropServices.JavaScript.JSImport("test")]
     public static partial void F(int a);
 }
  1. Open the project in VS and start debugging (F10)
  2. Change the signature of Test method to:
public static partial void F(int a, int b);
  1. Open Hot Reload pane of Output window

Expected behavior

[Error] C:\Test\testblazor01\Program.cs (line 18): error ENC0044: Modifying method which contains the stackalloc operator requires restarting the application.
[Error] C:\Test\testblazor01\Program.cs (line 18): error ENC0020: Renaming field '__signature_F_583343223' requires restarting the application.

Actual behavior

[Error] Microsoft.Interop.JavaScript.JSImportGenerator\Microsoft.Interop.JavaScript.JSImportGenerator\JSImports.g.cs (line 12): error ENC0044: Modifying method which contains the stackalloc operator requires restarting the application.
[Error] Microsoft.Interop.JavaScript.JSImportGenerator\Microsoft.Interop.JavaScript.JSImportGenerator\JSImports.g.cs (line 26): error ENC0020: Renaming field '__signature_F_583343223' requires restarting the application.

Regression?

No

Known Workarounds

No response

Configuration

No response

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Sep 9, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

@jkoritzinsky
Copy link
Member

If we were to emit line directives, how would that interact with stepping? A huge part of the choice to use source generators for interop was to be able to step through the generated code and see the C# for what's happening.

Would it be possible to only emit line directives for the signature and not the body to get the right experience?

@tmat
Copy link
Member Author

tmat commented Sep 9, 2024

Emitting it only for the signature wouldn't help. We don't have currently any means to make stepping work with #line directives that would remap to the original definition. I guess it's a matter of preference/priority: if stepping is important than the current state is better.

@jkoritzinsky
Copy link
Member

I think a decent compromise here for the JavaScript generators would be to remap the line that defines the signature field back to the original declaration's line as there's no interesting code to step there and the rude edit error itself doesn't let you easily trace back to the original method. That way the rude edit would point to user code and stepping would still work.

@tmat
Copy link
Member Author

tmat commented Sep 10, 2024

I guess we can try that. We would also want to remap all field definitions. That would likely cover most rude edits.

@lewing lewing added arch-wasm WebAssembly architecture os-browser Browser variant of arch-wasm labels Sep 10, 2024
Copy link
Contributor

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

@lewing lewing added this to the 10.0.0 milestone Sep 10, 2024
@lewing lewing removed the untriaged New issue has not been triaged by the area owner label Sep 10, 2024
@ilonatommy ilonatommy self-assigned this Sep 10, 2024
@ilonatommy
Copy link
Member

Using the reproduction steps we're getting a slightly different error:

[Error] C:\Users\testblazor01\testblazor01.sln (line 1): error ENC1002: Cannot apply changes -- unexpected error: 'Partial method implementation required (Parameter 'oldSymbol')'

When we use wasmbrowser instead of blazor, we are able to reproduce the quoted error:

 [Error] Microsoft.Interop.JavaScript.JSImportGenerator\Microsoft.Interop.JavaScript.JSImportGenerator\JSImports.g.cs (line 2): error ENC0033: Deleting field '__signature_F_55838023' requires restarting the application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm
Projects
Status: No status
Development

No branches or pull requests

5 participants