Skip to content

Commit 79516f1

Browse files
BadSingletonBadSingleton
committed
Code review fixes
1 parent 44b4800 commit 79516f1

File tree

10 files changed

+54
-44
lines changed

10 files changed

+54
-44
lines changed

‎src/domain_tests/TestRunner.cs‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,22 @@ namespace Python.DomainReloadTests
1616
/// 3. This class at runtime creates a directory that has both C# and
1717
/// python code, and compiles the C#.
1818
/// 4. This class then runs the C# code.
19-
///
20-
/// But wait there's more indirection. The C# code that's run -- known as
21-
/// the test runner --
22-
/// This class compiles a DLL that contains the class which code will change
23-
/// and a runner executable that will run Python code referencing the class.
24-
/// Each test case:
19+
///
20+
/// But there's a bit more indirection. This class compiles a DLL that
21+
/// contains code that will change.
22+
/// Then, the test case:
2523
/// * Compiles some code, loads it into a domain, runs python that refers to it.
26-
/// * Unload the domain.
27-
/// * Compile a new piece of code, load it into a domain, run a new piece of python that accesses the code.
24+
/// * Unload the domain, re-runs the domain to make sure domain reload happens correctly.
25+
/// * Compile a new piece of code, load it into a new domain, run a new piece of
26+
/// Python code to test the objects after they've been deleted or modified in C#.
2827
/// * Unload the domain. Reload the domain, run the same python again.
28+
///
2929
/// This class gets built into an executable which takes one argument:
3030
/// which test case to run. That's because pytest assumes we'll run
3131
/// everything in one process, but we really want a clean process on each
32-
/// test case to test the init/reload/teardown parts of the domain reload
33-
/// code.
32+
/// test case to test the init/reload/teardown parts of the domain reload.
3433
/// </summary>
34+
///
3535
class TestRunner
3636
{
3737
const string TestAssemblyName = "DomainTests";
@@ -347,12 +347,6 @@ assert called is True
347347
called = False
348348
Cls.Call()
349349
assert called is False
350-
#try:
351-
# assert 2 == Cls.Before
352-
#except TypeError:
353-
# print('Caught expected exception')
354-
#else:
355-
# raise AssertionError('Failed to throw exception')
356350
",
357351
},
358352

