-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Contribution idea: Refactor code generation to use Roslyn instead of CodeDOM #40
Comments
I'm curious about your thoughts on this. Is this about the orleans.codegen.cs files? Or about some other part? |
Yes, its about orleans.codegen.cs . We auto generate this file and we use CodeDom to do that. |
Yes, it's about orleans.codegen.cs. As you can see in the code, we use a mix of CodeDOM and code snippets to generate several categories of types: factories, grain references, invokers, state objects, serializers. There are multiple issues here:
|
+1 for a single grain factory. |
Regarding the intellisense, how do you see that working out with Roslyn? We still need an extra build step to make that work. I will do some investigating tomorrow, to get myself familiar with the code. Let's see if I can contribute... |
If we eliminate generation of per-interface factories and only support GrainFactory.GetGrain, then presumably we won't need IntelliSense beyond the normal. We'd still need a build step for generating other artefacts, unless we go all the way with 3. |
CodeDOM vs Roslyn is a false dichotomy: Roslyn does include some rather verbose syntax generation methods, but its also a fully managed compiler. Even just using the latter aspect with the current code gen being compiled at runtime instead of build time has its advantages. I am implying that we move away from XxxGrainFactory and use generic factory methods instead - which I think no one is arguing against. For anyone wanting to play with Roslyn CodeGen, Roslyn Quoter is very handy http://roslynquoter.azurewebsites.net/ |
A single factory with a generic method like A better way would be a generic factory that can be injected like this:
This way we save multiple problems:
|
I'm not sure about the progress on this, but I was able to recompile the ClientGenerator with Microsoft.CodeDom.Providers.DotNetCompilerPlatform, in order get Roslyn support. I had to remove a System.Runtime.dll from the options.ReferencedAssemblies in ClientGenerator.cs, and add a "using Microsoft.CodeDom.Providers.DotNetCompilerPlatform;" and remove Microsoft.CSharp and VisualBasic usings. |
Hi @Kittiepryde, my progress on this is represented by PR #528 (maybe you can review it 😄). This issue wasn't fully fleshed-out when it was opened, but the primary driver for a move to Roslyn is to allow us to generate code on-the-fly rather than requiring compile-time code generation. This obviates need for projects to contain MSBuild code and will hopefully help reduce the setup cost for new users as well as setup issues. Under #528, users are no longer required to separate their solution into multiple projects (they probably still should), and everything can be squished into one misguided file as in https://gist.github.com/ReubenBond/f7d7dd546e83a8a8856e. Finally, allowing for runtime code generation frees the project from the need to maintain code generators for each of the supported .NET languages (principally F# and VB). Most of that can be achieved using CodeDOM, but CodeDOM doesn't support all of the desired language features - for example, it doesn't support switch statements, which are featured prominently in the Orleans codegen (eg, in method invokers). I figured it was about time we rewrote code generation using a library which had full language support. Hopefully it is easier to change/add features using the new code generator. |
CodeDOM has been deprecated 5-6 years ago. That's why it doesn't support some of the new language feature, e.g. async/await, and most likely never will. |
Resolved by #528. |
PR Revamped TransactionConfiguration, DI enable TX class instantiation. dotnet#37 broke almost all of the transaction tests. This gets them working again.
No description provided.
The text was updated successfully, but these errors were encountered: