Clean up cypclass subscript operations analysis

Cython/Compiler/ExprNodes.py

@@ -3812,7 +3812,7 @@ class IndexNode(_IndexingBaseNode):
         elif base_type.is_ptr or base_type.is_array:
             return self.analyse_as_c_array(env, is_slice)
         elif base_type.is_cyp_class:
-            return self.analyse_as_cyp(env, setting, deleting)
+            return self.analyse_as_cyp_class(env, setting, deleting)
         elif base_type.is_cpp_class:
             return self.analyse_as_cpp(env, setting)
         elif base_type.is_cfunction:
@@ -3892,14 +3892,17 @@ class IndexNode(_IndexingBaseNode):
             error(self.pos, "Invalid index type '%s'" % self.index.type)
         return self
 
+    def cpp_error(self, msg):
+        error(self.pos, msg)
+        self.type = PyrexTypes.error_type
+        self.result_code = "<error>"
+        return self
+
     def analyse_as_cpp(self, env, setting):
         base_type = self.base.type
         function = env.lookup_operator("[]", [self.base, self.index])
         if function is None:
-            error(self.pos, "Indexing '%s' not supported for index type '%s'" % (base_type, self.index.type))
-            self.type = PyrexTypes.error_type
-            self.result_code = "<error>"
-            return self
+            return self.cpp_error("Indexing '%s' not supported for index type '%s'" % (base_type, self.index.type))
         func_type = function.type
         if func_type.is_ptr:
             func_type = func_type.base_type
@@ -3916,54 +3919,74 @@ class IndexNode(_IndexingBaseNode):
             error(self.pos, "Can't set non-reference result '%s'" % self.type)
         return self
 
-    def analyse_as_cyp(self, env, setting, deleting):
+    def analyse_as_cyp_class(self, env, setting, deleting):
         if not setting:
-            return self.analyse_as_cpp(env, setting)
+            return self.analyse_cyp_class_getitem(env)
+        elif not deleting:
+            return self.analyse_cyp_class_setitem(env)
         else:
-            return self.analyse_as_cyp_setitem_or_delitem(env, deleting)
+            return self.analyse_cyp_class_delitem(env)
 
-    def analyse_cyp_setitem_or_delitem_method(self, env, deleting):
+    def analyse_cyp_class_getitem(self, env):
         base_type = self.base.type
-        function_name = "__delitem__" if deleting else "__setitem__"
-        function = base_type.scope.lookup_here(function_name)
+        function = base_type.scope.lookup("__getitem__")
         if function is None:
-            action = "Deleting" if deleting else "Setting"
-            error(self.pos, "%s item '%s' not supported for index type '%s'" % (action, base_type, self.index.type))
-            return None
-        if len(function.all_alternatives()) > 1:
-            error(self.pos, "%s.%s has several alternatives" % (function_name, base_type))
-            return None
+            return self.cpp_error("Cypclass %s does not support getting subscript item" % base_type)
+        elif len(function.all_alternatives()) != 1:
+            return self.cpp_error("%s.__getitem__ must have only one signature" % base_type)
         func_type = function.type
-        expected_nargs = 1 if deleting else 2
-        if len(func_type.args) != expected_nargs:
-            error(self.pos, "%s.%s takes wrong number of arguments" % (function_name, base_type))
-            return None
-        return func_type
+        if len(func_type.args) != 1:
+            return self.cpp_error("%s.__getitem__ must take one argument" % base_type)
 
-    def analyse_as_cyp_setitem_or_delitem(self, env, deleting):
+        self.exception_check = func_type.exception_check
+        self.exception_value = func_type.exception_value
+        if self.exception_check == '+':
+            self.is_temp = True
+            if self.exception_value is None:
+                env.use_utility_code(UtilityCode.load_cached("CppExceptionConversion", "CppSupport.cpp"))
+
+        self.index = self.index.coerce_to(function.type.args[0].type, env)
+        self.type = func_type.return_type
+        return self
+
+    def analyse_cyp_class_setitem(self, env):
         base_type = self.base.type
-        func_type = self.analyse_cyp_setitem_or_delitem_method(env, deleting)
-        if func_type is None:
-            self.type = PyrexTypes.error_type
-            self.result_code = "<error>"
-            return self
+        function = base_type.scope.lookup("__setitem__")
+        if function is None:
+            return self.cpp_error("Cypclass %s does not support setting subscript item" % base_type)
+        elif len(function.all_alternatives()) != 1:
+            return self.cpp_error("Cypclass method %s.__setitem__ must have only one signature" % base_type)
+        func_type = function.type
+        if len(func_type.args) != 2:
+            return self.cpp_error("Cypclass method %s.__setitem__ must take two arguments" % base_type)
+
         self.exception_check = func_type.exception_check
         self.exception_value = func_type.exception_value
         if self.exception_check == '+' and self.exception_value is None:
             env.use_utility_code(UtilityCode.load_cached("CppExceptionConversion", "CppSupport.cpp"))
