Skip to content

C#: Generated code contains compilation errors (+ lots of other opinionated problems) #1265

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

Closed
just-ero opened this issue Apr 8, 2025 · 9 comments · Fixed by #1275
Closed
Labels
gen-c# Related to the C# code generator

Comments

@just-ero
Copy link
Contributor

just-ero commented Apr 8, 2025

Hello. I've used https://github.com/bytecodealliance/componentize-dotnet for the first time today. I ended up incredibly disappointed.

The main problem is a generated file containing some Result-related definitions. In the code, two ArgumentExceptions are thrown. There is no reference to the System namespace, such that compiling the library results in error CS0246: The type or namespace name 'ArgumentException' could not be found.


I have multiple other things I would like to address and discuss. Please let me know if I should take this somewhere else.

  1. Naming.
    • The generated code contains namespaces with parts which are all lowercase. This is unacceptable in C# and actually results in errors in all of my codebases.
    • The generated namespaces use the target world's name (world foo -> namespace FooWorld). This means it does not use the containing library's root namespace. This, along with generating FooWorld.wit.imports.sample.pkg, takes a lot of control out of the user's hands. I do not want the code to be generated in these namespaces.
  2. Types and declarations.
    • In simple examples, Result<TOk, TErr> and None (as well as IFooWorld and FooWorld) are not used at all. Why are these generated in those cases?
    • Why is Result<TOk, TErr>.Tags generated? The constants (if required at all) can be declared at the class level in Result<TOk, TErr>.
    • Why are these all declared as public? I do not want these types to be part of my library. A consumer of my library should never know about internal implementation details. Can I configure this?
      I may even want to wrap the generated imports and exports further. Everything being public is a nuisance for this purpose.
  3. DllImport module and entry point name.
    • My host app does not export functions in sample:pkg/foo, but instead in env. The generated code always uses [DllImport("sample:pkg/foo", ...), ...]. Can I configure this?
    • My host app does not export functions as kebab-case, but snake_case. The generated code always uses kebab-case. Can I configure this?
  4. This should be a source generator.
    • The fact that this is a separate software from componentize-dotnet is a huge problem for this point. An incremental source generator using additional files provided by the user would be much more convenient to use. The code would be generated while the user types, not upon executing a build. This would require the bindings generation to be done completely in C#, but is a lot more pleasant to use.
@just-ero just-ero changed the title C#: Generated code contains compilation errors C#: Generated code contains compilation errors (+ lots of other opinionated problems) Apr 8, 2025
@just-ero
Copy link
Contributor Author

just-ero commented Apr 8, 2025

The reason the main issue has not been caught until now is likely because all testing has been done with ImplicitUsings set to true, which globally includes System. Removing ImplicitUsings (or setting it false) results in the missing reference.

@jsturtevant jsturtevant added the gen-c# Related to the C# code generator label Apr 8, 2025
@jsturtevant
Copy link
Collaborator

Thanks for trying it out and giving feedback. Getting it to work in a larger project does have some rough edges but your feedback will help the experience improve. There are only a few of us working on it at the moment so feel free to jump in and help out. I am going to try to answer some of the questions and will probably want to break out some of the requests to seperate issues to make tracking them easier.

The reason the main issue has not been caught until now is likely because all testing has been done with ImplicitUsings set to true, which globally includes System. Removing ImplicitUsings (or setting it false) results in the missing reference.

This is definitely a bug we should address.

  1. Naming

The generated code contains namespaces with parts which are all lowercase. This is unacceptable in C# and actually results in errors in all of my codebases.

