Mangle __arg argument names in methods (GH-3123)

Follows Python behaviour, but excludes "__pyx_…" names in utility code.

Closes GH-1382.

8 files changed, 472 insertions, 22 deletions

diff --git a/Cython/Compiler/FlowControl.py b/Cython/Compiler/FlowControl.py
index 80ce05c60..2e019ae1b 100644
--- a/Cython/Compiler/FlowControl.py
+++ b/Cython/Compiler/FlowControl.py
@@ -1294,7 +1294,7 @@ class ControlFlowAnalysis(CythonTransform):
         self.visitchildren(node, ('dict', 'metaclass',
                                   'mkw', 'bases', 'class_result'))
         self.flow.mark_assignment(node.target, node.classobj,
-                                  self.env.lookup(node.name))
+                                  self.env.lookup(node.target.name))
         self.env_stack.append(self.env)
         self.env = node.scope
         self.flow.nextblock()
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index 3f97d149f..509ae65b8 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -842,6 +842,16 @@ class CArgDeclNode(Node):
     def name_cstring(self):
         return self.name.as_c_string_literal()
 
+    @property
+    def hdr_cname(self):
+        # done lazily - needs self.entry to be set to get the class-mangled
+        # name, which means it has to be generated relatively late
+        if self.needs_conversion:
+            return punycodify_name(Naming.arg_prefix + self.entry.name)
+        else:
+            return punycodify_name(Naming.var_prefix + self.entry.name)
+
+
     def analyse(self, env, nonempty=0, is_self_arg=False):
         if is_self_arg:
             self.base_type.is_self_arg = self.is_self_arg = True
@@ -3051,10 +3061,6 @@ class DefNode(FuncDefNode):
                         arg.needs_type_test = 1
                     else:
                         arg.needs_conversion = 1
-            if arg.needs_conversion:
-                arg.hdr_cname = punycodify_name(Naming.arg_prefix + arg.name)
-            else:
-                arg.hdr_cname = punycodify_name(Naming.var_prefix + arg.name)
 
         if nfixed > len(self.args):
             self.bad_signature()
@@ -3738,7 +3744,7 @@ class DefNodeWrapper(FuncDefNode):
         all_args = tuple(positional_args) + tuple(kw_only_args)
         non_posonly_args = [arg for arg in all_args if not arg.pos_only]
         non_pos_args_id = ','.join(
-            ['&%s' % code.intern_identifier(arg.name) for arg in non_posonly_args] + ['0'])
+            ['&%s' % code.intern_identifier(arg.entry.name) for arg in non_posonly_args] + ['0'])
         code.putln("#if CYTHON_COMPILING_IN_LIMITED_API")
         code.putln("PyObject **%s[] = {%s};" % (
             Naming.pykwdlist_cname,
@@ -3818,7 +3824,7 @@ class DefNodeWrapper(FuncDefNode):
             code.putln('} else {')
             for i, arg in enumerate(kw_only_args):
                 if not arg.default:
-                    pystring_cname = code.intern_identifier(arg.name)
+                    pystring_cname = code.intern_identifier(arg.entry.name)
                     # required keyword-only argument missing
                     code.globalstate.use_utility_code(
                         UtilityCode.load_cached("RaiseKeywordRequired", "FunctionArguments.c"))
@@ -4035,7 +4041,7 @@ class DefNodeWrapper(FuncDefNode):
                         code.putln('default:')
                     else:
                         code.putln('case %2d:' % i)
-                pystring_cname = code.intern_identifier(arg.name)
+                pystring_cname = code.intern_identifier(arg.entry.name)
                 if arg.default:
                     if arg.kw_only:
                         # optional kw-only args are handled separately below
diff --git a/Cython/Compiler/ParseTreeTransforms.py b/Cython/Compiler/ParseTreeTransforms.py
index 0add92ca6..d0aace9c5 100644
--- a/Cython/Compiler/ParseTreeTransforms.py
+++ b/Cython/Compiler/ParseTreeTransforms.py
@@ -169,7 +169,6 @@ class PostParse(ScopeTrackingTransform):
     reorganization that can be refactored into this transform
     if a more pure Abstract Syntax Tree is wanted.
     """
-
     def __init__(self, context):
         super(PostParse, self).__init__(context)
         self.specialattribute_handlers = {
@@ -2216,7 +2215,7 @@ class CalculateQualifiedNamesTransform(EnvTransform):
     def visit_ClassDefNode(self, node):
         orig_qualified_name = self.qualified_name[:]
         entry = (getattr(node, 'entry', None) or             # PyClass
-                 self.current_env().lookup_here(node.name))  # CClass
+                 self.current_env().lookup_here(node.target.name))  # CClass
         self._append_entry(entry)
         self._super_visit_ClassDefNode(node)
         self.qualified_name = orig_qualified_name
diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py
index 131af1e3b..00e23370b 100644
--- a/Cython/Compiler/Symtab.py
+++ b/Cython/Compiler/Symtab.py
@@ -922,12 +922,14 @@ class Scope(object):
     def lookup(self, name):
         # Look up name in this scope or an enclosing one.
         # Return None if not found.
+        name = self.mangle_class_private_name(name)
         return (self.lookup_here(name)
             or (self.outer_scope and self.outer_scope.lookup(name))
             or None)
 
     def lookup_here(self, name):
         # Look up in this scope only, return None if not found.
+        name = self.mangle_class_private_name(name)
         return self.entries.get(name, None)
 
     def lookup_target(self, name):
@@ -1788,6 +1790,7 @@ class LocalScope(Scope):
 
     def declare_arg(self, name, type, pos):
         # Add an entry for an argument of a function.
+        name = self.mangle_class_private_name(name)
         cname = self.mangle(Naming.var_prefix, name)
         entry = self.declare(name, cname, type, pos, 'private')
         entry.is_variable = 1
@@ -1801,6 +1804,7 @@ class LocalScope(Scope):
     def declare_var(self, name, type, pos,
                     cname = None, visibility = 'private',
                     api = 0, in_pxd = 0, is_cdef = 0):
+        name = self.mangle_class_private_name(name)
         # Add an entry for a local variable.
         if visibility in ('public', 'readonly'):
             error(pos, "Local variable cannot be declared %s" % visibility)
@@ -1837,6 +1841,7 @@ class LocalScope(Scope):
     def lookup(self, name):
         # Look up name in this scope or an enclosing one.
         # Return None if not found.
+
         entry = Scope.lookup(self, name)
         if entry is not None:
             entry_scope = entry.scope
@@ -1998,6 +2003,17 @@ class ClassScope(Scope):
     #                          declared in the class
     #  doc    string or None   Doc string
 
+    def mangle_class_private_name(self, name):
+        # a few utilitycode names need to specifically be ignored
+        if name and name.lower().startswith("__pyx_"):
+            return name
+        return self.mangle_special_name(name)
+
+    def mangle_special_name(self, name):
+        if name and name.startswith('__') and not name.endswith('__'):
+            name = EncodedString('_%s%s' % (self.class_name.lstrip('_'), name))
+        return name
+
     def __init__(self, name, outer_scope):
         Scope.__init__(self, name, outer_scope, outer_scope)
         self.class_name = name
@@ -2031,18 +2047,6 @@ class PyClassScope(ClassScope):
 
     is_py_class_scope = 1
 
-    def mangle_class_private_name(self, name):
-        return self.mangle_special_name(name)
-
-    def mangle_special_name(self, name):
-        if name and name.startswith('__') and not name.endswith('__'):
-            name = EncodedString('_%s%s' % (self.class_name.lstrip('_'), name))
-        return name
-
-    def lookup_here(self, name):
-        name = self.mangle_special_name(name)
-        return ClassScope.lookup_here(self, name)
-
     def declare_var(self, name, type, pos,
                     cname = None, visibility = 'private',
                     api = 0, in_pxd = 0, is_cdef = 0):
@@ -2176,6 +2180,7 @@ class CClassScope(ClassScope):
     def declare_var(self, name, type, pos,
                     cname = None, visibility = 'private',
                     api = 0, in_pxd = 0, is_cdef = 0):
+        name = self.mangle_special_name(name)
         if is_cdef:
             # Add an entry for an attribute.
             if self.defined:
@@ -2282,6 +2287,7 @@ class CClassScope(ClassScope):
     def declare_cfunction(self, name, type, pos,
                           cname=None, visibility='private', api=0, in_pxd=0,
                           defining=0, modifiers=(), utility_code=None, overridable=False):
+        name = self.mangle_class_private_name(name)
         if get_special_method_signature(name) and not self.parent_type.is_builtin_type:
             error(pos, "Special methods must be declared with 'def', not 'cdef'")
         args = type.args
diff --git a/tests/run/methodmangling_T5.py b/tests/run/methodmangling_T5.py
index 1cfa85310..473a0201b 100644
--- a/tests/run/methodmangling_T5.py
+++ b/tests/run/methodmangling_T5.py
@@ -1,6 +1,10 @@
 # mode: run
 # ticket: 5
 
+# A small number of extra tests checking:
+# 1) this works correctly with pure-Python-mode decorators - methodmangling_pure.py.
+# 2) this works correctly with cdef classes - methodmangling_cdef.pyx
+
 class CyTest(object):
     """
     >>> cy = CyTest()
@@ -15,8 +19,23 @@ class CyTest(object):
 
     >>> '__x' in dir(cy)
     False
+    >>> cy._CyTest__y
+    2
+
+    >>> '_CyTest___more_than_two' in dir(cy)
+    True
+    >>> '___more_than_two' in dir(cy)
+    False
+    >>> '___more_than_two_special___' in dir(cy)
+    True
     """
     __x = 1
+    ___more_than_two = 3
+    ___more_than_two_special___ = 4
+
+    def __init__(self):
+        self.__y = 2
+
     def __private(self): return 8
 
     def get(self):
@@ -88,8 +107,285 @@ class _UnderscoreTest(object):
     1
     >>> ut.get()
     1
+    >>> ut._UnderscoreTest__UnderscoreNested().ret1()
+    1
+    >>> ut._UnderscoreTest__UnderscoreNested.__name__
+    '__UnderscoreNested'
+    >>> ut._UnderscoreTest__prop
+    1
     """
     __x = 1
 
     def get(self):
         return self.__x
+
+    class __UnderscoreNested(object):
+        def ret1(self):
+            return 1
+
+    @property
+    def __prop(self):
+        return self.__x
+
+class C:
+    error = """Traceback (most recent call last):
+...
+TypeError:
+"""
+    __doc__ = """
+>>> instance = C()
+
+Instance methods have their arguments mangled
+>>> instance.method1(__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+>>> instance.method1(_C__arg=1)
+1
+>>> instance.method2(__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+>>> instance.method2(_C__arg=1)
+1
+
+Works when optional argument isn't passed
+>>> instance.method2()
+None
+
+Where args are in the function's **kwargs dict, names aren't mangled
+>>> instance.method3(__arg=1) # doctest:
+1
+>>> instance.method3(_C__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+Traceback (most recent call last):
+...
+KeyError:
+
+Lambda functions behave in the same way:
+>>> instance.method_lambda(__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+>>> instance.method_lambda(_C__arg=1)
+1
+
+Class methods - have their arguments mangled
+>>> instance.class_meth(__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+>>> instance.class_meth(_C__arg=1)
+1
+>>> C.class_meth(__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+>>> C.class_meth(_C__arg=1)
+1
+
+Static methods - have their arguments mangled
+>>> instance.static_meth(__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+>>> instance.static_meth(_C__arg=1)
+1
+>>> C.static_meth(__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+>>> C.static_meth(_C__arg=1)
+1
+
+Functions assigned to the class don't have their arguments mangled
+>>> instance.class_assigned_function(__arg=1)
+1
+>>> instance.class_assigned_function(_C__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+
+Functions assigned to an instance don't have their arguments mangled
+>>> instance.instance_assigned_function = free_function2
+>>> instance.instance_assigned_function(__arg=1)
+1
+>>> instance.instance_assigned_function(_C__arg=1) # doctest: +IGNORE_EXCEPTION_DETAIL
+{error}
+
+Locals are reported as mangled
+>>> list(sorted(k for k in instance.get_locals(1).keys()))
+['_C__arg', 'self']
+""".format(error=error)
+
+    def method1(self, __arg):
+        print(__arg)
+
+    def method2(self, __arg=None):
+        # __arg is optional
+        print(__arg)
+
+    def method3(self, **kwargs):
+        print(kwargs['__arg'])
+
+    method_lambda = lambda self, __arg: __arg
+
+    def get_locals(self, __arg):
+        return locals()
+
+    @classmethod
+    def class_meth(cls, __arg):
+        print(__arg)
+
+    @staticmethod
+    def static_meth(__arg, dummy_arg=None):
+        # dummy_arg is to mask https://github.com/cython/cython/issues/3090
+        print(__arg)
+
+def free_function1(x, __arg):
+    print(__arg)
+
+def free_function2(__arg, dummy_arg=None):
+    # dummy_arg is to mask https://github.com/cython/cython/issues/3090
+    print(__arg)
+
+C.class_assigned_function = free_function1
+
+__global_arg = True
+
+_D__arg1 = None
+_D__global_arg = False # define these because otherwise Cython gives a compile-time error
+       # while Python gives a runtime error (which is difficult to test)
+def can_find_global_arg():
+    """
+    >>> can_find_global_arg()
+    True
+    """
+    return __global_arg
+
+def cant_find_global_arg():
+    """
+    Gets _D_global_arg instead
+    >>> cant_find_global_arg()
+    False
+    """
+    class D:
+        def f(self):
+            return __global_arg
+    return D().f()
+
+class CMultiplyNested:
+    def f1(self, __arg, name=None, return_closure=False):
+        """
+        >>> inst = CMultiplyNested()
+        >>> for name in [None, '__arg', '_CMultiplyNested__arg', '_D__arg']:
+        ...    try:
+        ...        print(inst.f1(1,name))
+        ...    except TypeError:
+        ...        print("TypeError") # not concerned about exact details
+        ...    # now test behaviour is the same in closures
+        ...    closure = inst.f1(1, return_closure=True)
+        ...    try:
+        ...        if name is None:
+        ...            print(closure(2))
+        ...        else:
+        ...            print(closure(**{ name: 2}))
+        ...    except TypeError:
+        ...        print("TypeError")
+        2
+        2
+        TypeError
+        TypeError
+        TypeError
+        TypeError
+        2
+        2
+        """
+        class D:
+            def g(self, __arg):
+                return __arg
+        if return_closure:
+            return D().g
+        if name is not None:
+            return D().g(**{ name: 2 })
+        else:
+            return D().g(2)
+
+    def f2(self, __arg1):
+        """
+        This finds the global name '_D__arg1'
+        It's tested in this way because without the global
+        Python gives a runtime error and Cython a compile error
+        >>> print(CMultiplyNested().f2(1))
+        None
+        """
+        class D:
+            def g(self):
+                return __arg1
+        return D().g()
+
+    def f3(self, arg, name):
+        """
+        >>> inst = CMultiplyNested()
+        >>> inst.f3(1, None)
+        2
+        >>> inst.f3(1, '__arg') # doctest: +IGNORE_EXCEPTION_DETAIL
+        Traceback (most recent call last):
+        ...
+        TypeError:
+        >>> inst.f3(1, '_CMultiplyNested__arg')
+        2
+        """
+        def g(__arg, dummy=1):
+            return __arg
+        if name is not None:
+            return g(**{ name: 2})
+        else:
+            return g(2)
+
+    def f4(self, __arg):
+        """
+        >>> CMultiplyNested().f4(1)
+        1
+        """
+        def g():
+            return __arg
+        return g()
+
+    def f5(self, __arg):
+        """
+        Default values are found in the outer scope correcly
+        >>> CMultiplyNested().f5(1)
+        1
+        """
+        def g(x=__arg):
+            return x
+        return g()
+
+    def f6(self, __arg1):
+        """
+        This will find the global name _D__arg1
+        >>> print(CMultiplyNested().f6(1))
+        None
+        """
+        class D:
+            def g(self, x=__arg1):
+                return x
+        return D().g()
+
+    def f7(self, __arg):
+        """
+        Lookup works in generator expressions
+        >>> list(CMultiplyNested().f7(1))
+        [1]
+        """
+        return (__arg for x in range(1))
+
+class __NameWithDunder:
+    """
+    >>> __NameWithDunder.__name__
+    '__NameWithDunder'
+    """
+    pass
+
+class Inherits(__NameWithDunder):
+    """
+    Compile check that it can find the base class
+    >>> x = Inherits()
+    """
+    pass
+
+def regular_function(__x, dummy=None):
+    # as before, dummy stops Cython creating a 1 arg, non-keyword call
+    return __x
+
+class CallsRegularFunction:
+    def call(self):
+        """
+        >>> CallsRegularFunction().call()
+        1
+        """
+        return regular_function(__x=1) # __x shouldn't be mangled as an argument elsewhere
diff --git a/tests/run/methodmangling_cdef.pxd b/tests/run/methodmangling_cdef.pxd
new file mode 100644
index 000000000..58a9130a4
--- /dev/null
+++ b/tests/run/methodmangling_cdef.pxd
@@ -0,0 +1,3 @@
+cdef class InPxd:
+    cdef public int __y
+    cdef int __private_cdef(self)
diff --git a/tests/run/methodmangling_cdef.pyx b/tests/run/methodmangling_cdef.pyx
new file mode 100644
index 000000000..ebcd7c290
--- /dev/null
+++ b/tests/run/methodmangling_cdef.pyx
@@ -0,0 +1,64 @@
+# mode: run
+
+def call_cdt_private_cdef(CDefTest o):
+    return o._CDefTest__private_cdef()
+
+cdef class CDefTest:
+    """
+    >>> cd = CDefTest()
+    >>> '_CDefTest__private' in dir(cd)
+    True
+    >>> cd._CDefTest__private()
+    8
+    >>> call_cdt_private_cdef(cd)
+    8
+    >>> '__private' in dir(cd)
+    False
+    >>> '_CDefTest__x' in dir(cd)
+    True
+
+    >>> '__x' in dir(cd)
+    False
+    >>> cd._CDefTest__y
+    2
+    """
+    __x = 1
+    cdef public int __y
+
+    def __init__(self):
+        self.__y = 2
+
+    def __private(self): return 8
+
+    cdef __private_cdef(self): return 8
+
+    def get(self):
+        """
+        >>> CDefTest().get()
+        (1, 1, 8)
+        """
+        return self._CDefTest__x, self.__x, self.__private()
+
+    def get_inner(self):
+        """
+        >>> CDefTest().get_inner()
+        (1, 1, 8)
+        """
+        def get(o):
+            return o._CDefTest__x, o.__x, o.__private()
+        return get(self)
+
+def call_inpdx_private_cdef(InPxd o):
+    return o._InPxd__private_cdef()
+
+cdef class InPxd:
+    """
+    >>> InPxd()._InPxd__y
+    2
+    >>> call_inpdx_private_cdef(InPxd())
+    8
+    """
+    def __init__(self):
+        self.__y = 2
+
+    cdef int __private_cdef(self): return 8
diff --git a/tests/run/methodmangling_pure.py b/tests/run/methodmangling_pure.py
new file mode 100644
index 000000000..ede968cb5
--- /dev/null
+++ b/tests/run/methodmangling_pure.py
@@ -0,0 +1,76 @@
+# mode: run
+# cython: language_level=3
+
+# This file tests that methodmangling is applied correctly to
+# pure Python decorated classes.
+
+import cython
+
+if cython.compiled:
+    # don't run in Python mode since a significant number of the tests
+    # are only for Cython features
+
+    def declare(**kwargs):
+        return kwargs['__x']
+
+    class RegularClass:
+        @cython.locals(__x=cython.int)
+        def f1(self, __x, dummy=None):
+            """
+            Is the locals decorator correctly applied
+            >>> c = RegularClass()
+            >>> c.f1(1)
+            1
+            >>> c.f1("a")
+            Traceback (most recent call last):
+            ...
+            TypeError: an integer is required
+            >>> c.f1(_RegularClass__x = 1)
+            1
+            """
+            return __x
+
+        def f2(self, x):
+            """
+            Is the locals decorator correctly applied
+            >>> c = RegularClass()
+            >>> c.f2(1)
+            1
+            >>> c.f2("a")
+            Traceback (most recent call last):
+            ...
+            TypeError: an integer is required
+            """
+            __x = cython.declare(cython.int, x)
+
+            return __x
+
+        def f3(self, x):
+            """
+            Is the locals decorator correctly applied
+            >>> c = RegularClass()
+            >>> c.f3(1)
+            1
+            >>> c.f3("a")
+            Traceback (most recent call last):
+            ...
+            TypeError: an integer is required
+            """
+            cython.declare(__x=cython.int)
+            __x = x
+
+            return __x
+
+        def f4(self, x):
+            """
+            We shouldn't be tripped up by a function called
+            "declare" that is nothing to do with cython
+            >>> RegularClass().f4(1)
+            1
+            """
+            return declare(__x=x)
+else:
+    __doc__ = """
+    >>> True
+    True
+    """ # stops Python2 from failing