-        self.index = self.index.coerce_to(func_type.args[0].type, env)
-        if not deleting:
-            self.type = func_type.args[1].type
-        else:
-            setitem_type = self.analyse_cyp_setitem_or_delitem_method(env, deleting=False)
-            if setitem_type is None:
-                # Hack: we use the setitem method to find the contained type and error out if there isn't one.
-                # We need it to add elements anyway, but a better way to find the contained type would be good.
-                error(self.pos, "Deleting element from collection without a __setitem__ method")
-                self.type = PyrexTypes.error_type
-                self.result_code = "<error>"
-                return self
-            self.type = setitem_type.args[1].type
+
+        self.index = self.index.coerce_to(function.type.args[0].type, env)
+        self.type = func_type.args[1].type
+        return self
+
+    def analyse_cyp_class_delitem(self, env):
+        base_type = self.base.type
+        function = base_type.scope.lookup("__delitem__")
+        if function is None:
+            return self.cpp_error("Cypclass %s does not support deleting subscript item" % base_type)
+        elif len(function.all_alternatives()) != 1:
+            return self.cpp_error("Cypclass method %s.__delitem__ must have only one signature" % base_type)
+        func_type = function.type
+        if len(func_type.args) != 1:
+            return self.cpp_error("Cypclass method %s.__delitem__ must take one argument" % base_type)
+
+        self.exception_check = func_type.exception_check
+        self.exception_value = func_type.exception_value
+        if self.exception_check == '+' and self.exception_value is None:
+            env.use_utility_code(UtilityCode.load_cached("CppExceptionConversion", "CppSupport.cpp"))
+
+        self.index = self.index.coerce_to(function.type.args[0].type, env)
+        self.type = func_type.return_type
         return self
 
     def analyse_as_c_function(self, env):
@@ -4333,7 +4356,7 @@ class IndexNode(_IndexingBaseNode):
                 self.extra_index_params(code)),
             self.pos))
 
-    def generate_cyp_setitem_code(self, value_code, code):
+    def generate_cyp_class_setitem_code(self, value_code, code):
         function = self.base.type.scope.lookup_here("__setitem__")
         function_code = function.cname
         setitem_code = "%s->%s(%s, %s);" % (
@@ -4356,7 +4379,7 @@ class IndexNode(_IndexingBaseNode):
         if self.type.is_pyobject:
             self.generate_setitem_code(rhs.py_result(), code)
         elif self.base.type.is_cyp_class:
-            self.generate_cyp_setitem_code(rhs.result(), code)
+            self.generate_cyp_class_setitem_code(rhs.result(), code)
         elif self.base.type is bytearray_type:
             value_code = self._check_byte_value(code, rhs)
             self.generate_setitem_code(value_code, code)
@@ -4420,7 +4443,7 @@ class IndexNode(_IndexingBaseNode):
             value_code = '((unsigned char)%s)' % value_code
         return value_code
 
-    def generate_cyp_delitem_code(self, code):
+    def generate_cyp_class_delitem_code(self, code):
         function = self.base.type.scope.lookup_here("__delitem__")
         function_code = function.cname
         setitem_code = "%s->%s(%s);" % (
@@ -4438,7 +4461,7 @@ class IndexNode(_IndexingBaseNode):
     def generate_deletion_code(self, code, ignore_nonexisting=False):
         self.generate_subexpr_evaluation_code(code)
         if self.base.type.is_cyp_class:
-            self.generate_cyp_delitem_code(code)
+            self.generate_cyp_class_delitem_code(code)
         else:
             #if self.type.is_pyobject:
             if self.index.type.is_int:

Cython/Compiler/Nodes.py

@@ -6458,6 +6458,10 @@ class DelStatNode(StatNode):
         for i, arg in enumerate(self.args):
             if arg.is_subscript:
                 arg = self.args[i] = arg.analyse_del_expression(env)
+                if arg.base.type.is_cyp_class:
+                    # We don't need to know the type of base[index], we'll just call base.__delitem__(index).
+                    self.cpp_check(env)
+                    return self
             else:
                 arg = self.args[i] = arg.analyse_target_expression(env, None)
             if arg.type.is_pyobject or (arg.is_name and arg.type.is_memoryviewslice):