Code review comment for lp:~jameinel/bzr/2.1-static-tuple-btree-string-intern
- 2.1-static-tuple-btree-string-intern
- Merge into bzr.dev
Revision history for this message
Andrew Bennetts (spiv) wrote : | # |
1 | === modified file 'bzrlib/_static_tuple_c.c' |
2 | --- bzrlib/_static_tuple_c.c 2009-10-15 18:18:44 +0000 |
3 | +++ bzrlib/_static_tuple_c.c 2009-10-16 08:24:36 +0000 |
4 | @@ -513,6 +513,78 @@ |
5 | "Check to see if this tuple has been interned.\n"; |
6 | |
7 | |
8 | +int |
9 | +StaticTuple_coerce(PyObject **v, PyObject **w) |
10 | +{ |
11 | + StaticTuple *st; |
12 | + if (PyTuple_Check(*v)) { |
13 | + st = (StaticTuple*) StaticTuple_new_constructor( |
14 | + &StaticTuple_Type, *v, NULL); |
15 | + if (!st) |
16 | + return -1; |
17 | + Py_INCREF(st); |
18 | + *v = (PyObject*)st; |
19 | + } else if (StaticTuple_CheckExact(*v)) |
20 | + Py_INCREF(*v); |
21 | + else |
22 | + return 1; |
23 | + |
24 | + if (PyTuple_Check(*w)) { |
25 | + st = (StaticTuple*) StaticTuple_new_constructor( |
26 | + &StaticTuple_Type, *w, NULL); |
27 | + if (!st) |
28 | + return -1; |
29 | + Py_INCREF(st); |
30 | + *w = (PyObject*)st; |
31 | + } else if (StaticTuple_CheckExact(*w)) |
32 | + Py_INCREF(*w); |
33 | + else |
34 | + return 1; |
35 | + return 0; |
36 | +} |
37 | + |
38 | +static PyObject * |
39 | +StaticTuple_add(PyObject *v, PyObject *w) |
40 | +{ |
41 | + PyObject *v_t = NULL, *w_t = NULL; |
42 | + PyObject *tmp_tuple, *result; |
43 | + /* StaticTuples and plain tuples may be added (concatenated) to |
44 | + * StaticTuples. |
45 | + */ |
46 | + if (StaticTuple_CheckExact(v)) { |
47 | + v_t = StaticTuple_as_tuple((StaticTuple*)v); |
48 | + if (!v_t) |
49 | + goto fail; |
50 | + } else if (PyTuple_Check(v)) |
51 | + v_t = v; |
52 | + else |
53 | + goto not_imp; |
54 | + |
55 | + if (StaticTuple_CheckExact(w)) { |
56 | + w_t = StaticTuple_as_tuple((StaticTuple*)w); |
57 | + if (!w_t) |
58 | + goto fail; |
59 | + } else if (PyTuple_Check(w)) |
60 | + w_t = w; |
61 | + else |
62 | + goto not_imp; |
63 | + |
64 | + tmp_tuple = PySequence_Concat(v_t, w_t); |
65 | + result = StaticTuple_new_constructor(&StaticTuple_Type, tmp_tuple, NULL); |
66 | + Py_DECREF(tmp_tuple); |
67 | + Py_INCREF(result); |
68 | + return result; |
69 | + |
70 | +not_imp: |
71 | + Py_XDECREF(v_t); |
72 | + Py_XDECREF(w_t); |
73 | + return Py_NotImplemented; |
74 | +fail: |
75 | + Py_XDECREF(v_t); |
76 | + Py_XDECREF(w_t); |
77 | + return NULL; |
78 | +} |
79 | + |
80 | static PyObject * |
81 | StaticTuple_item(StaticTuple *self, Py_ssize_t offset) |
82 | { |
83 | @@ -574,6 +646,29 @@ |
84 | {NULL, NULL} /* sentinel */ |
85 | }; |
86 | |
87 | + |
88 | +static PyNumberMethods StaticTuple_as_number = { |
89 | + (binaryfunc) StaticTuple_add, /* nb_add */ |
90 | + 0, /* nb_subtract */ |
91 | + 0, /* nb_multiply */ |
92 | + 0, /* nb_divide */ |
93 | + 0, /* nb_remainder */ |
94 | + 0, /* nb_divmod */ |
95 | + 0, /* nb_power */ |
96 | + 0, /* nb_negative */ |
97 | + 0, /* nb_positive */ |
98 | + 0, /* nb_absolute */ |
99 | + 0, /* nb_nonzero */ |
100 | + 0, /* nb_invert */ |
101 | + 0, /* nb_lshift */ |
102 | + 0, /* nb_rshift */ |
103 | + 0, /* nb_and */ |
104 | + 0, /* nb_xor */ |
105 | + 0, /* nb_or */ |
106 | + StaticTuple_coerce, /* nb_coerce */ |
107 | +}; |
108 | + |
109 | + |
110 | static PySequenceMethods StaticTuple_as_sequence = { |
111 | (lenfunc)StaticTuple_length, /* sq_length */ |
112 | 0, /* sq_concat */ |
113 | @@ -604,7 +699,7 @@ |
114 | 0, /* tp_setattr */ |
115 | 0, /* tp_compare */ |
116 | (reprfunc)StaticTuple_repr, /* tp_repr */ |
117 | - 0, /* tp_as_number */ |
118 | + &StaticTuple_as_number, /* tp_as_number */ |
119 | &StaticTuple_as_sequence, /* tp_as_sequence */ |
120 | 0, /* tp_as_mapping */ |
121 | (hashfunc)StaticTuple_hash, /* tp_hash */ |
[...]
> > In plain Python you could define an __radd__ for this, so surely there's a
> > way to do this in C?
[...]
> > You may need to do something odd like provide the nb_add slot, even though
> > this isn't really a numeric type, but I think that's ok. (All pure python
> > classes would have that I think, even the non-numeric ones, so presumably
> > having tp_as_number filled doesn't automatically make Python do dumb
> > things.)
By the way, because lack of support for tuple + StaticTuple caused all the tests.per_ repository. test_write_ group.TestResum eableWriteGroup .test_commit_ resumed_ write_group_ with_missing_ parents
babune builders to go red, I took a look at this. (specifically,
bzrlib.
was failing)
You actually need nb_coerce as well as nb_add. Here's a rough patch that does
this. The error it gives when you try to add a plain tuple with incompatible
elements (e.g. ints) is probably not ideal, but it works.
-Andrew.