cpdef method fails to compile in MSVC 9.0 #1566

Open
jcrist opened this Issue Dec 28, 2016 · 7 comments

Projects

None yet

4 participants

@jcrist
jcrist commented Dec 28, 2016

For a reason unbeknownst to me, changing a def method to a cpdef method results in code that compiles fine on linux/osx, but fails on windows with MSVC 9.0 for python 2.7.

To reproduce:

The failure seems to be caused by the calls to the two cpdef methods here. If you replace those lines with a pass, then everything works. MSVC reports syntax errors for open-parenthesis for the generated code, which then causes a bunch of failures down the line. The first two errors are:

crick/tdigest.c(4219) : error C2059: syntax error : '('
crick/tdigest.c(4228) : error C2059: syntax error : '('

The generated code is in a gist here

Please let me know if there's anything else needed to help debug. I also admit this may be user error - I'm fairly new to using Cython.

@robertwb
Contributor
@pitrou
pitrou commented Dec 28, 2016

For clarity, the two offending lines seem to be the following ones (generated C code):

      /* "crick/tdigest.pyx":196
 *         if range is None:
 *             if size != 0:
 *                 left = self.min()             # <<<<<<<<<<<<<<
 *                 right = self.max()
 *         else:
 */
      __pyx_v_left = ((struct __pyx_vtabstruct_5crick_7tdigest_TDigest *)__pyx_v_self->__pyx_vtab)->min(__pyx_v_self, 0);

      /* "crick/tdigest.pyx":197
 *             if size != 0:
 *                 left = self.min()
 *                 right = self.max()             # <<<<<<<<<<<<<<
 *         else:
 *             left = <double?>range[0]
 */
      __pyx_v_right = ((struct __pyx_vtabstruct_5crick_7tdigest_TDigest *)__pyx_v_self->__pyx_vtab)->max(__pyx_v_self, 0);

@pitrou
pitrou commented Dec 28, 2016

One random suggestion: perhaps min() and max() are macros for MSVC? How about you try to rename those two methods?

@jcrist
jcrist commented Dec 28, 2016

That fixes it, nice catch. Here is a simple test case that reproduces:

cdef class Test:
    cpdef double min(self):
        return 1

    cpdef double max(self):
        return 1

    def test(self):
        return self.min(), self.max()

If you switch cpdef to cdef, the error occurs, but you get a nice warning about "not enough parameters to macro min".

I tried rewriting the generated c code to ->(min)(...), but that didn't fix it (I thought it would, but I may have misunderstood how the c preprocessor works). Avoiding min and max as callable struct members did though. Perhaps just add a trailing underscore or something on cdef'd methods named min or max? There's probably other builtin macro names to avoid.

@pitrou
pitrou commented Dec 28, 2016

Yes, I suppose you can have the cdef functions or methods under a different name, while keeping min and max for the Python-facing APIs.

I have no idea how many macros there are to avoid, and it's certainly system-specific, so I'm not sure Cython can do that automatically. It's a pity that MSVC's error message is so obscure.

@jcrist
jcrist commented Dec 28, 2016

A couple ideas for a general solution:

  1. Add a prefix/postfix to all generated cdef methods
  2. Wrap names in parenthesis before calling to prevent function-like macro expansion (although this doesn't seem to work in this case, and I'm confused why not).
  3. Adding a macro defined as nothing between the name and the ( seems to work though:
#define CY_STOP_MACROS
stuff->min CY_STOP_MACROS (...)
@cython-notifications
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment