Special-case enum method calls

[ilevkivskyi]

Improves mypyc/mypyc#1121, this gives a bit above 1% on mypy self-check.

This only adds support for regular and overloaded methods without decorators (class/static methods and properties stay slow). When working on this I considered (and actually tried) four options:

• Make enums extension classes, then many methods will use fast calls ~automatically (we will just need to set a final flag). This just didn't work, in the sense no segfaults, but it looks like we don't call __prepare__(), or don't call it at the right moment. Or maybe I just didn't try hard enough. In general, for some reason this feels risky.
• Use existing CPyDefs for (non-extension) enum methods, but since they have an extra argument, __mypyc_self__, we can supply NULL there, since we know it is unused. This is actually easy and it works, but IMO it is ultra-ugly, so I decided to not do it.
• Write a separate CPyDef without __mypy_self__, use it for direct calls, and make existing callable classes CPyDefs one-line functions that simply call the first one. This is possible, but quite complicated, and I am not sure it is easy to generalize (e.g. on classmethods).
• Finally, the way I do this is to simply generate a second method, that is almost a copy of the original one. This involves a bit of code duplication (in C), but the benefit is that it is conceptually simple, and easily extendable. We can cover more special cases on as-needed basis.

[ilevkivskyi]

Btw, this same idea can be applied for:

• All non-extension classes explicitly marked as final
• All methods in non-final non-extension classes that are explicitly marked as final

I will however defer these cases, unless I will find that mypy itself will benefit from these.

[ilevkivskyi]

Oh interesting, there is another 3.9 test failure because there is no Self in typing, but it causes a segfault. I can reproduce this locally. Looks like a pre-existing bug (happens on master as well). Interestingly, this happens only with non-extension classes, simple repro

[case testSegfaultOnUndefined]
from mypy_extensions import mypyc_attr
from typing import NoWay  # type: ignore

@mypyc_attr(native_class=False)
class Test:
    def next(self) -> NoWay:
        return 1

Also, segfault happens after printing the traceback. I guess we add an incref too late for the method register (which only exists for non-extension classes) or for the class object register itself, or something similar. I will look tomorrow if there is a simple fix, if no simple fix, I will ignore this, as this is not related to my PR.

[ilevkivskyi]

I think I see the bug

146      char result = CPyDef___top_level__();
147      if (result == 2)
148          goto fail;
149      Py_DECREF(modname);
150      return 0;
151      fail:
152      Py_CLEAR(CPyModule_native_internal);
153      Py_CLEAR(modname);
154      Py_CLEAR(CPyType_Test);
155      Py_CLEAR(CPyType_next_Test_obj);

but CPyType_Test is only initialized within __top_level__() near the end (in a place we didn't reach yet because of an import error), so we may be decrefing NULL.

[ilevkivskyi]

It looks a bit trickier than that, in gdb I see segfault occurs on this line

PyMem_Free((char *)type->tp_doc);

now here is a punishment for not writing docstrings :-)

[ilevkivskyi]

    /* A type's tp_doc is heap allocated, unlike the tp_doc slots
     * of most other objects.  It's okay to cast it to char *.
     */

hm LOL doesn't look that way

99       .tp_doc = "next_Test_obj()\n--\n\n",

[ilevkivskyi]

It looks like this was broken recently in #19307 cc @brianschubert

[ilevkivskyi]

Well, adding some conditional logic looks annoying, I guess we should simply skip adding .tp_doc for non-extension classes.

[ilevkivskyi]

Oh wait, this applies to extension classes as well. Then we should always allocate the docstring on the heap. @brandtbucher I hope you can fix this, segfault on an uncaught exception is a pretty bad behavior. TBH I still don't understand what exactly is going on, like why don't we have this problem if the program exits normally (i.e. without exception)? A minimal test to reproduce:

[case testDocExplosion]
class Empty: pass
raise

generates

*** Exit status: -6
  munmap_chunk(): invalid pointer

[brianschubert]

Still catching up, but would something like this do the trick?

diff --git a/mypyc/lib-rt/misc_ops.c b/mypyc/lib-rt/misc_ops.c
index 3787ea553..23981f9f3 100644
--- a/mypyc/lib-rt/misc_ops.c
+++ b/mypyc/lib-rt/misc_ops.c
@@ -300,6 +300,19 @@ PyObject *CPyType_FromTemplate(PyObject *template,
 
     Py_XDECREF(dummy_class);
 
+    if (template_->tp_doc) {
+        // Silently truncate the docstring if it contains a null byte
+        Py_ssize_t size = strlen(template_->tp_doc) + 1;
+        char *tp_doc = (char *)PyMem_Malloc(size);
+        if (tp_doc == NULL) {
+            PyErr_NoMemory();
+            goto error;
+        }
+
+        memcpy(tp_doc, template_->tp_doc, size);
+        t->ht_type.tp_doc = tp_doc;
+    }
+
 #if PY_MINOR_VERSION == 11
     // This is a hack. Python 3.11 doesn't include good public APIs to work with managed
     // dicts, which are the default for heap types. So we try to opt-out until Python 3.12.

Following after https://github.com/python/cpython/blob/485b16b4f7b28cefdfb524c2869d473078e349bf/Objects/typeobject.c#L4530-L4540

That seems to resolve the seg fault

[ilevkivskyi]

Yeah, that should work, I think we can simply add that chunk to CPyType_FromTemplate.

[ilevkivskyi]

(this however still doesn't explain why we don't fail without exception, this means we may have an extra incref for class objects, but this is a much smaller problem)

[sterliakov]

I also explored your options 1 and 3, with the same result for 1 and "no, doesn't look right" feeling regarding 3. Such duplication looks quite reasonable here - after all, normally enums are small and do not account for significant portion of a codebase, so this extra size should be negligible.

I'll try to look deeper into this tonight, thanks!

[brandtbucher]

@brandtbucher I hope you can fix this, segfault on an uncaught exception is a pretty bad behavior.

I'm guessing this is a misspelling of "@brianschubert"? Otherwise, sorry to disappoint, but I'm not sure how I can help with this issue specifically. :)

[ilevkivskyi]

@brandtbucher Oops, this is some serious misspelling, sorry :-)