We had done some cleanup but I guess there are some more (#1075). If you have explicit examples that would help.

The generated namespaces use the target world's name (world foo -> namespace FooWorld). This means it does not use the containing library's root namespace. This, along with generating FooWorld.wit.imports.sample.pkg, takes a lot of control out of the user's hands. I do not want the code to be generated in these namespaces.

What name spacing would you like to see? I guess we might be able to provide some ability to customize, I am not sure the original reasoning for the this namespace. @silesmo any thoughts?

  1. Types and Declarations

There is a lot we need to do here to make this usable including putting types like Result and Option in a resulable library (#1073)

In simple examples, Result<TOk, TErr> and None (as well as IFooWorld and FooWorld) are not used at all. Why are these generated in those cases?
Why is Result<TOk, TErr>.Tags generated? The constants (if required at all) can be declared at the class level in Result<TOk, TErr>.

These are just need to be cleaned up. The goal was to get things working, then improve the ergonomics.

Why are these all declared as public? I do not want these types to be part of my library. A consumer of my library should never know about internal implementation details. Can I configure this?
I may even want to wrap the generated imports and exports further. Everything being public is a nuisance for this purpose.

you can customize this with --internal when running the generation.

  1. DllImport module and entry point name

DllImport module and entry point name.
My host app does not export functions in sample:pkg/foo, but instead in env. The generated code always uses [DllImport("sample:pkg/foo", ...), ...]. Can I configure this?

This is taken from the wit since its the naming that is exported in the component and I don't believe it is configurable unless you change the wit otherwise it won't match.

My host app does not export functions as kebab-case, but snake_case. The generated code always uses kebab-case. Can I configure this?

This is from the wit. Maybe need a example to understand deeper but here I think we are just following what is defined in wit.

  1. source generator

We discuss this early on when we first launched the project. See for a full discussion dotnet/runtimelab#2409.

@just-ero
Copy link
Contributor Author

just-ero commented Apr 8, 2025

I'm unfortunately not familiar with Rust at all. Though I am very familiar with C#, auto-generated C# code, common practices, low level code and interop. I would love to help, but the current code is somewhat overwhelming.

Is there some place I could join some discussion on this?

@jsturtevant
Copy link
Collaborator

Yes! The c# group meets every other Wednesday: https://github.com/bytecodealliance/meetings/tree/main/SIG-Guest-Languages/Csharp. There is also zulip channel: #C#/.net-collaboration

@silesmo
Copy link
Member

silesmo commented Apr 16, 2025

There will be a meeting today at 5PM CEST. Here is the link to join: https://zoom.us/j/95573248270?pwd=LXFt63RShQbx7xgTl3LsFY26Z2F7oz.1 @just-ero

@silesmo
Copy link
Member

silesmo commented Apr 16, 2025

What name spacing would you like to see? I guess we might be able to provide some ability to customize, I am not sure the original reasoning for the this namespace. @silesmo any thoughts?

Initially it generated a namespace that was the same as the wit world meaning foo would result in Foo instead of FooWorld
There are also exported functions that are put in the nested namespaces and we didn't want to pollute the root namespace with all those generated functions that would never be directly used by the C# side of the code. But I'm more than happy to look into changing this. How should it be structured from your perspective @just-ero?

And also thanks a lot for the feedback, it's very appreciated and we will try and address your concerns as best we can!

We had a discussion around using C# source generators over a year ago when we started the group, but decided to continue down this route as we already had something working but also we didn't have the manpower to rebuild it from scratch. The view from the dotnet team didn't have such a strong concern about it not being a C# source generator as well at the time. But who knows, maybe we can re-evaluate this in the future, especially if there are new contributors willing to assist with the required work.

@just-ero
Copy link
Contributor Author

I would take a look at the LibraryImport generator for the C# BCL and follow most of its examples.
This means using local functions for the DllImports, fully qualified type names, Utf8StringMarshaller for marshalling strings, etc.

I can get started on using the fully qualified type name for all the types used in the generator and removing the generation of usings. It's a fairly simple PR and would close this issue.

@pavelsavara
Copy link
Collaborator

We had a discussion around using C# source generators over a year ago when we started the group, but decided to continue down this route as we already had something working but also we didn't have the manpower to rebuild it from scratch. The view from the dotnet team didn't have such a strong concern about it not being a C# source generator as well at the time. But who knows, maybe we can re-evaluate this in the future, especially if there are new contributors willing to assist with the required work.

Source generators is good when the source metadata is in C# also. Otherwise it's just making the Roslyn slower and VS users hate it.
Also source generator syntax for emitting code is not convenient at all. It's designed to live inside compiler, not to be good templating language.

We could implement another generator as separate executable tool. Doing it in C# would lower the barrier for C# people to fix the generator.

But there are not that many people willing to learn WIT details and for those who are, it's probably OK to learn bit of Rust instead.
Also I think we should stick to wit-bindgen in Rust, because otherwise we would have to maintain the parser of WIT and the many things that this code does already. Like calculating stack stack sizes and alignments or resolving bindings.

@just-ero
Copy link
Contributor Author

I concur. After talking to someone from the Roslyn team, it appears I was incorrect in my assessment over whether a source generator is appropriate for this situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gen-c# Related to the C# code generator
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants