cython/cython — pull request #354 by mattjj (2014-12-29 18:39:23+00:00)
Title: Fix vectors of memviews

## The problem ##

These two example functions each result in C++ compiler errors using the current Cython master (db4001f):

```Cython
# distutils: language = c++
from libcpp.vector cimport vector

def test_memviews_same(a,b):
cdef vector[double[:]] aa = a
cdef vector[double[:]] bb = b

def test_memviews_diff(a,b):
cdef vector[double[:]] aa = a
cdef vector[int[:]] bb = b
```

Compiling that file using db4001f would result in an error like

```
blah.cpp: In function 'std::vector<__Pyx_memviewslice> __pyx_convert_vector_from_py___Pyx_memviewslice(PyObject*)':
blah.cpp:12109:40: error: redefinition of 'std::vector<__Pyx_memviewslice> __pyx_convert_vector_from_py___Pyx_memviewslice(PyObject*)'
static std::vector<__Pyx_memviewslice> __pyx_convert_vector_from_py___Pyx_memviewslice(PyObject *__pyx_v_o) {
```

However, similar code using native types instead of typed memoryviews would work fine:

```cython
def test_native_same(a,b):
cdef vector[double] aa = a
cdef vector[double] bb = b
```

See the rest of the code and really basic tests in [the full example gist](https://gist.github.com/mattjj/e9eef9aad52008d333b1) or [similar examples](https://gist.github.com/mattjj/15f28177d68238659386) from [a mailing list post](https://groups.google.com/forum/#!msg/cython-users/YnxIFUFa5zA/izb8yKgHa1kJ).

## This PR's potential fix ##

The code in this PR allows those examples to compile by adding three methods to the `MemoryViewSliceType` class in PyrexTypes.py, namely `specialization_name()`, `__hash__()`, and `__eq__()`.

I believe there were two related problems here, the first resulting in failures in the `double[:]`/`double[:]` example above and the second in the failure in the `double[:]`/`int[:]` example.

### Fix for the first example ###

In the first example, the conversion utility code was being generated twice because the `CppClassType.specialize()` method uses a hash table to store previously-generated collisions, but since `MemoryViewSliceType` defaulted to a generic object id-based hash, that hash table would not recognize and return identical specializations and instead generate a new speicalization, including new utility code. This same problem did not occurr using native types as the vector template argument, like `double` instead of `double[:]` as in the last example because only one instance of the `CNumericType` would be created.

One way to avoid the issue, which this PR attempts, is to add `__hash__()` and `__eq__()` methods to `MemoryViewSliceType`. I based their design on the existing `MemoryViewSliceType.same_as_resolved_type()` method, which seems to encode the notion of equality we care about here. Other `BaseType` childre in PyrexTypes.py also define `__hash__()` methods, and these choices seem to comport with the general sense I gleaned from those methods.

This fix avoids the compilation error pasted above and allows the first example to work, though the second example runs into some trouble (see below).

For more details on this change, see the commit message for 5e77573.

### Fix for the second example ###

The second example still didn't work: the same utility code function name was being generated for the two different memory view specializations because `MemoryViewSliceType.specialization_name()` was inherited from `BaseType` (in the same file) and thus the class generated names like `__pyx_convert_from_py__Pyx_memviewslice` no matter the base dtype. That gave rise to other problems in the generated code.

One way to address the issue, also attempted in this PR, is just to add dtype tag information to the specialization name string. I just appended the suffix generated by the existing method `MemoryViewSliceType.specialization_suffix()`. As a result, the generated naems look like `__pyx_convert_vector_from_py___Pyx_memviewslice_ds_double`.

For more details on this change, see the commit message for 6460e49.

## Questions ##

I'm not familiar with Cython's internals, so this fix could be misguided! In particular, while [basic tests](https://gist.github.com/mattjj/e9eef9aad52008d333b1) pass and my own more complex projects still buildand work fine, my changes could have side effects I'm not considering.

Are there any potential interactions here?

Relatedly, does this PR need automatic tests for these (simple) changes? If so, what tests are needed?