[PATCH V2] obsolete: use C code for headrevs calculation

[Durham Goode]

On 9/17/14, 1:37 AM, Antoine Pitrou wrote:
> On Tue, 16 Sep 2014 21:15:09 -0700
> Durham Goode <durham at fb.com> wrote:
>> +static int check_filter(PyObject *filter, Py_ssize_t arg) {
>> +	if (filter) {
>> +		PyObject *arglist = Py_BuildValue("(i)", arg);
> With a Py_ssize_t you should use "n", not "i".
>
>> +		PyObject *isfiltered = PyEval_CallObject(filter, arglist);
>> +		Py_DECREF(arglist);
> arglist or isfiltered could be NULL here.
>
>> +static PyObject *index_headrevs(indexObject *self, PyObject *args)
>>   {
>>   	Py_ssize_t i, len, addlen;
>>   	char *nothead = NULL;
>> -	PyObject *heads;
>> +	PyObject *heads, *filteredrevs;
>>   
>> -	if (self->headrevs)
>> +	if (PyTuple_Size(args) == 0) {
>> +		filteredrevs = Py_None;
> Is that some kind of micro-optimization? You are already asking for an
> optional argument below:
I tried relying on the  "|O" below, but filteredrevs was still being 
filled with data (something of type 'super'). I couldn't figure out what 
was going on, so I worked around it.
>
>> +	} else if (!PyArg_ParseTuple(args, "|O", &filteredrevs)) {
>> +		return NULL;
>> +	}
>> +	PyObject *filter = NULL;
> Is the C code supposed to be compilable with MSVC? MSVC will choke on
> declarations in the middle of blocks (unless you are in C++ mode).
>
>> +	if (filteredrevs != Py_None) {
>> +		filter = PyObject_GetAttrString(filteredrevs, "__contains__");
>> +	}
> So filter could be NULL here.
>
>> +
>>   	len = index_length(self) - 1;
>>   	heads = PyList_New(0);
>>   	if (heads == NULL)
>> @@ -850,6 +880,11 @@
>>   		goto bail;
>>   
>>   	for (i = 0; i < self->raw_length; i++) {
>> +		if (check_filter(filter, i)) {
>> +			nothead[i] = 1;
>> +			continue;
>> +		}
> Now the declarations below will fail under MSVC :-)
>
>>   		const char *data = index_deref(self, i);
>>   		int parent_1 = getbe32(data + 24);
>>   		int parent_2 = getbe32(data + 28);
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel