Change default of "always_allow_keywords" directive to True (GH-3605)

This avoids the METH_O function signature by default, since it does not match normal Python semantics.

* Fix unicode name handling of no-args functions when allowing keywords.

* Fix a crash when a keyword argument is passed to a function that does not allow them. Previously, the reported key name was not set and thus NULL.

* Extend "always_allow_keywords" test to cover some edge cases.
Some are commented out as they currently only work with the fastcall implementation.

CHANGES.rst

@@ -2,6 +2,16 @@
 Cython Changelog
 ================
 
+3.0.0 alpha 6 (2020-0?-??)
+==========================
+
+Bugs fixed
+----------
+
+* Single argument functions did not accept keyword arguments.
+  (Github issue #3090)
+
+
 3.0.0 alpha 5 (2020-05-19)
 ==========================
 

Cython/Compiler/Nodes.py

@@ -3663,8 +3663,8 @@ class DefNodeWrapper(FuncDefNode):
             code.globalstate.use_utility_code(
                 UtilityCode.load_cached("RaiseArgTupleInvalid", "FunctionArguments.c"))
             code.putln("if (unlikely(%s > 0)) {" % Naming.nargs_cname)
-            code.put('__Pyx_RaiseArgtupleInvalid("%s", 1, 0, 0, %s); return %s;' % (
-                self.name, Naming.nargs_cname, self.error_value()))
+            code.put('__Pyx_RaiseArgtupleInvalid(%s, 1, 0, 0, %s); return %s;' % (
+                self.name.as_c_string_literal(), Naming.nargs_cname, self.error_value()))
             code.putln("}")
 
         if self.starstar_arg:
@@ -3678,8 +3678,8 @@ class DefNodeWrapper(FuncDefNode):
         code.globalstate.use_utility_code(
             UtilityCode.load_cached("KeywordStringCheck", "FunctionArguments.c"))
         code.putln(
-            "if (%s && unlikely(!__Pyx_CheckKeywordStrings(%s, \"%s\", %d))) return %s;" % (
-                kwarg_check, Naming.kwds_cname, self.name,
+            "if (%s && unlikely(!__Pyx_CheckKeywordStrings(%s, %s, %d))) return %s;" % (
+                kwarg_check, Naming.kwds_cname, self.name.as_c_string_literal(),
                 bool(self.starstar_arg), self.error_value()))
 
         if self.starstar_arg and self.starstar_arg.entry.cf_used:

Cython/Compiler/Options.py

@@ -180,7 +180,7 @@ _directive_defaults = {
     'cdivision_warnings': False,
     'overflowcheck': False,
     'overflowcheck.fold': True,
-    'always_allow_keywords': False,
+    'always_allow_keywords': True,
     'allow_none_for_extension_args': True,
     'wraparound' : True,
     'ccomplex' : False,  # use C99/C++ for complex types and arith

Cython/Utility/FunctionArguments.c

@@ -144,8 +144,10 @@ static int __Pyx_CheckKeywordStrings(
     if (CYTHON_METH_FASTCALL && likely(PyTuple_Check(kw))) {
         if (unlikely(PyTuple_GET_SIZE(kw) == 0))
             return 1;
-        if (!kw_allowed)
+        if (!kw_allowed) {
+            key = PyTuple_GET_ITEM(kw, 0);
             goto invalid_keyword;
+        }
 #if PY_VERSION_HEX < 0x03090000
         // On CPython >= 3.9, the FASTCALL protocol guarantees that keyword
         // names are strings (see https://bugs.python.org/issue37540)

Cython/Utility/TestCythonScope.pyx

 ########## TestClass ##########
 # These utilities are for testing purposes
 
+# The "cythonscope" test calls METH_O functions with their (self, arg) signature.
+# cython: always_allow_keywords=False
+
 from __future__ import print_function
 
 cdef extern from *:

docs/src/userguide/source_files_and_compilation.rst

@@ -819,10 +819,10 @@ Cython code.  Here is the list of currently supported directives:
     False.
 
 ``always_allow_keywords`` (True / False)
-    Avoid the ``METH_NOARGS`` and ``METH_O`` when constructing
-    functions/methods which take zero or one arguments. Has no effect
-    on special methods and functions with more than one argument. The
-    ``METH_NOARGS`` and ``METH_O`` signatures provide faster
+    When disabled, uses the ``METH_NOARGS`` and ``METH_O`` signatures when
+    constructing functions/methods which take zero or one arguments. Has no
+    effect on special methods and functions with more than one argument. The
+    ``METH_NOARGS`` and ``METH_O`` signatures provide slightly faster
     calling conventions but disallow the use of keywords.
 
 ``profile`` (True / False)

tests/buffers/bufaccess.pyx

@@ -960,6 +960,7 @@ def decref(*args):
     for item in args: Py_DECREF(item)
 
 @cython.binding(False)
+@cython.always_allow_keywords(False)
 def get_refcount(x):
     return (<PyObject*>x).ob_refcnt
 

tests/compile/fused_redeclare_T3111.pyx

@@ -22,9 +22,9 @@ def foo(dtype_t[:] a, dtype_t_out[:, :] b):
 # "__pyxutil:16:4: '___pyx_npy_uint8' redeclared".  The remaining warnings are
 # unrelated to this test.
 _WARNINGS = """
-# cpdef redeclaration bug
-22:10: 'cpdef_method' redeclared
-33:10: 'cpdef_cname_method' redeclared
+# cpdef redeclaration bug, from TestCythonScope.pyx
+25:10: 'cpdef_method' redeclared
+36:10: 'cpdef_cname_method' redeclared
 # from MemoryView.pyx
 984:29: Ambiguous exception value, same as default return value: 0
 984:29: Ambiguous exception value, same as default return value: 0

tests/memoryview/memoryview.pyx

@@ -627,6 +627,7 @@ def decref(*args):
     for item in args: Py_DECREF(item)
 
 @cython.binding(False)
+@cython.always_allow_keywords(False)
 def get_refcount(x):
     return (<PyObject*>x).ob_refcnt
 

tests/memoryview/memslice.pyx

@@ -1059,6 +1059,7 @@ def decref(*args):
     for item in args: Py_DECREF(item)
 
 @cython.binding(False)
+@cython.always_allow_keywords(False)
 def get_refcount(x):
     return (<PyObject*>x).ob_refcnt
 

tests/run/always_allow_keywords_T295.pyx

+# mode: run
 # ticket: 295
 
 cimport cython
 
+import sys
+IS_PY2 = sys.version_info[0] == 2
+
+
 def assert_typeerror_no_keywords(func, *args, **kwds):
     # Python 3.9 produces an slightly different error message
     # to previous versions, so doctest isn't matching the
@@ -14,13 +19,18 @@ def assert_typeerror_no_keywords(func, *args, **kwds):
         assert False, "call did not raise TypeError"
 
 
+def func0():
+    """
+    >>> func0()
+    >>> func0(**{})
+    """
+
 def func1(arg):
     """
     >>> func1(None)
     >>> func1(*[None])
-    >>> assert_typeerror_no_keywords(func1, arg=None)
+    >>> func1(arg=None)
     """
-    pass
 
 @cython.always_allow_keywords(False)
 def func2(arg):
@@ -29,7 +39,6 @@ def func2(arg):
     >>> func2(*[None])
     >>> assert_typeerror_no_keywords(func2, arg=None)
     """
-    pass
 
 @cython.always_allow_keywords(True)
 def func3(arg):
@@ -40,26 +49,132 @@ def func3(arg):
     """
     pass
 
+
 cdef class A:
     """
-    >>> A().meth1(None)
-    >>> A().meth1(*[None])
-    >>> assert_typeerror_no_keywords(A().meth1, arg=None)
-    >>> A().meth2(None)
-    >>> A().meth2(*[None])
-    >>> assert_typeerror_no_keywords(A().meth2, arg=None)
-    >>> A().meth3(None)
-    >>> A().meth3(*[None])
-    >>> A().meth3(arg=None)
+    >>> class PyA(object):
+    ...     def meth0(self): pass
+    ...     def meth1(self, arg): pass
+
+    >>> PyA().meth0()
+    >>> PyA.meth0(PyA())
+    >>> if not IS_PY2: PyA.meth0(self=PyA())
+    >>> try: PyA().meth0(self=PyA())
+    ... except TypeError as exc: assert 'multiple' in str(exc), "Unexpected message: %s" % exc
+    ... else: assert False, "No TypeError when passing 'self' argument twice"
+
+    >>> PyA().meth1(1)
+    >>> PyA.meth1(PyA(), 1)
+    >>> PyA.meth1(PyA(), arg=1)
+    >>> if not IS_PY2: PyA.meth1(self=PyA(), arg=1)
     """
 
+    @cython.always_allow_keywords(False)
+    def meth0_nokw(self):
+        """
+        >>> A().meth0_nokw()
+        >>> A().meth0_nokw(**{})
+        >>> try: pass  #A.meth0_nokw(self=A())
+        ... except TypeError as exc: assert 'needs an argument' in str(exc), "Unexpected message: %s" % exc
+        ... else: pass  #assert False, "No TypeError for missing 'self' positional argument"
+        """
+
+    @cython.always_allow_keywords(True)
+    def meth0_kw(self):
+        """
+        >>> A().meth0_kw()
+        >>> A().meth0_kw(**{})
+        >>> A.meth0_kw(A())
+        >>> #A.meth0_kw(self=A())
+        >>> try: pass  #A().meth0_kw(self=A())
+        ... except TypeError as exc: assert 'multiple' in str(exc), "Unexpected message: %s" % exc
+        ... else: pass  #assert False, "No TypeError when passing 'self' argument twice"
+        """
+
+    @cython.always_allow_keywords(True)
+    def meth1_kw(self, arg):
+        """
+        >>> A().meth1_kw(None)
+        >>> A().meth1_kw(*[None])
+        >>> A().meth1_kw(arg=None)
+        >>> A.meth1_kw(A(), arg=None)
+        >>> #A.meth1_kw(self=A(), arg=None)
+        """
+
+    @cython.always_allow_keywords(False)
+    def meth1_nokw(self, arg):
+        """
+        >>> A().meth1_nokw(None)
+        >>> A().meth1_nokw(*[None])
+        >>> assert_typeerror_no_keywords(A().meth1_nokw, arg=None)
+        >>> assert_typeerror_no_keywords(A.meth1_nokw, A(), arg=None)
+        >>> try: pass  # A.meth1_nokw(self=A(), arg=None)
+        ... except TypeError as exc: assert 'needs an argument' in str(exc), "Unexpected message: %s" % exc
+        ... else: pass  # assert False, "No TypeError for missing 'self' positional argument"
+        """
+
+    @cython.always_allow_keywords(False)
+    def meth2(self, arg):
+        """
+        >>> A().meth2(None)
+        >>> A().meth2(*[None])
+        >>> assert_typeerror_no_keywords(A().meth2, arg=None)
+        """
+
+    @cython.always_allow_keywords(True)
+    def meth3(self, arg):
+        """
+        >>> A().meth3(None)
+        >>> A().meth3(*[None])
+        >>> A().meth3(arg=None)
+        """
+
+
+class B(object):
+    @cython.always_allow_keywords(False)
+    def meth0_nokw(self):
+        """
+        >>> B().meth0_nokw()
+        >>> B().meth0_nokw(**{})
+        >>> if not IS_PY2: assert_typeerror_no_keywords(B.meth0_nokw, self=B())
+        """
+
+    @cython.always_allow_keywords(True)
+    def meth0_kw(self):
+        """
+        >>> B().meth0_kw()
+        >>> B().meth0_kw(**{})
+        >>> B.meth0_kw(B())
+        >>> if not IS_PY2: B.meth0_kw(self=B())
+        >>> try: B().meth0_kw(self=B())
+        ... except TypeError as exc: assert 'multiple' in str(exc), "Unexpected message: %s" % exc
+        ... else: assert False, "No TypeError when passing 'self' argument twice"
+        """
+
+    @cython.always_allow_keywords(True)
     def meth1(self, arg):
-        pass
+        """
+        >>> B().meth1(None)
+        >>> B().meth1(*[None])
+        >>> B().meth1(arg=None)
+        >>> B.meth1(B(), arg=None)
+        >>> if not IS_PY2: B.meth1(self=B(), arg=None)
+        """
 
     @cython.always_allow_keywords(False)
     def meth2(self, arg):
-        pass
+        """
+        >>> B().meth2(None)
+        >>> B().meth2(*[None])
+        >>> B.meth2(B(), None)
+        >>> if not IS_PY2: B.meth2(self=B(), arg=None)
+        >>> B().meth2(arg=None)  # assert_typeerror_no_keywords(B().meth2, arg=None)  -> not a cdef class!
+        """
 
     @cython.always_allow_keywords(True)
     def meth3(self, arg):
-        pass
+        """
+        >>> B().meth3(None)
+        >>> B().meth3(*[None])
+        >>> B().meth3(arg=None)
+        """

tests/run/cyfunction_METH_O_GH1728.pyx

@@ -8,9 +8,9 @@ cdef class TestMethodOneArg:
 
 def call_meth(x):
     """
-    >>> call_meth(TestMethodOneArg())
+    >>> call_meth(TestMethodOneArg())  # doctest: +ELLIPSIS
     Traceback (most recent call last):
     ...
-    TypeError: meth() takes exactly one argument (0 given)
+    TypeError: meth() takes exactly ... argument (0 given)
     """
     return x.meth()

tests/run/exceptionrefcount.pyx

@@ -27,8 +27,11 @@ __doc__ = u"""
 >>> run_test(50, test_finally)
 """
 
+cimport cython
 from cpython.ref cimport PyObject
 
+@cython.binding(False)
+@cython.always_allow_keywords(False)
 def get_refcount(obj):
     return (<PyObject*>obj).ob_refcnt
 

tests/run/refcount_in_meth.pyx

@@ -12,8 +12,10 @@ True
 True
 """
 
+cimport cython
 from cpython.ref cimport PyObject
 
+@cython.always_allow_keywords(False)
 def get_refcount(obj):
     return (<PyObject*>obj).ob_refcnt
 

tests/run/unicode_identifiers.pyx

 # -*- coding: utf-8 -*-
-# cython: language_level=3
 # mode: run
 # tag: pep3131, traceback
 
+# cython: language_level=3
+
 # Code with unicode identifiers can be compiled with Cython running either Python 2 or 3.
 # However Python access to unicode identifiers is only possible in Python 3. In Python 2
 # it's only really safe to use the unicode identifiers for purely Cython interfaces
@@ -11,10 +12,13 @@
 # This is controlled by putting the Python3 only tests in the module __doc__ attribute
 # Most of the individual function and class docstrings are only present as a compile test
 
+cimport cython
+
 import sys
 
-if sys.version_info[0]>2:
-    __doc__ = """
+
+if sys.version_info[0] > 2:
+    __doc__ = u"""
     >>> f()()
     2
     >>> f().__name__
@@ -37,6 +41,9 @@ if sys.version_info[0]>2:
     >>> print(x.α)
     200
 
+    >>> B().Ƒ()
+    >>> C().Ƒ()
+
     Test generation of locals()
     >>> sorted(Γναμε2().boring_function(1,2).keys())
     ['self', 'somevalue', 'x', 'ναμε5', 'ναμε6']
@@ -45,6 +52,8 @@ if sys.version_info[0]>2:
     0
     >>> function_taking_fancy_argument(Γναμε2()).ναμε3
     1
+    >>> metho_function_taking_fancy_argument(Γναμε2()).ναμε3
+    1
     >>> NormalClassΓΓ().ναμε
     10
     >>> NormalClassΓΓ().εxciting_function(None).__qualname__
@@ -81,7 +90,7 @@ cdef class A:
     def __init__(self):
         self.ναμε = 1
     cdef Ƒ(self):
-        return self.ναμε==1
+        return self.ναμε == 1
     def regular_function(self):
         """
         Can use unicode cdef functions and (private) attributes internally
@@ -174,9 +183,16 @@ cdef class Derived(Γναμε2):
 
 cdef Γναμε2 global_ναμε3 = Γναμε2()
 
+
+@cython.always_allow_keywords(False)  # METH_O signature
+def metho_function_taking_fancy_argument(Γναμε2 αrγ):
+    return αrγ
+
+@cython.always_allow_keywords(True)
 def function_taking_fancy_argument(Γναμε2 αrγ):
     return αrγ
 
+
 class NormalClassΓΓ(Γναμε2):
     """
     docstring