Skip to content
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

Miscellaneous fixes and cleanups #1760

Merged
merged 4 commits into from
May 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/runtime/AssemblyManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ internal class AssemblyManager
// than it can end up referring to assemblies that are already unloaded (default behavior after unload appDomain -
// unless LoaderOptimization.MultiDomain is used);
// So for multidomain support it is better to have the dict. recreated for each app-domain initialization
private static ConcurrentDictionary<string, ConcurrentDictionary<Assembly, string>> namespaces =
new ConcurrentDictionary<string, ConcurrentDictionary<Assembly, string>>();
private static readonly ConcurrentDictionary<string, ConcurrentDictionary<Assembly, string>> namespaces =
new();

#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable.
// domain-level handlers are initialized in Initialize
Expand All @@ -33,7 +33,7 @@ internal class AssemblyManager
#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable.

// updated only under GIL?
private static Dictionary<string, int> probed = new Dictionary<string, int>(32);
private static readonly Dictionary<string, int> probed = new(32);

// modified from event handlers below, potentially triggered from different .NET threads
private static readonly ConcurrentQueue<Assembly> assemblies = new();
Expand Down
3 changes: 1 addition & 2 deletions src/runtime/ClassManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,9 @@ internal static void InitClassBase(Type type, ClassBase impl, ReflectedClrType p
Runtime.PyDict_SetItem(dict, PyIdentifier.__doc__, doc.Borrow());
}

