Skip to content

Commit 69583de

Browse files
committed
Address peer review
1 parent 2ab6692 commit 69583de

File tree

1 file changed

+36
-16
lines changed

1 file changed

+36
-16
lines changed

src/runtime/Types/ClassBase.cs

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ public static NewReference tp_richcompare(BorrowedReference ob, BorrowedReferenc
9595
{
9696
CLRObject co1;
9797
CLRObject? co2;
98+
object co1Inst;
99+
object co2Inst;
100+
NewReference error;
98101
BorrowedReference tp = Runtime.PyObject_TYPE(ob);
99102
var cls = (ClassBase)GetManagedObject(tp)!;
100103
// C# operator methods take precedence over IComparable.
@@ -127,16 +130,14 @@ public static NewReference tp_richcompare(BorrowedReference ob, BorrowedReferenc
127130
return new NewReference(pytrue);
128131
}
129132

130-
co1 = (CLRObject)GetManagedObject(ob)!;
131-
var o2 = GetSecondCompareOperandInstance(other);
132-
if (null == o2)
133+
GetSecondCompareOperandInstance(ob, other, out co1, out co2, out co1Inst, out co2Inst, out error);
134+
135+
if (co2Inst == null)
133136
{
134137
return new NewReference(pyfalse);
135138
}
136139

137-
object o1 = co1.inst;
138-
139-
if (Equals(o1, o2))
140+
if (Equals(co1Inst, co2Inst))
140141
{
141142
return new NewReference(pytrue);
142143
}
@@ -146,14 +147,14 @@ public static NewReference tp_richcompare(BorrowedReference ob, BorrowedReferenc
146147
case Runtime.Py_LE:
147148
case Runtime.Py_GT:
148149
case Runtime.Py_GE:
149-
co1 = (CLRObject)GetManagedObject(ob)!;
150-
var co2Inst = GetSecondCompareOperandInstance(other);
150+
GetSecondCompareOperandInstance(ob, other, out co1, out co2, out co1Inst, out co2Inst, out error);
151151

152-
if (co1 == null || co2Inst == null)
152+
if (!error.IsNone() && !error.IsNull())
153153
{
154154
return Exceptions.RaiseTypeError("Cannot get managed object");
155155
}
156-
var co1Comp = co1.inst as IComparable;
156+
157+
var co1Comp = co1Inst as IComparable;
157158
if (co1Comp == null)
158159
{
159160
Type co1Type = co1.GetType();
@@ -208,30 +209,49 @@ public static NewReference tp_richcompare(BorrowedReference ob, BorrowedReferenc
208209
}
209210
}
210211

211-
private static object GetSecondCompareOperandInstance(BorrowedReference other)
212+
private static void GetSecondCompareOperandInstance(BorrowedReference left, BorrowedReference right,
213+
out CLRObject co1, out CLRObject co2, out object co1Inst, out object co2Inst, out NewReference error)
212214
{
213-
var co2 = GetManagedObject(other) as CLRObject;
215+
co1Inst = null;
216+
co2Inst = null;
217+
error = new NewReference(Runtime.PyNone);
214218

215-
object co2Inst = null;
219+
co1 = (CLRObject)GetManagedObject(left)!;
220+
co2 = GetManagedObject(right) as CLRObject;
221+
222+
var co2IsValid = true;
216223
// The object comparing against is not a managed object. It could still be a Python object
217224
// that can be compared against (e.g. comparing against a Python string)
218225
if (co2 == null)
219226
{
220-
if (other != null)
227+
if (right != null)
221228
{
222-
using var pyCo2 = new PyObject(other);
229+
using var pyCo2 = new PyObject(right);
223230
if (Converter.ToManagedValue(pyCo2, typeof(object), out var result, false))
224231
{
225232
co2Inst = result;
226233
}
234+
else
235+
{
236+
co2IsValid = false;
237+
}
238+
}
239+
else
240+
{
241+
co2IsValid = false;
227242
}
228243
}
229244
else
230245
{
231246
co2Inst = co2.inst;
232247
}
233248

234-
return co2Inst;
249+
if (co1 == null || !co2IsValid)
250+
{
251+
error = Exceptions.RaiseTypeError("Cannot get managed object");
252+
}
253+
254+
co1Inst = co1.inst;
235255
}
236256

237257
/// <summary>

0 commit comments

Comments
 (0)