Remove dependency on Type.IsSerializable from OptionSet serialization#54990
Conversation
|
|
||
| private enum OptionValueKind | ||
| { | ||
| Null, |
There was a problem hiding this comment.
Null and String and Serializable were merged into a single item called 'Object'
| } | ||
|
|
||
| var kind = OptionValueKind.Null; | ||
| object? valueToWrite = null; |
There was a problem hiding this comment.
no need to specially handle 'null', the default case below handles it.
There was a problem hiding this comment.
also, no need for 'valueToWrite'. we always just write out the literal value.
| @@ -215,57 +215,28 @@ public void Serialize(ObjectWriter writer, CancellationToken cancellationToken) | |||
| Debug.Assert(ShouldSerialize(optionKey)); | |||
There was a problem hiding this comment.
should view with whitespace off.
| if (type.IsEnum) | ||
| { | ||
| kind = OptionValueKind.Enum; | ||
| valueToWrite = (int)value; |
There was a problem hiding this comment.
this cast isn't necessary here due to teh line where we actually write the enum doing this already.
|
|
||
| case OptionValueKind.Null: | ||
| optionValue = null; | ||
| break; |
There was a problem hiding this comment.
no need for this. already handled by the object reading code.
|
|
||
| case ICodeStyleOption: | ||
| if (optionKey.Option.Type.GenericTypeArguments.Length != 1) | ||
| continue; |
There was a problem hiding this comment.
We just ignore options without single generic type param? Shouldn't this throw or something?
There was a problem hiding this comment.
i am adding this to the list of things to try out in my followup PR :) but yes. seems insane.
Previous code depended on both sides of the OOP communication channel agreeing on what types are serializable or not. new code does not.