var co = impl as ClassObject;
// If this is a ClassObject AND it has constructors, generate a __doc__ attribute.
// required that the ClassObject.ctors be changed to internal
if (co != null)
if (impl is ClassObject co)
{
if (co.NumCtors > 0 && !co.HasCustomNew())
{
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/Codecs/DecoderGroup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Python.Runtime.Codecs
/// </summary>
public sealed class DecoderGroup: IPyObjectDecoder, IEnumerable<IPyObjectDecoder>, IDisposable
{
readonly List<IPyObjectDecoder> decoders = new List<IPyObjectDecoder>();
readonly List<IPyObjectDecoder> decoders = new();

/// <summary>
/// Add specified decoder to the group
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/Codecs/EncoderGroup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Python.Runtime.Codecs
/// </summary>
public sealed class EncoderGroup: IPyObjectEncoder, IEnumerable<IPyObjectEncoder>, IDisposable
{
readonly List<IPyObjectEncoder> encoders = new List<IPyObjectEncoder>();
readonly List<IPyObjectEncoder> encoders = new();

/// <summary>
/// Add specified encoder to the group
Expand Down
6 changes: 3 additions & 3 deletions src/runtime/Codecs/PyObjectConversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace Python.Runtime
/// </summary>
public static class PyObjectConversions
{
static readonly DecoderGroup decoders = new DecoderGroup();
static readonly EncoderGroup encoders = new EncoderGroup();
static readonly DecoderGroup decoders = new();
static readonly EncoderGroup encoders = new();

/// <summary>
/// Registers specified encoder (marshaller)
Expand Down Expand Up @@ -62,7 +62,7 @@ public static void RegisterDecoder(IPyObjectDecoder decoder)
}

static readonly ConcurrentDictionary<Type, IPyObjectEncoder[]>
clrToPython = new ConcurrentDictionary<Type, IPyObjectEncoder[]>();
clrToPython = new();
static IPyObjectEncoder[] GetEncoders(Type type)
{
lock (encoders)
Expand Down
23 changes: 11 additions & 12 deletions src/runtime/Converter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ private Converter()
{
}

private static Type objectType;
private static Type stringType;
private static Type singleType;
private static Type doubleType;
private static Type int16Type;
private static Type int32Type;
private static Type int64Type;
private static Type boolType;
private static Type typeType;
private static readonly Type objectType;
private static readonly Type stringType;
private static readonly Type singleType;
private static readonly Type doubleType;
private static readonly Type int16Type;
private static readonly Type int32Type;
private static readonly Type int64Type;
private static readonly Type boolType;
private static readonly Type typeType;

static Converter()
{
Expand Down Expand Up @@ -151,8 +151,7 @@ internal static NewReference ToPython(object? value, Type type)

// it the type is a python subclass of a managed type then return the
// underlying python object rather than construct a new wrapper object.
var pyderived = value as IPythonDerivedType;
if (null != pyderived)
if (value is IPythonDerivedType pyderived)
{
if (!IsTransparentProxy(pyderived))
return ClassDerivedObject.ToPython(pyderived);
Expand All @@ -161,7 +160,7 @@ internal static NewReference ToPython(object? value, Type type)
// ModuleObjects are created in a way that their wrapping them as
// a CLRObject fails, the ClassObject has no tpHandle. Return the
// pyHandle as is, do not convert.
if (value is ModuleObject modobj)
if (value is ModuleObject)
{
throw new NotImplementedException();
}
Expand Down
6 changes: 3 additions & 3 deletions src/runtime/DelegateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ namespace Python.Runtime
/// </summary>
internal class DelegateManager
{
private readonly Dictionary<Type,Type> cache = new Dictionary<Type, Type>();
private readonly Dictionary<Type,Type> cache = new();
private readonly Type basetype = typeof(Dispatcher);
private readonly Type arrayType = typeof(object[]);
private readonly Type voidtype = typeof(void);
private readonly Type typetype = typeof(Type);
private readonly Type pyobjType = typeof(PyObject);
private readonly CodeGenerator codeGenerator = new CodeGenerator();
private readonly CodeGenerator codeGenerator = new();
private readonly ConstructorInfo arrayCtor;
private readonly MethodInfo dispatch;

Expand Down Expand Up @@ -309,7 +309,7 @@ protected Dispatcher(PyObject target, Type dtype)
{
tpName += $" of size {Runtime.PyTuple_Size(op)}";
}
StringBuilder sb = new StringBuilder();
var sb = new StringBuilder();
if (!isVoid) sb.Append(rtype.FullName);
for (int i = 0; i < pi.Length; i++)
{
Expand Down
3 changes: 1 addition & 2 deletions src/runtime/Exceptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,7 @@ public static bool SetError(Exception e)
// might get a managed exception raised that is a wrapper for a
// Python exception. In that case we'd rather have the real thing.

var pe = e as PythonException;
if (pe != null)
if (e is PythonException pe)
{
pe.Restore();
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/Finalizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public ErrorArgs(Exception error)
[DefaultValue(true)]
public bool Enable { get; set; } = true;

private ConcurrentQueue<PendingFinalization> _objQueue = new();
private readonly ConcurrentQueue<PendingFinalization> _objQueue = new();
private readonly ConcurrentQueue<PendingFinalization> _derivedQueue = new();
private readonly ConcurrentQueue<Py_buffer> _bufferQueue = new();
private int _throttled;
Expand Down
3 changes: 1 addition & 2 deletions src/runtime/InternString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ public static void Shutdown()

public static string? GetManagedString(BorrowedReference op)
{
string s;
if (TryGetInterned(op, out s))
if (TryGetInterned(op, out string s))
{
return s;
}
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/Interop.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ internal static Type GetPrototype(MethodInfo method)
}


internal static Dictionary<IntPtr, Delegate> allocatedThunks = new Dictionary<IntPtr, Delegate>();
internal static Dictionary<IntPtr, Delegate> allocatedThunks = new();

internal static ThunkInfo GetThunk(MethodInfo method)
{
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/InteropConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Python.Runtime
public sealed class InteropConfiguration: IDisposable
{
internal readonly PythonBaseTypeProviderGroup pythonBaseTypeProviders
= new PythonBaseTypeProviderGroup();
= new();

/// <summary>Enables replacing base types of CLR types as seen from Python</summary>
public IList<IPythonBaseTypeProvider> PythonBaseTypeProviders => this.pythonBaseTypeProviders;
Expand Down
25 changes: 4 additions & 21 deletions src/runtime/Loader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,8 @@ public unsafe static int Initialize(IntPtr data, int size)
PythonDLL = null;
}

var gs = PyGILState_Ensure();

try
{
// Console.WriteLine("Startup thread");
PythonEngine.InitExt();
// Console.WriteLine("Startup finished");
}
finally
{
PyGILState_Release(gs);
}
using var _ = Py.GIL();
PythonEngine.InitExt();
}
catch (Exception exc)
{
Expand All @@ -55,15 +45,8 @@ public unsafe static int Shutdown(IntPtr data, int size)

if (command == "full_shutdown")
{
var gs = PyGILState_Ensure();
try
{
PythonEngine.Shutdown();
}
finally
{
PyGILState_Release(gs);
}
using var _ = Py.GIL();
PythonEngine.Shutdown();
}
}
catch (Exception exc)
Expand Down
88 changes: 28 additions & 60 deletions src/runtime/MethodBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -242,52 +242,24 @@ internal static int ArgPrecedence(Type t)

TypeCode tc = Type.GetTypeCode(t);
// TODO: Clean up
switch (tc)
return tc switch
{
case TypeCode.Object:
return 1;

case TypeCode.UInt64:
return 10;

case TypeCode.UInt32:
return 11;

case TypeCode.UInt16:
return 12;

case TypeCode.Int64:
return 13;

case TypeCode.Int32:
return 14;

case TypeCode.Int16:
return 15;

case TypeCode.Char:
return 16;

case TypeCode.SByte:
return 17;

case TypeCode.Byte:
return 18;

case TypeCode.Single:
return 20;

case TypeCode.Double:
return 21;

case TypeCode.String:
return 30;

case TypeCode.Boolean:
return 40;
}

return 2000;
TypeCode.Object => 1,
TypeCode.UInt64 => 10,
TypeCode.UInt32 => 11,
TypeCode.UInt16 => 12,
TypeCode.Int64 => 13,
TypeCode.Int32 => 14,
TypeCode.Int16 => 15,
TypeCode.Char => 16,
TypeCode.SByte => 17,
TypeCode.Byte => 18,
TypeCode.Single => 20,
TypeCode.Double => 21,
TypeCode.String => 30,
TypeCode.Boolean => 40,
_ => 2000,
};
}

/// <summary>
Expand Down Expand Up @@ -410,18 +382,14 @@ public MismatchedMethod(Exception exception, MethodBase mb)
isGeneric = true;
}
ParameterInfo[] pi = mi.GetParameters();
ArrayList? defaultArgList;
bool paramsArray;
int kwargsMatched;
int defaultsNeeded;
bool isOperator = OperatorMethod.IsOperatorMethod(mi);
// Binary operator methods will have 2 CLR args but only one Python arg
// (unary operators will have 1 less each), since Python operator methods are bound.
isOperator = isOperator && pynargs == pi.Length - 1;
bool isReverse = isOperator && OperatorMethod.IsReverse((MethodInfo)mi); // Only cast if isOperator.
if (isReverse && OperatorMethod.IsComparisonOp((MethodInfo)mi))
continue; // Comparison operators in Python have no reverse mode.
if (!MatchesArgumentCount(pynargs, pi, kwargDict, out paramsArray, out defaultArgList, out kwargsMatched, out defaultsNeeded) && !isOperator)
if (!MatchesArgumentCount(pynargs, pi, kwargDict, out bool paramsArray, out ArrayList? defaultArgList, out int kwargsMatched, out int defaultsNeeded) && !isOperator)
{
continue;
}
Expand All @@ -436,8 +404,7 @@ public MismatchedMethod(Exception exception, MethodBase mb)
// We need to take the first CLR argument.
pi = pi.Take(1).ToArray();
}
int outs;
var margs = TryConvertArguments(pi, paramsArray, args, pynargs, kwargDict, defaultArgList, outs: out outs);
var margs = TryConvertArguments(pi, paramsArray, args, pynargs, kwargDict, defaultArgList, outs: out int outs);
if (margs == null)
{
var mismatchCause = PythonException.FetchCurrent();
Expand Down Expand Up @@ -495,7 +462,7 @@ public MismatchedMethod(Exception exception, MethodBase mb)
{
// Best effort for determining method to match on gives multiple possible
// matches and we need at least one default argument - bail from this point
StringBuilder stringBuilder = new StringBuilder("Not enough arguments provided to disambiguate the method. Found:");
var stringBuilder = new StringBuilder("Not enough arguments provided to disambiguate the method. Found:");
foreach (var matchedMethod in argMatchedMethods)
{
stringBuilder.AppendLine();
Expand Down Expand Up @@ -523,18 +490,20 @@ public MismatchedMethod(Exception exception, MethodBase mb)
//CLRObject co = (CLRObject)ManagedType.GetManagedObject(inst);
// InvalidCastException: Unable to cast object of type
// 'Python.Runtime.ClassObject' to type 'Python.Runtime.CLRObject'
var co = ManagedType.GetManagedObject(inst) as CLRObject;

// Sanity check: this ensures a graceful exit if someone does
// something intentionally wrong like call a non-static method
// on the class rather than on an instance of the class.
// XXX maybe better to do this before all the other rigmarole.
if (co == null)
if (ManagedType.GetManagedObject(inst) is CLRObject co)
{
target = co.inst;
}
else
{
Exceptions.SetError(Exceptions.TypeError, "Invoked a non-static method with an invalid instance");
return null;
}
target = co.inst;
}

return new Binding(mi, target, margs, outs);
Expand Down Expand Up @@ -623,7 +592,7 @@ static BorrowedReference HandleParamsArray(BorrowedReference args, int arrayStar
for (int paramIndex = 0; paramIndex < pi.Length; paramIndex++)
{
var parameter = pi[paramIndex];
bool hasNamedParam = parameter.Name != null ? kwargDict.ContainsKey(parameter.Name) : false;
bool hasNamedParam = parameter.Name != null && kwargDict.ContainsKey(parameter.Name);

if (paramIndex >= pyArgCount && !(hasNamedParam || (paramsArray && paramIndex == arrayStart)))
{
Expand Down Expand Up @@ -658,8 +627,7 @@ static BorrowedReference HandleParamsArray(BorrowedReference args, int arrayStar
}
}

bool isOut;
if (!TryConvertArgument(op, parameter.ParameterType, out margs[paramIndex], out isOut))
if (!TryConvertArgument(op, parameter.ParameterType, out margs[paramIndex], out bool isOut))
{
tempObject.Dispose();
return null;
Expand Down Expand Up @@ -789,7 +757,7 @@ static BorrowedReference HandleParamsArray(BorrowedReference args, int arrayStar
{
defaultArgList = null;
var match = false;
paramsArray = parameters.Length > 0 ? Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute)) : false;
paramsArray = parameters.Length > 0 && Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute));
kwargsMatched = 0;
defaultsNeeded = 0;
if (positionalArgumentCount == parameters.Length && kwargDict.Count == 0)
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/Native/BorrowedReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public IntPtr DangerousGetAddress()
/// <summary>Gets a raw pointer to the Python object</summary>
public IntPtr DangerousGetAddressOrNull() => this.pointer;

public static BorrowedReference Null => new BorrowedReference();
public static BorrowedReference Null => new();

/// <summary>
/// Creates new instance of <see cref="BorrowedReference"/> from raw pointer. Unsafe.
Expand Down