Codegen for module initializers feature#43281
Codegen for module initializers feature#43281AlekseyTs merged 24 commits intodotnet:features/module-initializersfrom
Conversation
src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedRootModuleType.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedRootModuleTypeStaticConstructor.cs
Outdated
Show resolved
Hide resolved
|
I don't need to do anything with Answer: looks like we're going the no-synthesized-symbols route. |
|
Looks good to me so far, just have a bootstrap build failure that needs to be addressed. |
…e compiler produces
db5c01e to
46e61ef
Compare
|
@RikkiGibson I forgot VB :-) (fix) |
src/Compilers/VisualBasic/Portable/Compilation/MethodCompiler.vb
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Portable/Symbols/Attributes/AttributeData.cs
Outdated
Show resolved
Hide resolved
RikkiGibson
left a comment
There was a problem hiding this comment.
LGTM! Just suggest changing TODOs to the PROTOTYPE comment format we use in feature branches. This makes it a little easier to search and track down things we still need to do before merging the feature to master.
|
@dotnet/roslyn-compiler could I please get a senior review on this? |
|
I, personally, am not interested to see any intermediate wrong turns and typo fixes to be "burnt" into the history. We work on a specific, logically separate work item. Going through the process of trials and errors, etc. At the end we arrive to completion of the work item, that is what I would prefer us to record. I am not suggesting to squash PRs that integrate changes between branches, etc. Only PRs that are focused on specific work items, be it a bug fix or a logical step in a feature implementation. #Closed |
|
@AlekseyTs I think we view keeping the Git line history clean the same way. That's why I fix up commits for things like typos or anything else that is pure noise that doesn't document the changes. Depending on the intermediate wrong turn in question, I might or might not keep it around if I think it could potentially be interesting as future documentation. "This is what it looked like when we tried X." Anyway, there are no ideals, only tradeoffs, and what you say makes sense. |
|
@jnm2 If @RikkiGibson signs off, please let us know how do you want to go about merging this PR:
|
|
@AlekseyTs Squash merge is fine. Looking at the path these commits took, I'm not seeing anything of value that would be lost. |
Are we good to merge then or do you plan to make more changes in this PR? |
|
@dotnet/roslyn-infrastructure It looks like there is some kind of problem with roslyn-integration jobs |
|
@AlekseyTs I think it's ready to merge. |
|
@AlekseyTs Merge the latest master into your feature branch; that should have all the fixes. |
…yn into module_initializers/success_case
|
Thanks for your contribution @jnm2! |
|
@RikkiGibson @AlekseyTs Thanks very much both of you for all the help! |
Test plan: #40500
I have simple codegen working and would like feedback on the approach.
The two things I know of that remain are marked by TODO comments:
I haven't started looking at either of them yet. Would this PR be good to merge to the feature branch this way?