-
Notifications
You must be signed in to change notification settings - Fork 4.9k
1) Adding support for excluding [NonSerialized] members from serialization, this is prerequisite for #2; 2) Adding support for ReadOnlyDictionary. #6141
Conversation
| "System.Collections.Stack", | ||
| "System.Globalization.CultureInfo", | ||
| "System.Version", | ||
| private static string[] s_knownSerializableGenericTypeNames = new string[] { |
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.
Why is parsing this and converting it to a Dictionary even necessary? Why not a pre-parsed dictionary? e.g.:
private static readonly Dictionary<string, string[]> s_knownSerializableGenericTypes = new Dictionary<string, string[]>
{
{ "System.Collections.Generic.KeyValuePair`2", Array.Empty<string>() },
{ "System.Collections.Generic.Queue`1", new[] { "_syncRoot" } },
{ "System.Collections.Generic.Stack`1", new[] { "_syncRoot" } },
{ "System.Collections.ObjectModel.ReadOnlyCollection`1", new[] { "_syncRoot" } },
{ "System.Collections.ObjectModel.ReadOnlyDictionary`2", new[] { "_syncRoot", "_keys", "_values" } },
{ "System.Tuple`1", Array.Empty<string>() },
... etc. ...
};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.
Also, why not just have a single Dictionary for both generic and non-generic types...
Then change IsKnownSerializableType to something like:
internal static bool IsKnownSerializableType(Type type)
{
// Applies to known types that DCS understands how to serialize/deserialize
//
TypeInfo typeInfo = type.GetTypeInfo();
string fullName = typeInfo.IsGenericType && !typeInfo.IsGenericTypeDefinition ?
typeInfo.GetGenericTypeDefinition().FullName :
type.FullName;
return s_knownSerializableTypes.ContainsKey(fullName) ||
Globals.TypeOfException.IsAssignableFrom(type);
}c560f98 to
55e8fc5
Compare
| { "System.Collections.Generic.Queue`1", new[] { "_syncRoot" } }, | ||
| { "System.Collections.Generic.Stack`1", new[] {"_syncRoot" } }, | ||
| { "System.Collections.ObjectModel.ReadOnlyCollection`1", new [] {"_syncRoot" } }, | ||
| { "System.Collections.ObjectModel.ReadOnlyDictionary`2", new [] {"_syncRoot", "_keys","_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.
Nit: for consistency, in the above two lines, new [] can be new[] (without the space between new and []).
55e8fc5 to
1a2a913
Compare
| private static string GetGeneralTypeName(Type type) | ||
| { | ||
| TypeInfo typeInfo = type.GetTypeInfo(); | ||
| return type.GetTypeInfo().IsGenericType && !typeInfo.IsGenericParameter |
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.
Sorry, I missed this previously: type.GetTypeInfo().IsGenericType -> typeInfo.IsGenericType
…ation, this is prerequisite for dotnet#2. 2) Adding support for ReadOnlyDictionary.
1a2a913 to
b86016c
Compare
|
LGTM |
|
LGTM. |
|
@dotnet-bot test this please |
2 similar comments
|
@dotnet-bot test this please |
|
@dotnet-bot test this please |
|
@dotnet-bot test Innerloop OSX Release Build and Test |
1) Adding support for excluding [NonSerialized] members from serialization, this is prerequisite for #2; 2) Adding support for ReadOnlyDictionary.
1) Adding support for excluding [NonSerialized] members from serialization, this is prerequisite for dotnet/corefx#2; 2) Adding support for ReadOnlyDictionary. Commit migrated from dotnet/corefx@650b049
@shmao @khdang @zhenlan