@@ -398,7 +392,6 @@ def before_reload():
398392
def after_reload():
399393
try:
400394
TestNamespace.Cls(2)
401-
sys.my_cls.Member()
402395
except AttributeError:
403396
print('Caught expected exception')
404397
else:
@@ -845,7 +838,7 @@ raise AssertionError('failed to raise')
845838
# foo should have changed
846839
assert foo.num == 7
847840
assert bar.num == 7
848-
# Pythonnet also returns a new object with `ref`-quialified parameters
841+
# Pythonnet also returns a new object with `ref`-qualified parameters
849842
assert foo is not bar
850843
",
851844
},
@@ -1029,7 +1022,6 @@ static string CreateCaseRunnerAssembly(string verb, string shutdownMode = "Shutd
10291022
}
10301023
static string CreateAssembly(string name, string code, bool exe = false)
10311024
{
1032-
// Console.WriteLine(code);
10331025
// Never return or hold the Assembly instance. This will cause
10341026
// the assembly to be loaded into the current domain and this
10351027
// interferes with the tests. The Domain can execute fine from a

‎src/runtime/StateSerialization/MaybeMethodBase.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ internal MaybeMethodBase(SerializationInfo serializationInfo, StreamingContext c
105105
deserializationException = null;
106106
try
107107
{
108-
// Retrive the reflected type of the method;
108+
// Retrieve the reflected type of the method;
109109
var typeName = serializationInfo.GetString(SerializationType);
110110
var tp = Type.GetType(typeName);
111111
if (tp == null)
@@ -144,7 +144,7 @@ internal MaybeMethodBase(SerializationInfo serializationInfo, StreamingContext c
144144

145145
if (mb != null && hasRefType)
146146
{
147-
CheckRefTypes(mb, param);
147+
mb = CheckRefTypes(mb, param);
148148
}
149149

150150
// Do like in ClassManager.GetClassInfo
@@ -165,7 +165,7 @@ MethodBase CheckRefTypes(MethodBase mb, ParameterHelper[] ph)
165165
// void MyFn (ref int a)
166166
// to:
167167
// void MyFn (out int a)
168-
// will still find the fucntion correctly as, `in`, `out` and `ref`
168+
// will still find the function correctly as, `in`, `out` and `ref`
169169
// are all represented as a reference type. Query the method we got
170170
// and validate the parameters
171171
if (ph.Length != 0)

‎src/runtime/arrayobject.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public static IntPtr tp_new(IntPtr tpRaw, IntPtr args, IntPtr kw)
3434
{
3535
return Exceptions.RaiseTypeError(self.type.DeletedMessage);
3636
}
37+
Type arrType = self.type.Value;
3738

3839
long[] dimensions = new long[Runtime.PyTuple_Size(args)];
3940
if (dimensions.Length == 0)
@@ -42,7 +43,7 @@ public static IntPtr tp_new(IntPtr tpRaw, IntPtr args, IntPtr kw)
4243
}
4344
if (dimensions.Length != 1)
4445
{
45-
return CreateMultidimensional(self.type.Value.GetElementType(), dimensions,
46+
return CreateMultidimensional(arrType.GetElementType(), dimensions,
4647
shapeTuple: new BorrowedReference(args),
4748
pyType: tp)
4849
.DangerousMoveToPointerOrNull();
@@ -60,14 +61,14 @@ public static IntPtr tp_new(IntPtr tpRaw, IntPtr args, IntPtr kw)
6061
}
6162
else
6263
{
63-
return NewInstance(self.type.Value.GetElementType(), tp, dimensions)
64+
return NewInstance(arrType.GetElementType(), tp, dimensions)
6465
.DangerousMoveToPointerOrNull();
6566
}
6667
}
6768
object result;
6869

6970
// this implements casting to Array[T]
70-
if (!Converter.ToManaged(op, self.type.Value, out result, true))
71+
if (!Converter.ToManaged(op, arrType, out result, true))
7172
{
7273
return IntPtr.Zero;
7374
}

‎src/runtime/constructorbinder.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,10 @@ internal object InvokeRaw(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info)
5656
return Exceptions.RaiseTypeError(_containingType.DeletedMessage);
5757
}
5858
object result;
59+
Type tp = _containingType.Value;
5960

60-
if (_containingType.Value.IsValueType && !_containingType.Value.IsPrimitive &&
61-
!_containingType.Value.IsEnum && _containingType.Value != typeof(decimal) &&
61+
if (tp.IsValueType && !tp.IsPrimitive &&
62+
!tp.IsEnum && tp != typeof(decimal) &&
6263
Runtime.PyTuple_Size(args) == 0)
6364
{
6465
// If you are trying to construct an instance of a struct by
@@ -68,7 +69,7 @@ internal object InvokeRaw(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info)
6869
// Activator.CreateInstance().
6970
try
7071
{
71-
result = Activator.CreateInstance(_containingType.Value);
72+
result = Activator.CreateInstance(tp);
7273
}
7374
catch (Exception e)
7475
{

‎src/runtime/constructorbinding.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public static IntPtr mp_subscript(IntPtr op, IntPtr key)
9696
{
9797
return Exceptions.RaiseTypeError(self.type.DeletedMessage);
9898
}
99+
Type tp = self.type.Value;
99100

100101
Type[] types = Runtime.PythonArgsToTypeArray(key);
101102
if (types == null)
@@ -104,12 +105,12 @@ public static IntPtr mp_subscript(IntPtr op, IntPtr key)
104105
}
105106
//MethodBase[] methBaseArray = self.ctorBinder.GetMethods();
106107
//MethodBase ci = MatchSignature(methBaseArray, types);
107-
ConstructorInfo ci = self.type.Value.GetConstructor(types);
108+
ConstructorInfo ci = tp.GetConstructor(types);
108109
if (ci == null)
109110
{
110111
return Exceptions.RaiseTypeError("No match found for constructor signature");
111112
}
112-
var boundCtor = new BoundContructor(self.type.Value, self.pyTypeHndl, self.ctorBinder, ci);
113+
var boundCtor = new BoundContructor(tp, self.pyTypeHndl, self.ctorBinder, ci);
113114

114115
return boundCtor.pyHandle;
115116
}
@@ -126,6 +127,11 @@ public static IntPtr tp_repr(IntPtr ob)
126127
return self.repr;
127128
}
128129
MethodBase[] methods = self.ctorBinder.GetMethods();
130+
131+
if (!self.type.Valid)
132+
{
133+
return Exceptions.RaiseTypeError(self.type.DeletedMessage);
134+
}
129135
string name = self.type.Value.FullName;
130136
var doc = "";
131137
foreach (MethodBase t in methods)

‎src/runtime/converter.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,13 @@ internal static bool ToManagedValue(IntPtr value, Type obType,
344344
}
345345
if (mt is ClassBase)
346346
{
347-
result = ((ClassBase)mt).type.Value;
347+
var cb = (ClassBase)mt;
348+
if (!cb.type.Valid)
349+
{
350+
Exceptions.SetError(Exceptions.TypeError, cb.type.DeletedMessage);
351+
return false;
352+
}
353+
result = cb.type.Value;
348354
return true;
349355
}
350356
// shouldn't happen

‎src/runtime/delegateobject.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public static IntPtr tp_new(IntPtr tp, IntPtr args, IntPtr kw)
5656
{
5757
return Exceptions.RaiseTypeError(self.type.DeletedMessage);
5858
}
59+
Type type = self.type.Value;
5960

6061
if (Runtime.PyTuple_Size(args) != 1)
6162
{
@@ -69,7 +70,7 @@ public static IntPtr tp_new(IntPtr tp, IntPtr args, IntPtr kw)
6970
return Exceptions.RaiseTypeError("argument must be callable");
7071
}
7172

72-
Delegate d = PythonEngine.DelegateManager.GetDelegate(self.type.Value, method);
73+
Delegate d = PythonEngine.DelegateManager.GetDelegate(type, method);
7374
return CLRObject.GetInstHandle(d, self.pyHandle);
7475
}
7576

‎src/runtime/methodbinding.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,14 @@ public static IntPtr tp_call(IntPtr ob, IntPtr args, IntPtr kw)
114114
// for example this method in the tests: string Overloaded<T>(int arg1, int arg2, string arg3)
115115
if (self.info.Valid)
116116
{
117-
if (self.info.Value.IsGenericMethod)
117+
var info = self.info.Value;
118+
if (info.IsGenericMethod)
118119
{
119120
var len = Runtime.PyTuple_Size(args); //FIXME: Never used
120121
Type[] sigTp = Runtime.PythonArgsToTypeArray(args, true);
121122
if (sigTp != null)
122123
{
123-
Type[] genericTp = self.info.Value.GetGenericArguments();
124+
Type[] genericTp = info.GetGenericArguments();
124125
MethodInfo betterMatch = MethodBinder.MatchSignatureAndParameters(self.m.info, genericTp, sigTp);
125126
if (betterMatch != null)
126127
{

‎src/runtime/moduleobject.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ protected override void OnSave(InterDomainContext context)
346346
{
347347
Runtime.PyDict_DelItemString(dict, pair.Key);
348348
pair.Value.DecrRefCount();
349-
if (Exceptions.ExceptionMatches(Exceptions.KeyError))
349+
if (Exceptions.ExceptionMatches(Exceptions.KeyError))
350350
{
351351
// Trying to remove a key that's not in the dictionary
352352
// raises an error. We don't care about it.

‎src/runtime/propertyobject.cs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ public static IntPtr tp_descr_get(IntPtr ds, IntPtr ob, IntPtr tp)
3636
{
3737
return Exceptions.RaiseTypeError(self.info.DeletedMessage);
3838
}
39-
MethodInfo getter = self.getter.Value;
39+
var info = self.info.Value;
40+
MethodInfo getter = self.getter.UnsafeValue;
4041
object result;
4142

4243

@@ -56,8 +57,8 @@ public static IntPtr tp_descr_get(IntPtr ds, IntPtr ob, IntPtr tp)
5657

5758
try
5859
{
59-
result = self.info.Value.GetValue(null, null);
60-
return Converter.ToPython(result, self.info.Value.PropertyType);
60+
result = info.GetValue(null, null);
61+
return Converter.ToPython(result, info.PropertyType);
6162
}
6263
catch (Exception e)
6364
{
@@ -73,8 +74,8 @@ public static IntPtr tp_descr_get(IntPtr ds, IntPtr ob, IntPtr tp)
7374

7475
try
7576
{
76-
result = self.info.Value.GetValue(co.inst, null);
77-
return Converter.ToPython(result, self.info.Value.PropertyType);
77+
result = info.GetValue(co.inst, null);
78+
return Converter.ToPython(result, info.PropertyType);
7879
}
7980
catch (Exception e)
8081
{
@@ -101,8 +102,9 @@ public static IntPtr tp_descr_get(IntPtr ds, IntPtr ob, IntPtr tp)
101102
Exceptions.RaiseTypeError(self.info.DeletedMessage);
102103
return -1;
103104
}
105+
var info = self.info.Value;
104106

105-
MethodInfo setter = self.setter.Value;
107+
MethodInfo setter = self.setter.UnsafeValue;
106108
object newval;
107109

108110
if (val == IntPtr.Zero)
@@ -118,7 +120,7 @@ public static IntPtr tp_descr_get(IntPtr ds, IntPtr ob, IntPtr tp)
118120
}
119121

120122

121-
if (!Converter.ToManaged(val, self.info.Value.PropertyType, out newval, true))
123+
if (!Converter.ToManaged(val, info.PropertyType, out newval, true))
122124
{
123125
return -1;
124126
}
@@ -144,11 +146,11 @@ public static IntPtr tp_descr_get(IntPtr ds, IntPtr ob, IntPtr tp)
144146
Exceptions.RaiseTypeError("invalid target");
145147
return -1;
146148
}
147-
self.info.Value.SetValue(co.inst, newval, null);
149+
info.SetValue(co.inst, newval, null);
148150
}
149151
else
150152
{
151-
self.info.Value.SetValue(null, newval, null);
153+
info.SetValue(null, newval, null);
152154
}
153155
return 0;
154156
}

0 commit comments

Comments
 (0)