-
Notifications
You must be signed in to change notification settings - Fork 4.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
add new compilation options to data sync to OOP. #32726
add new compilation options to data sync to OOP. #32726
Conversation
2 new options. NullableContextOptions and MetadataImportOptions if compiler team do this (dotnet#12795), IDE side can simply use compiler's serialization rather than doing it in IDE side
@dotnet/roslyn-ide @jinujoseph @jcouv can you take a look? |
You probably want to get the compiler team to actually assign this to someone (or just do that work yourself). it's unlikely to get done otherwise. :) |
@@ -95,6 +97,7 @@ protected void WriteCompilationOptionsTo(CompilationOptions options, ObjectWrite | |||
out bool concurrentBuild, | |||
out bool deterministic, | |||
out bool publicSign, | |||
out MetadataImportOptions metadataImportOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was there a reason to add this above the existing values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other values were using default not serialized/deserialized.
Moved #12795 to "Backlog". Please talk to Jared to schedule. |
tagging @jinujoseph for approval |
@jinujoseph the API is already in master. so, I think 16.0 should be fine. also, if they change something, it will let them to know that they have to change this as well. |
@@ -23,6 +21,7 @@ public override void WriteTo(CompilationOptions options, ObjectWriter writer, Ca | |||
var csharpOptions = (CSharpCompilationOptions)options; | |||
writer.WriteValue(csharpOptions.Usings.ToArray()); | |||
writer.WriteBoolean(csharpOptions.AllowUnsafe); | |||
writer.WriteByte((byte)csharpOptions.NullableContextOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whenever the serialization format is updated, is there some version number which should be incremented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serialization format version number is needed if one is used for persistence. otherwise, version is not needed.
Thanks! |
2 new options. NullableContextOptions and MetadataImportOptions if compiler team do this (dotnet#12795), IDE side can simply use compiler's serialization rather than doing it in IDE side
2 new options. NullableContextOptions and MetadataImportOptions
if compiler team do this (#12795), IDE side can simply use compiler's serialization rather than doing it in IDE side
fix #31870