Skip to content

Commit

Permalink
DataContractSerialization cleanup (#41824)
Browse files Browse the repository at this point in the history
* Clean up DataContractSerialization

- Remove DiagnosticUtility.IsFatal, which always returns false.
- Remove dead CreateDelegate method
- Fix a TODO-NULLABLE where MemberInfo will never be null

* Clean up remaining #if USE_REFEMIT and other unused defines in DataContractSerialization
  • Loading branch information
eerhardt authored Sep 4, 2020
1 parent 300df46 commit a196d53
Show file tree
Hide file tree
Showing 16 changed files with 21 additions and 519 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -170,20 +170,4 @@ private static Setter CreateSetterInternal<DeclaringType, PropertyType>(Property
}
}
}

internal static class CreateDelegateExtension
{
// a generic extension for CreateDelegate
public static T CreateDelegate<T>(this MethodInfo method) where T : class
{
try
{
return (method.CreateDelegate(typeof(T)) as T)!;
}
catch (Exception e)
{
throw new InvalidOperationException(SR.Format(SR.FailedToCreateMethodDelegate, method.Name, method.DeclaringType!.FullName), e);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,6 @@ private static MethodInfo StringFormat

private Type _delegateType = null!; // initialized in BeginMethod

#if USE_REFEMIT
AssemblyBuilder assemblyBuilder;
ModuleBuilder moduleBuilder;
TypeBuilder typeBuilder;
static int typeCounter;
MethodBuilder methodBuilder;
#else
private static Module? s_serializationModule;
private static Module SerializationModule
{
Expand All @@ -109,7 +102,6 @@ private static Module SerializationModule
}
}
private DynamicMethod _dynamicMethod = null!; // initialized in BeginMethod
#endif

private ILGenerator _ilGen = null!; // initialized in BeginMethod
private List<ArgBuilder> _argList = null!; // initialized in BeginMethod
Expand All @@ -128,7 +120,6 @@ internal CodeGenerator()
_codeGenTrace = CodeGenTrace.None;
}

#if !USE_REFEMIT
internal void BeginMethod(DynamicMethod dynamicMethod, Type delegateType, string methodName, Type[] argTypes, bool allowPrivateMemberAccess)
{
_dynamicMethod = dynamicMethod;
Expand All @@ -137,7 +128,6 @@ internal void BeginMethod(DynamicMethod dynamicMethod, Type delegateType, string

InitILGeneration(methodName, argTypes);
}
#endif

internal void BeginMethod(string methodName, Type delegateType, bool allowPrivateMemberAccess)
{
Expand All @@ -152,17 +142,9 @@ internal void BeginMethod(string methodName, Type delegateType, bool allowPrivat

private void BeginMethod(Type returnType, string methodName, Type[] argTypes, bool allowPrivateMemberAccess)
{
#if USE_REFEMIT
string typeName = "Type" + (typeCounter++);
InitAssemblyBuilder(typeName + "." + methodName);
this.typeBuilder = moduleBuilder.DefineType(typeName, TypeAttributes.Public);
this.methodBuilder = typeBuilder.DefineMethod(methodName, MethodAttributes.Public|MethodAttributes.Static, returnType, argTypes);
this.ilGen = this.methodBuilder.GetILGenerator();
#else
_dynamicMethod = new DynamicMethod(methodName, returnType, argTypes, SerializationModule, allowPrivateMemberAccess);

_ilGen = _dynamicMethod.GetILGenerator();
#endif

InitILGeneration(methodName, argTypes);
}
Expand All @@ -186,15 +168,8 @@ internal Delegate EndMethod()
Ret();

Delegate? retVal = null;
#if USE_REFEMIT
Type type = typeBuilder.CreateType();
MethodInfo method = type.GetMethod(methodBuilder.Name);
retVal = Delegate.CreateDelegate(delegateType, method);
methodBuilder = null;
#else
retVal = _dynamicMethod.CreateDelegate(_delegateType);
_dynamicMethod = null!;
#endif
_delegateType = null!;

_ilGen = null!;
Expand All @@ -207,11 +182,7 @@ internal MethodInfo CurrentMethod
{
get
{
#if USE_REFEMIT
return methodBuilder;
#else
return _dynamicMethod;
#endif
}
}

Expand Down Expand Up @@ -1385,17 +1356,6 @@ private IfState PopIfState()
return ifState;
}

#if USE_REFEMIT
void InitAssemblyBuilder(string methodName)
{
AssemblyName name = new AssemblyName();
name.Name = "Microsoft.GeneratedCode."+methodName;
//Add SecurityCritical and SecurityTreatAsSafe attributes to the generated method
assemblyBuilder = AppDomain.CurrentDomain.DefineDynamicAssembly(name, AssemblyBuilderAccess.Run);
moduleBuilder = assemblyBuilder.DefineDynamicModule(name.Name + ".dll", false);
}
#endif

[DoesNotReturn]
private void ThrowMismatchException(object expected)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,10 +444,6 @@ internal static int GetId(RuntimeTypeHandle typeHandle)
}
catch (Exception ex)
{
if (DiagnosticUtility.IsFatal(ex))
{
throw;
}
throw DiagnosticUtility.ExceptionUtility.ThrowHelperFatal(ex.Message, ex);
}
}
Expand Down Expand Up @@ -931,10 +927,6 @@ internal static string GetNamespace(string key)
}
catch (Exception ex)
{
if (DiagnosticUtility.IsFatal(ex))
{
throw;
}
throw DiagnosticUtility.ExceptionUtility.ThrowHelperFatal(ex.Message, ex);
}
return key;
Expand All @@ -955,10 +947,6 @@ internal static XmlDictionaryString GetClrTypeString(string key)
}
catch (Exception ex)
{
if (DiagnosticUtility.IsFatal(ex))
{
throw;
}
throw DiagnosticUtility.ExceptionUtility.ThrowHelperFatal(ex.Message, ex);
}
}
Expand All @@ -972,10 +960,6 @@ internal static XmlDictionaryString GetClrTypeString(string key)
}
catch (Exception ex)
{
if (DiagnosticUtility.IsFatal(ex))
{
throw;
}
throw DiagnosticUtility.ExceptionUtility.ThrowHelperFatal(ex.Message, ex);
}
return value;
Expand All @@ -996,10 +980,6 @@ internal static void ThrowInvalidDataContractException(string? message, Type? ty
}
catch (Exception ex)
{
if (DiagnosticUtility.IsFatal(ex))
{
throw;
}
throw DiagnosticUtility.ExceptionUtility.ThrowHelperFatal(ex.Message, ex);
}
}
Expand Down Expand Up @@ -1290,12 +1270,8 @@ internal static bool IsValidNCName(string name)
{
return false;
}
catch (Exception ex)
catch (Exception)
{
if (DiagnosticUtility.IsFatal(ex))
{
throw;
}
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,21 +232,17 @@ internal DataContract MemberTypeContract
{
if (_memberTypeContract == null)
{
if (MemberInfo != null)
if (this.IsGetOnlyCollection)
{
if (this.IsGetOnlyCollection)
{
_memberTypeContract = DataContract.GetGetOnlyCollectionDataContract(DataContract.GetId(MemberType.TypeHandle), MemberType.TypeHandle, MemberType, SerializationMode.SharedContract);
}
else
{
_memberTypeContract = DataContract.GetDataContract(MemberType);
}
_memberTypeContract = DataContract.GetGetOnlyCollectionDataContract(DataContract.GetId(MemberType.TypeHandle), MemberType.TypeHandle, MemberType, SerializationMode.SharedContract);
}
else
{
_memberTypeContract = DataContract.GetDataContract(MemberType);
}
}

// TODO-NULLABLE - MemberInfo is never null, so this can never return null
return _memberTypeContract!;
return _memberTypeContract;
}
set
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,6 @@ public static void DebugAssert([DoesNotReturnIf(false)] bool condition, string m
Debug.Assert(condition, message);
}

internal static bool IsFatal(Exception? exception)
{
while (exception != null)
{
// These exceptions aren't themselves fatal, but since the CLR uses them to wrap other exceptions,
// we want to check to see whether they've been used to wrap a fatal exception. If so, then they
// count as fatal.
if (exception is TypeInitializationException)
{
exception = exception.InnerException;
}
else
{
break;
}
}

return false;
}

internal static class ExceptionUtility
{
public static Exception ThrowHelperArgumentNull(string message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ internal IList<ExtensionDataMember>? Members
}
}


internal class ExtensionDataMember
{
private IDataNode? _value;
Expand Down Expand Up @@ -114,13 +113,6 @@ public T GetValue()
return _value;
}

#if NotUsed
public void SetValue(T value)
{
this.value = value;
}
#endif

public string? DataContractName
{
get { return _dataContractName; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,6 @@ internal static int GetId(RuntimeTypeHandle typeHandle)
}
catch (Exception ex)
{
if (DiagnosticUtility.IsFatal(ex))
{
throw;
}
throw DiagnosticUtility.ExceptionUtility.ThrowHelperFatal(ex.Message, ex);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,6 @@ protected override void Dispose(bool disposing)
}
catch (Exception e)
{
if (DiagnosticUtility.IsFatal(e))
{
throw;
}

throw new InvalidOperationException(SR.GenericCallbackException, e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,6 @@ internal static object UnsafeGetUninitializedObject(Type type)
/// Safe - marked as such so that it's callable from transparent generated IL. Takes id as parameter which
/// is guaranteed to be in internal serialization cache.
/// </SecurityNote>

internal static object UnsafeGetUninitializedObject(int id)
{
var type = DataContract.GetDataContractForInitialization(id).TypeForInitialization;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ internal XmlFormatCollectionWriterDelegate GenerateCollectionWriter(CollectionDa
/// </SecurityNote>
private class CriticalHelper
{
#if !USE_REFEMIT
private CodeGenerator _ilg = null!; // initialized in GenerateXXXWriter
private ArgBuilder _xmlWriterArg = null!; // initialized in InitArgs
private ArgBuilder _contextArg = null!; // initialized in InitArgs
Expand All @@ -56,7 +55,6 @@ private class CriticalHelper
private LocalBuilder? _childElementNamespacesLocal;
private int _typeIndex = 1;
private int _childElementIndex;
#endif

private XmlFormatClassWriterDelegate CreateReflectionXmlFormatClassWriterDelegate()
{
Expand Down Expand Up @@ -130,7 +128,6 @@ internal XmlFormatCollectionWriterDelegate GenerateCollectionWriter(CollectionDa
}
}

#if !USE_REFEMIT
private void InitArgs(Type objType)
{
_xmlWriterArg = _ilg.GetArg(0);
Expand Down Expand Up @@ -763,7 +760,6 @@ private bool CheckIfConflictingMembersHaveDifferentTypes(DataMember member)
}
return false;
}
#endif
}
}
}
Loading

0 comments on commit a196d53

Please sign in to comment.