From 312bc218bb775ac3a87be18da8a1ce6d91213df4 Mon Sep 17 00:00:00 2001 From: da-woods Date: Thu, 1 Sep 2022 21:02:56 +0100 Subject: Alternative implementation of mapping that avoids varargs in favour of fixed arrays --- Cython/Compiler/MatchCaseNodes.py | 194 +++++++++++++++++++++++++------------- Cython/Utility/MatchCase.c | 180 +++++++++++++---------------------- 2 files changed, 196 insertions(+), 178 deletions(-) diff --git a/Cython/Compiler/MatchCaseNodes.py b/Cython/Compiler/MatchCaseNodes.py index 684f8cc45..4f00e480d 100644 --- a/Cython/Compiler/MatchCaseNodes.py +++ b/Cython/Compiler/MatchCaseNodes.py @@ -945,30 +945,32 @@ class MatchMappingPatternNode(PatternNode): ], exception_value="-1", ) + # lie about the types of keys for simplicity Pyx_mapping_check_duplicates_type = PyrexTypes.CFuncType( PyrexTypes.c_int_type, [ - PyrexTypes.CFuncTypeArg("fixed_keys", PyrexTypes.py_object_type, None), - PyrexTypes.CFuncTypeArg("var_keys", PyrexTypes.py_object_type, None), + PyrexTypes.CFuncTypeArg("keys", PyrexTypes.c_void_ptr_type, None), + PyrexTypes.CFuncTypeArg("nKeys", PyrexTypes.c_py_ssize_t_type, None), ], exception_value="-1", ) + # lie about the types of keys and subjects for simplicity Pyx_mapping_extract_subjects_type = PyrexTypes.CFuncType( PyrexTypes.c_bint_type, [ - PyrexTypes.CFuncTypeArg("map", PyrexTypes.py_object_type, None), - PyrexTypes.CFuncTypeArg("fixed_keys", PyrexTypes.py_object_type, None), - PyrexTypes.CFuncTypeArg("var_keys", PyrexTypes.py_object_type, None), + PyrexTypes.CFuncTypeArg("mapping", PyrexTypes.py_object_type, None), + PyrexTypes.CFuncTypeArg("keys", PyrexTypes.c_void_ptr_type, None), + PyrexTypes.CFuncTypeArg("nKeys", PyrexTypes.c_py_ssize_t_type, None), + PyrexTypes.CFuncTypeArg("subjects", PyrexTypes.c_void_ptr_ptr_type, None), ], exception_value="-1", - has_varargs=True, ) Pyx_mapping_doublestar_type = PyrexTypes.CFuncType( Builtin.dict_type, [ - PyrexTypes.CFuncTypeArg("map", PyrexTypes.py_object_type, None), - PyrexTypes.CFuncTypeArg("fixed_keys", PyrexTypes.py_object_type, None), - PyrexTypes.CFuncTypeArg("var_keys", PyrexTypes.py_object_type, None), + PyrexTypes.CFuncTypeArg("mapping", PyrexTypes.py_object_type, None), + PyrexTypes.CFuncTypeArg("keys", PyrexTypes.c_void_ptr_type, None), + PyrexTypes.CFuncTypeArg("nKeys", PyrexTypes.c_py_ssize_t_type, None), ], ) @@ -1085,8 +1087,10 @@ class MatchMappingPatternNode(PatternNode): self.pos, arg=subject_node, fallback=call, check=self.is_dict_type_check ) - def make_duplicate_keys_check(self, static_keys_tuple, var_keys_tuple): + def make_duplicate_keys_check(self, n_fixed_keys): utility_code = UtilityCode.load_cached("MappingKeyCheck", "MatchCase.c") + if n_fixed_keys == len(self.keys): + return None # nothing to check return Nodes.ExprStatNode( self.pos, @@ -1095,11 +1099,15 @@ class MatchMappingPatternNode(PatternNode): "__Pyx_MatchCase_CheckDuplicateKeys", self.Pyx_mapping_check_duplicates_type, utility_code=utility_code, - args=[static_keys_tuple.clone_node(), var_keys_tuple], + args=[ + MappingComparisonNode.make_keys_node(self.pos), + ExprNodes.IntNode(self.pos, value=str(n_fixed_keys)), + ExprNodes.IntNode(self.pos, value=str(len(self.keys))) + ], ), ) - def check_all_keys(self, subject_node, const_keys_tuple, var_keys_tuple): + def check_all_keys(self, subject_node): # It's debatable here whether to go for individual unpacking or a function. # Current implementation is a function that's loosely copied from CPython. # For small numbers of keys it might be better to generate the code instead. @@ -1125,24 +1133,23 @@ class MatchMappingPatternNode(PatternNode): util_code = UtilityCode.load_cached("ExtractGeneric", "MatchCase.c") func_name = "__Pyx_MatchCase_Mapping_Extract" - subject_derefs = [ - ExprNodes.NullNode(self.pos) - if t is None - else AddressOfPyObjectNode(self.pos, obj=t) - for t in self.subject_temps - ] return ExprNodes.PythonCapiCallNode( self.pos, func_name, self.Pyx_mapping_extract_subjects_type, utility_code=util_code, - args=[subject_node, const_keys_tuple.clone_node(), var_keys_tuple] - + subject_derefs, + args=[ + subject_node, + MappingComparisonNode.make_keys_node(self.pos), + ExprNodes.IntNode( + self.pos, + value=str(len(self.keys)) + ), + MappingComparisonNode.make_subjects_node(self.pos), + ], ) - def make_double_star_capture( - self, subject_node, const_tuple, var_tuple, test_result - ): + def make_double_star_capture(self, subject_node, test_result): # test_result being the variable that holds "case check passed until now" is_dict = self.is_dict_type_check(subject_node.type) if is_dict: @@ -1159,7 +1166,11 @@ class MatchMappingPatternNode(PatternNode): "__Pyx_MatchCase_DoubleStarCapture" + tag, self.Pyx_mapping_doublestar_type, utility_code=utility_code, - args=[subject_node, const_tuple, var_tuple], + args=[ + subject_node, + MappingComparisonNode.make_keys_node(self.pos), + ExprNodes.IntNode(self.pos, value=str(len(self.keys))) + ], ) assignment = Nodes.SingleAssignmentNode( self.double_star_capture_target.pos, lhs=self.double_star_temp, rhs=func @@ -1176,22 +1187,21 @@ class MatchMappingPatternNode(PatternNode): def get_comparison_node(self, subject_node, sequence_mapping_temp=None): from . import UtilNodes - const_keys = [] + keys_for_array = [] var_keys = [] + n_literal_keys = 0 for k in self.keys: if not k.arg.is_literal: k = UtilNodes.ResultRefNode(k, is_temp=False) + keys_for_array.append(k) var_keys.append(k) else: - const_keys.append(k.arg.clone_node()) - const_keys_tuple = ExprNodes.TupleNode(self.pos, args=const_keys) - var_keys_tuple = ExprNodes.TupleNode(self.pos, args=var_keys) - if var_keys: - var_keys_tuple = UtilNodes.ResultRefNode(var_keys_tuple, is_temp=True) + keys_for_array.append(k.arg.clone_node()) + n_literal_keys += 1 all_tests = [] all_tests.append(self.make_mapping_check(subject_node, sequence_mapping_temp)) - all_tests.append(self.check_all_keys(subject_node, const_keys_tuple, var_keys_tuple)) + all_tests.append(self.check_all_keys(subject_node)) if any(isinstance(test, ExprNodes.BoolNode) and not test.value for test in all_tests): # identify automatic-failure @@ -1206,10 +1216,10 @@ class MatchMappingPatternNode(PatternNode): all_tests = generate_binop_tree_from_list(self.pos, "and", all_tests) test_result = UtilNodes.ResultRefNode(pos=self.pos, type=PyrexTypes.c_bint_type) + duplicate_check = self.make_duplicate_keys_check(n_literal_keys) body = Nodes.StatListNode( self.pos, - stats=[ - self.make_duplicate_keys_check(const_keys_tuple, var_keys_tuple), + stats=([duplicate_check] if duplicate_check else []) + [ Nodes.SingleAssignmentNode(self.pos, lhs=test_result, rhs=all_tests), ], ) @@ -1217,21 +1227,24 @@ class MatchMappingPatternNode(PatternNode): assert self.double_star_temp body.stats.append( # make_double_star_capture wraps itself in an if - self.make_double_star_capture( - subject_node, const_keys_tuple, var_keys_tuple, test_result - ) + self.make_double_star_capture(subject_node, test_result) ) - if var_keys or self.double_star_capture_target: + if duplicate_check or self.double_star_capture_target: body = UtilNodes.TempResultFromStatNode(test_result, body) - if var_keys: - body = UtilNodes.EvalWithTempExprNode(var_keys_tuple, body) - for k in var_keys: - if isinstance(k, UtilNodes.ResultRefNode): - body = UtilNodes.EvalWithTempExprNode(k, body) - return LazyCoerceToBool(body.pos, arg=body) else: - return LazyCoerceToBool(all_tests.pos, arg=all_tests) + body = all_tests + if keys_for_array or self.double_star_capture_target: + body = MappingComparisonNode( + body.pos, + arg=LazyCoerceToBool(body.pos, arg=body), + keys_array=keys_for_array, + subjects_array=self.subject_temps + ) + for k in var_keys: + if isinstance(k, UtilNodes.ResultRefNode): + body = UtilNodes.EvalWithTempExprNode(k, body) + return LazyCoerceToBool(body.pos, arg=body) def analyse_pattern_expressions(self, env, sequence_mapping_temp): def to_temp_or_literal(node): @@ -1610,27 +1623,6 @@ class CompilerDirectivesExprNode(ExprNodes.ProxyNode): self.arg.annotate(code) -class AddressOfPyObjectNode(ExprNodes.ExprNode): - """ - obj - some temp node - """ - - type = PyrexTypes.c_void_ptr_ptr_type - is_temp = False - subexprs = ["obj"] - - def analyse_types(self, env): - self.obj = self.obj.analyse_types(env) - assert self.obj.type.is_pyobject, repr(self.obj.type) - return self - - def generate_result_code(self, code): - self.obj.generate_result_code(code) - - def calculate_result_code(self): - return "&%s" % self.obj.result() - - class LazyCoerceToPyObject(ExprNodes.ExprNode): """ Just calls "self.arg.coerce_to_pyobject" when it's analysed, @@ -1680,4 +1672,74 @@ def generate_binop_tree_from_list(pos, operator, list_of_tests): operator=operator, operand1=operand1, operand2=operand2 - ) \ No newline at end of file + ) + +class MappingComparisonNode(ExprNodes.ExprNode): + """ + Sets up the arrays of subjects and keys + + has attributes: + * arg - the main comparison node + * keys_array - list of ExprNodes representing keys + * subjects_array - list of ExprNodes representing subjects + """ + subexprs = ['arg', 'keys_array'] + keys_array_cname = "__pyx_match_mapping_keys" + subjects_array_cname = "__pyx_match_mapping_subjects" + + @property + def type(self): + return self.arg.type + + @classmethod + def make_keys_node(cls, pos): + return ExprNodes.RawCNameExprNode( + pos, + type=PyrexTypes.c_void_ptr_type, + cname=cls.keys_array_cname + ) + + @classmethod + def make_subjects_node(cls, pos): + return ExprNodes.RawCNameExprNode( + pos, + type=PyrexTypes.c_void_ptr_ptr_type, + cname=cls.subjects_array_cname + ) + + def analyse_types(self, env): + self.arg = self.arg.analyse_types(env) + for n in range(len(self.keys_array)): + key = self.keys_array[n].analyse_types(env) + key = key.coerce_to_pyobject(env) + self.keys_array[n] = key + assert self.arg.type is PyrexTypes.c_bint_type + return self + + def generate_evaluation_code(self, code): + code.putln("{") + decl_insertion_point = code.insertion_point() + super(MappingComparisonNode, self).generate_evaluation_code(code) + + # defer evaluating k.result() until we've processed them in + # generate_subexpr_evaluation_code. This order is OK - the keys + # are either constants or ResultRefNodes + keys_str = ", ".join(k.result() for k in self.keys_array) + decl_insertion_point.putln("PyObject *%s[] = {%s};" % ( + self.keys_array_cname, + keys_str, + )) + subjects_str = ", ".join( + "&"+subject.result() if subject is not None else "NULL" for subject in self.subjects_array + ) + decl_insertion_point.putln("PyObject **%s[] = {%s};" % ( + self.subjects_array_cname, + subjects_str + )) + code.putln("}") + + def generate_result_code(self, code): + pass + + def calculate_result_code(self): + return self.arg.result() \ No newline at end of file diff --git a/Cython/Utility/MatchCase.c b/Cython/Utility/MatchCase.c index 85eb6bf0a..3f71ed1e8 100644 --- a/Cython/Utility/MatchCase.c +++ b/Cython/Utility/MatchCase.c @@ -456,13 +456,13 @@ static int __Pyx_MatchCase_IsMapping(PyObject *o, unsigned int *sequence_mapping //////////////////////// MappingKeyCheck.proto ///////////////////////// -static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *fixed_keys, PyObject *var_keys); /*proto */ +static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *keys[], Py_ssize_t nFixedKeys, Py_ssize_t nKeys); /*proto */ /////////////////////// MappingKeyCheck //////////////////////////////// -static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *fixed_keys, PyObject *var_keys) { - // Inputs are tuples, and typically fairly small. It may be more efficient to - // loop over the tuple than create a set. +static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *keys[], Py_ssize_t nFixedKeys, Py_ssize_t nKeys) { + // Inputs are arrays, and typically fairly small. It may be more efficient to + // loop over the array than create a set. // The CPython implementation (match_keys in ceval.c) does this concurrently with // taking the keys out of the dictionary. I'm choosing to do it separately since the @@ -477,8 +477,8 @@ static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *fixed_keys, PyObject *va var_keys_set = PySet_New(NULL); if (!var_keys_set) return -1; - for (n=0; n < PyTuple_GET_SIZE(var_keys); ++n) { - key = PyTuple_GET_ITEM(var_keys, n); + for (n=nFixedKeys; n < nKeys; ++n) { + key = keys[n]; contains = PySet_Contains(var_keys_set, key); if (contains < 0) { goto bad; @@ -490,8 +490,8 @@ static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *fixed_keys, PyObject *va } } } - for (n=0; n < PyTuple_GET_SIZE(fixed_keys); ++n) { - key = PyTuple_GET_ITEM(fixed_keys, n); + for (n=0; n < nFixedKeys; ++n) { + key = keys[n]; contains = PySet_Contains(var_keys_set, key); if (contains < 0) { goto bad; @@ -521,48 +521,31 @@ static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *fixed_keys, PyObject *va #if CYTHON_REFNANNY #define __Pyx_MatchCase_Mapping_ExtractDict(...) __Pyx__MatchCase_Mapping_ExtractDict(__pyx_refnanny, __VA_ARGS__) -#define __Pyx_MatchCase_Mapping_ExtractDictV(...) __Pyx__MatchCase_Mapping_ExtractDictV(__pyx_refnanny, __VA_ARGS__) #else #define __Pyx_MatchCase_Mapping_ExtractDict(...) __Pyx__MatchCase_Mapping_ExtractDict(NULL, __VA_ARGS__) -#define __Pyx_MatchCase_Mapping_ExtractDictV(...) __Pyx__MatchCase_Mapping_ExtractDictV(NULL, __VA_ARGS__) #endif -static CYTHON_INLINE int __Pyx__MatchCase_Mapping_ExtractDict(void *__pyx_refnanny, PyObject *dict, PyObject *fixed_keys, PyObject *var_keys, ...); /* proto */ -static int __Pyx__MatchCase_Mapping_ExtractDictV(void *__pyx_refnanny, PyObject *dict, PyObject *fixed_keys, PyObject *var_keys, va_list subjects); /* proto */ +static CYTHON_INLINE int __Pyx__MatchCase_Mapping_ExtractDict(void *__pyx_refnanny, PyObject *dict, PyObject *keys[], Py_ssize_t nKeys, PyObject **subjects[]); /* proto */ /////////////////////////// ExtractExactDict //////////////// -static CYTHON_INLINE int __Pyx__MatchCase_Mapping_ExtractDict(void *__pyx_refnanny, PyObject *dict, PyObject *fixed_keys, PyObject *var_keys, ...) { - int result; - va_list subjects; - - va_start(subjects, var_keys); - result = __Pyx_MatchCase_Mapping_ExtractDictV(dict, fixed_keys, var_keys, subjects); - va_end(subjects); - return result; -} +static CYTHON_INLINE int __Pyx__MatchCase_Mapping_ExtractDict(void *__pyx_refnanny, PyObject *dict, PyObject *keys[], Py_ssize_t nKeys, PyObject **subjects[]) { + Py_ssize_t i; -static int __Pyx__MatchCase_Mapping_ExtractDictV(void *__pyx_refnanny, PyObject *dict, PyObject *fixed_keys, PyObject *var_keys, va_list subjects) { - PyObject *keys[] = {fixed_keys, var_keys}; - Py_ssize_t i, j; - - for (i=0; i<2; ++i) { - PyObject *tuple = keys[i]; - for (j=0; j Date: Sat, 3 Sep 2022 15:16:37 +0100 Subject: Refactor MappingComparisonNode into two To make it clear who/what is responsible for generating the "keys" evaluation code --- Cython/Compiler/MatchCaseNodes.py | 88 ++++++++++++++++++++++++++------------- 1 file changed, 60 insertions(+), 28 deletions(-) diff --git a/Cython/Compiler/MatchCaseNodes.py b/Cython/Compiler/MatchCaseNodes.py index 4f00e480d..39c1323a6 100644 --- a/Cython/Compiler/MatchCaseNodes.py +++ b/Cython/Compiler/MatchCaseNodes.py @@ -1187,16 +1187,12 @@ class MatchMappingPatternNode(PatternNode): def get_comparison_node(self, subject_node, sequence_mapping_temp=None): from . import UtilNodes - keys_for_array = [] var_keys = [] n_literal_keys = 0 for k in self.keys: - if not k.arg.is_literal: - k = UtilNodes.ResultRefNode(k, is_temp=False) - keys_for_array.append(k) + if not k.is_literal: var_keys.append(k) else: - keys_for_array.append(k.arg.clone_node()) n_literal_keys += 1 all_tests = [] @@ -1234,16 +1230,13 @@ class MatchMappingPatternNode(PatternNode): body = UtilNodes.TempResultFromStatNode(test_result, body) else: body = all_tests - if keys_for_array or self.double_star_capture_target: + if self.keys or self.double_star_capture_target: body = MappingComparisonNode( body.pos, arg=LazyCoerceToBool(body.pos, arg=body), - keys_array=keys_for_array, + keys_array=self.keys, subjects_array=self.subject_temps ) - for k in var_keys: - if isinstance(k, UtilNodes.ResultRefNode): - body = UtilNodes.EvalWithTempExprNode(k, body) return LazyCoerceToBool(body.pos, arg=body) def analyse_pattern_expressions(self, env, sequence_mapping_temp): @@ -1254,7 +1247,7 @@ class MatchMappingPatternNode(PatternNode): return node.coerce_to_temp(env) self.keys = [ - ExprNodes.ProxyNode(to_temp_or_literal(k.analyse_expressions(env))) + to_temp_or_literal(k.analyse_expressions(env)) for k in self.keys ] @@ -1674,22 +1667,24 @@ def generate_binop_tree_from_list(pos, operator, list_of_tests): operand2=operand2 ) + class MappingComparisonNode(ExprNodes.ExprNode): """ - Sets up the arrays of subjects and keys + Combined with MappingComparisonNodeInner this is responsible + for setting up up the arrays of subjects and keys - has attributes: - * arg - the main comparison node - * keys_array - list of ExprNodes representing keys - * subjects_array - list of ExprNodes representing subjects + Note that self.keys_array is owned by this but used by + MappingComparisonNodeInner - that's mainly to ensure that + it gets evaluated in the correct order """ - subexprs = ['arg', 'keys_array'] + subexprs = ["keys_array", "inner"] + keys_array_cname = "__pyx_match_mapping_keys" subjects_array_cname = "__pyx_match_mapping_subjects" @property def type(self): - return self.arg.type + return self.inner.type @classmethod def make_keys_node(cls, pos): @@ -1707,6 +1702,47 @@ class MappingComparisonNode(ExprNodes.ExprNode): cname=cls.subjects_array_cname ) + def __init__(self, pos, arg, subjects_array, **kwds): + super(MappingComparisonNode, self).__init__(pos, **kwds) + self.inner = MappingComparisonNodeInner( + pos, + arg=arg, + keys_array = self.keys_array, + subjects_array = subjects_array + ) + + def analyse_types(self, env): + self.inner = self.inner.analyse_types(env) + self.keys_array = [ + key.analyse_types(env).coerce_to_simple(env) for key in self.keys_array + ] + return self + + def generate_result_code(self, code): + pass + + def calculate_result_code(self): + return self.inner.calculate_result_code() + + +class MappingComparisonNodeInner(ExprNodes.ExprNode): + """ + Sets up the arrays of subjects and keys + + Created by the constructor of MappingComparisonNode + (no need to create directly) + + has attributes: + * arg - the main comparison node + * keys_array - list of ExprNodes representing keys + * subjects_array - list of ExprNodes representing subjects + """ + subexprs = ['arg'] + + @property + def type(self): + return self.arg.type + def analyse_types(self, env): self.arg = self.arg.analyse_types(env) for n in range(len(self.keys_array)): @@ -1718,24 +1754,20 @@ class MappingComparisonNode(ExprNodes.ExprNode): def generate_evaluation_code(self, code): code.putln("{") - decl_insertion_point = code.insertion_point() - super(MappingComparisonNode, self).generate_evaluation_code(code) - - # defer evaluating k.result() until we've processed them in - # generate_subexpr_evaluation_code. This order is OK - the keys - # are either constants or ResultRefNodes keys_str = ", ".join(k.result() for k in self.keys_array) - decl_insertion_point.putln("PyObject *%s[] = {%s};" % ( - self.keys_array_cname, + code.putln("PyObject *%s[] = {%s};" % ( + MappingComparisonNode.keys_array_cname, keys_str, )) subjects_str = ", ".join( "&"+subject.result() if subject is not None else "NULL" for subject in self.subjects_array ) - decl_insertion_point.putln("PyObject **%s[] = {%s};" % ( - self.subjects_array_cname, + code.putln("PyObject **%s[] = {%s};" % ( + MappingComparisonNode.subjects_array_cname, subjects_str )) + super(MappingComparisonNodeInner, self).generate_evaluation_code(code) + code.putln("}") def generate_result_code(self, code): -- cgit v1.2.1 From 020ce410c3c90c165ea6c2a968265c9f54ee049d Mon Sep 17 00:00:00 2001 From: da-woods Date: Sat, 3 Sep 2022 16:51:45 +0100 Subject: Disable memview test on Py2 --- tests/run/extra_patma.pyx | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/run/extra_patma.pyx b/tests/run/extra_patma.pyx index f3f77cc27..0c114823b 100644 --- a/tests/run/extra_patma.pyx +++ b/tests/run/extra_patma.pyx @@ -5,6 +5,21 @@ cimport cython import array +import sys + +__doc__ = "" + +if sys.version_info[0] > 2: + __doc__ += """ + array.array doesn't have the buffer protocol in Py2 and + this doesn't really feel worth working around to test + >>> print(test_memoryview(array.array('i', [0, 1, 2]))) + a 1 + >>> print(test_memoryview(array.array('i', []))) + b + >>> print(test_memoryview(array.array('i', [5]))) + c [5] + """ # goes via .shape instead @cython.test_fail_if_path_exists("//CallNode//NameNode[@name = 'len']") @@ -12,14 +27,8 @@ import array @cython.test_fail_if_path_exists("//PythonCapiCallNode//PythonCapiFunctionNode[@cname = '__Pyx_MatchCase_IsSequence']") def test_memoryview(int[:] x): """ - >>> print(test_memoryview(array.array('i', [0, 1, 2]))) - a 1 - >>> print(test_memoryview(array.array('i', []))) - b >>> print(test_memoryview(None)) no! - >>> print(test_memoryview(array.array('i', [5]))) - c [5] """ match x: case [0, y, 2]: @@ -89,7 +98,7 @@ def class_attr_lookup(x): assert cython.typeof(y) == "double", cython.typeof(y) return y -class PyClass: +class PyClass(object): pass @cython.test_assert_path_exists("//PythonCapiFunctionNode[@cname='__Pyx_TypeCheck']") @@ -110,6 +119,7 @@ def class_typecheck_exists(x): case _: return False + @cython.test_fail_if_path_exists("//NameNode[@name='isinstance']") @cython.test_fail_if_path_exists("//PythonCapiFunctionNode[@cname='__Pyx_TypeCheck']") def class_typecheck_doesnt_exist(C x): -- cgit v1.2.1 From 3eb193be37f47a894737150c6833f707d729362d Mon Sep 17 00:00:00 2001 From: da-woods Date: Sat, 3 Sep 2022 16:51:45 +0100 Subject: Disable memview test on Py2 --- tests/run/extra_patma.pyx | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/run/extra_patma.pyx b/tests/run/extra_patma.pyx index 4482b2bf4..a712864f5 100644 --- a/tests/run/extra_patma.pyx +++ b/tests/run/extra_patma.pyx @@ -5,6 +5,21 @@ cimport cython import array +import sys + +__doc__ = "" + +if sys.version_info[0] > 2: + __doc__ += """ + array.array doesn't have the buffer protocol in Py2 and + this doesn't really feel worth working around to test + >>> print(test_memoryview(array.array('i', [0, 1, 2]))) + a 1 + >>> print(test_memoryview(array.array('i', []))) + b + >>> print(test_memoryview(array.array('i', [5]))) + c [5] + """ # goes via .shape instead @cython.test_fail_if_path_exists("//CallNode//NameNode[@name = 'len']") @@ -12,14 +27,8 @@ import array @cython.test_fail_if_path_exists("//PythonCapiCallNode//PythonCapiFunctionNode[@cname = '__Pyx_MatchCase_IsSequence']") def test_memoryview(int[:] x): """ - >>> print(test_memoryview(array.array('i', [0, 1, 2]))) - a 1 - >>> print(test_memoryview(array.array('i', []))) - b >>> print(test_memoryview(None)) no! - >>> print(test_memoryview(array.array('i', [5]))) - c [5] """ match x: case [0, y, 2]: @@ -71,3 +80,4 @@ def test_ctuple_to_sequence((int, int) x): case [a, b]: assert cython.typeof(a) == "int", cython.typeof(a) # test that types have inferred return a, b + -- cgit v1.2.1 From d6f585acdd5595b745465b8f479b741806cb4c82 Mon Sep 17 00:00:00 2001 From: da-woods Date: Sat, 3 Sep 2022 16:52:01 +0100 Subject: Simplify type checking Note that there's an error formatting issue on Python 2 unfortunately --- Cython/Utility/MatchCase.c | 27 ++++++++++++++++----------- tests/run/extra_patma_py.py | 22 +++++++++++++++++++++- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/Cython/Utility/MatchCase.c b/Cython/Utility/MatchCase.c index c7cc78900..2f4157143 100644 --- a/Cython/Utility/MatchCase.c +++ b/Cython/Utility/MatchCase.c @@ -786,19 +786,17 @@ static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subj _Py_TPFLAGS_MATCH_SELF); #else // probably an earlier version of Python. Go off the known list in the specification - match_self = (PyType_IsSubtype(type, &PyByteArray_Type) || - PyType_IsSubtype(type, &PyBytes_Type) || - PyType_IsSubtype(type, &PyDict_Type) || + match_self = ((PyType_GetFlags(type) & + // long should capture bool too + (Py_TPFLAGS_LONG_SUBCLASS | Py_TPFLAGS_LIST_SUBCLASS | Py_TPFLAGS_TUPLE_SUBCLASS | + Py_TPFLAGS_BYTES_SUBCLASS | Py_TPFLAGS_UNICODE_SUBCLASS | Py_TPFLAGS_DICT_SUBCLASS + #if PY_MAJOR_VERSION < 3 + | Py_TPFLAGS_IN_SUBCLASS + #endif + )) || + PyType_IsSubtype(type, &PyByteArray_Type) || PyType_IsSubtype(type, &PyFloat_Type) || PyType_IsSubtype(type, &PyFrozenSet_Type) || - PyType_IsSubtype(type, &PyLong_Type) || // This should capture bool too - #if PY_MAJOR_VERSION < 3 - PyType_IsSubtype(type, &PyInt_Type) || - #endif - PyType_IsSubtype(type, &PyList_Type) || - PyType_IsSubtype(type, &PySet_Type) || - PyType_IsSubtype(type, &PyUnicode_Type) || - PyType_IsSubtype(type, &PyTuple_Type) ); #endif } @@ -887,6 +885,13 @@ static PyTypeObject* __Pyx_MatchCase_IsType(PyObject* type); /* proto */ //////////////////////// MatchClassIsType ///////////////////////////// static PyTypeObject* __Pyx_MatchCase_IsType(PyObject* type) { + #if PY_MAJOR_VERSION < 3 + if (PyClass_Check(type)) { + // I don't really think it's worth the effort getting this to work! + PyErr_Format(PyExc_TypeError, "called match pattern must be a new-style class."); + return NULL; + } + #endif if (!PyType_Check(type)) { PyErr_Format(PyExc_TypeError, "called match pattern must be a type"); return NULL; diff --git a/tests/run/extra_patma_py.py b/tests/run/extra_patma_py.py index e06d41591..ded678ea7 100644 --- a/tests/run/extra_patma_py.py +++ b/tests/run/extra_patma_py.py @@ -2,6 +2,9 @@ # tag: pure3.10 import array +import sys + +__doc__ = "" def test_array_is_sequence(x): """ @@ -43,7 +46,7 @@ def test_duplicate_keys(key1, key2): return False -class PyClass: +class PyClass(object): pass @@ -63,3 +66,20 @@ class PrivateAttrLookupOuter: match x: case PyClass(__something=y): return y + + +if sys.version_info[0] < 3: + class OldStyleClass: + pass + + def test_oldstyle_class_failure(x): + match x: + case OldStyleClass(): + return True + + __doc__ += """ + >>> test_oldstyle_class_failure(1) + Traceback (most recent call last): + ... + TypeError: called match pattern must be a new-style class. + """ -- cgit v1.2.1 From e3882cf3885677a0dc372d46e288981ed0281869 Mon Sep 17 00:00:00 2001 From: da-woods Date: Sat, 3 Sep 2022 17:05:56 +0100 Subject: "Fix" Py2 error formatting issue --- Cython/Utility/MatchCase.c | 7 +++++++ tests/run/extra_patma_py.py | 14 ++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/Cython/Utility/MatchCase.c b/Cython/Utility/MatchCase.c index 3f71ed1e8..08eb50433 100644 --- a/Cython/Utility/MatchCase.c +++ b/Cython/Utility/MatchCase.c @@ -503,8 +503,15 @@ static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *keys[], Py_ssize_t nFixe return 0; raise_error: + #if PY_MAJOR_VERSION > 2 PyErr_Format(PyExc_ValueError, "mapping pattern checks duplicate key (%R)", key); + #else + // DW really can't be bothered working around features that don't exist in + // Python 2, so just provide less information! + PyErr_SetString(PyExc_ValueError, + "mapping pattern checks duplicate key"); + #endif bad: Py_DECREF(var_keys_set); return -1; diff --git a/tests/run/extra_patma_py.py b/tests/run/extra_patma_py.py index 0ff51e25a..182529683 100644 --- a/tests/run/extra_patma_py.py +++ b/tests/run/extra_patma_py.py @@ -2,6 +2,7 @@ # tag: pure3.10 import array +import sys def test_array_is_sequence(x): """ @@ -27,10 +28,15 @@ def test_duplicate_keys(key1, key2): >>> test_duplicate_keys("a", "b") True - >>> test_duplicate_keys("a", "a") - Traceback (most recent call last): - ... - ValueError: mapping pattern checks duplicate key ('a') + + Slightly awkward doctest to work around Py2 incompatibility + >>> try: + ... test_duplicate_keys("a", "a") + ... except ValueError as e: + ... if sys.version_info[0] > 2: + ... assert e.args[0] == "mapping pattern checks duplicate key ('a')", e.args[0] + ... else: + ... assert e.args[0] == "mapping pattern checks duplicate key" """ class Keys: KEY_1 = key1 -- cgit v1.2.1 From 132a80c75596361457353b5d80ebe4b08c158f0a Mon Sep 17 00:00:00 2001 From: da-woods Date: Sat, 3 Sep 2022 17:08:32 +0100 Subject: "Fix" Py2.7 issue --- Cython/Utility/MatchCase.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Cython/Utility/MatchCase.c b/Cython/Utility/MatchCase.c index cfa9e862c..6720d7383 100644 --- a/Cython/Utility/MatchCase.c +++ b/Cython/Utility/MatchCase.c @@ -762,8 +762,14 @@ static int __Pyx_MatchCase_ClassCheckDuplicateAttrs(const char *tp_name, PyObjec return 0; raise_error: + #if PY_MAJOR_VERSION > 2 PyErr_Format(PyExc_TypeError, "%s() got multiple sub-patterns for attribute %R", tp_name, attr); + #else + // DW has no interest in working around the lack of %R in Python 2.7 + PyErr_Format(PyExc_TypeError, "%s() got multiple sub-patterns for attribute", + tp_name); + #endif bad: Py_DECREF(attrs_set); return -1; -- cgit v1.2.1 From e7b0aed87599751a8199354a27d41322e76b65a8 Mon Sep 17 00:00:00 2001 From: da-woods Date: Sat, 3 Sep 2022 20:36:17 +0100 Subject: Remove unneeded includes and silence a compiler warning --- Cython/Compiler/MatchCaseNodes.py | 8 ++++++++ Cython/Utility/MatchCase.c | 6 ------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Cython/Compiler/MatchCaseNodes.py b/Cython/Compiler/MatchCaseNodes.py index cbfec32a2..d8833d2c8 100644 --- a/Cython/Compiler/MatchCaseNodes.py +++ b/Cython/Compiler/MatchCaseNodes.py @@ -2105,6 +2105,10 @@ class MappingComparisonNodeInner(ExprNodes.ExprNode): def generate_evaluation_code(self, code): code.putln("{") keys_str = ", ".join(k.result() for k in self.keys_array) + if not keys_str: + # GCC gets worried about overflow if we pass + # a genuinely empty array + keys_str = "NULL" code.putln("PyObject *%s[] = {%s};" % ( MappingComparisonNode.keys_array_cname, keys_str, @@ -2112,6 +2116,10 @@ class MappingComparisonNodeInner(ExprNodes.ExprNode): subjects_str = ", ".join( "&"+subject.result() if subject is not None else "NULL" for subject in self.subjects_array ) + if not subjects_str: + # GCC gets worried about overflow if we pass + # a genuinely empty array + subjects_str = "NULL" code.putln("PyObject **%s[] = {%s};" % ( MappingComparisonNode.subjects_array_cname, subjects_str diff --git a/Cython/Utility/MatchCase.c b/Cython/Utility/MatchCase.c index 6720d7383..7ea182f26 100644 --- a/Cython/Utility/MatchCase.c +++ b/Cython/Utility/MatchCase.c @@ -519,8 +519,6 @@ static int __Pyx_MatchCase_CheckMappingDuplicateKeys(PyObject *keys[], Py_ssize_ /////////////////////////// ExtractExactDict.proto //////////////// -#include - // the variadic arguments are a list of PyObject** to subjects to be filled. They may be NULL // in which case they're ignored. // @@ -565,8 +563,6 @@ static CYTHON_INLINE int __Pyx__MatchCase_Mapping_ExtractDict(void *__pyx_refnan // // This is a specialized version for the rarer case when the type isn't an exact dict. -#include - #if CYTHON_REFNANNY #define __Pyx_MatchCase_Mapping_ExtractNonDict(...) __Pyx__MatchCase_Mapping_ExtractNonDict(__pyx_refnanny, __VA_ARGS__) #else @@ -629,8 +625,6 @@ static int __Pyx__MatchCase_Mapping_ExtractNonDict(void *__pyx_refnanny, PyObjec ///////////////////////// ExtractGeneric.proto //////////////////////////////// -#include - #if CYTHON_REFNANNY #define __Pyx_MatchCase_Mapping_Extract(...) __Pyx__MatchCase_Mapping_Extract(__pyx_refnanny, __VA_ARGS__) #else -- cgit v1.2.1 From 8526c392ac8bcc3667eb6bdfb715b5bca51e910a Mon Sep 17 00:00:00 2001 From: da-woods Date: Sat, 3 Sep 2022 20:34:15 +0100 Subject: Use static arrays instead of tuples and varargs for classes --- Cython/Compiler/MatchCaseNodes.py | 105 +++++++++++++++++--------------------- Cython/Utility/MatchCase.c | 29 +++++------ 2 files changed, 58 insertions(+), 76 deletions(-) diff --git a/Cython/Compiler/MatchCaseNodes.py b/Cython/Compiler/MatchCaseNodes.py index d8833d2c8..823907283 100644 --- a/Cython/Compiler/MatchCaseNodes.py +++ b/Cython/Compiler/MatchCaseNodes.py @@ -1100,7 +1100,7 @@ class MatchMappingPatternNode(PatternNode): self.Pyx_mapping_check_duplicates_type, utility_code=utility_code, args=[ - MappingComparisonNode.make_keys_node(self.pos), + MappingOrClassComparisonNode.make_keys_node(self.pos), ExprNodes.IntNode(self.pos, value=str(n_fixed_keys)), ExprNodes.IntNode(self.pos, value=str(len(self.keys))) ], @@ -1140,12 +1140,12 @@ class MatchMappingPatternNode(PatternNode): utility_code=util_code, args=[ subject_node, - MappingComparisonNode.make_keys_node(self.pos), + MappingOrClassComparisonNode.make_keys_node(self.pos), ExprNodes.IntNode( self.pos, value=str(len(self.keys)) ), - MappingComparisonNode.make_subjects_node(self.pos), + MappingOrClassComparisonNode.make_subjects_node(self.pos), ], ) @@ -1168,7 +1168,7 @@ class MatchMappingPatternNode(PatternNode): utility_code=utility_code, args=[ subject_node, - MappingComparisonNode.make_keys_node(self.pos), + MappingOrClassComparisonNode.make_keys_node(self.pos), ExprNodes.IntNode(self.pos, value=str(len(self.keys))) ], ) @@ -1231,7 +1231,7 @@ class MatchMappingPatternNode(PatternNode): else: body = all_tests if self.keys or self.double_star_capture_target: - body = MappingComparisonNode( + body = MappingOrClassComparisonNode( body.pos, arg=LazyCoerceToBool(body.pos, arg=body), keys_array=self.keys, @@ -1298,16 +1298,19 @@ class ClassPatternNode(PatternNode): keyword_pattern_names = [] keyword_pattern_patterns = [] + # as with the mapping functions, lie a little about some of the types for + # ease of declaration Pyx_positional_type = PyrexTypes.CFuncType( PyrexTypes.c_bint_type, [ PyrexTypes.CFuncTypeArg("subject", PyrexTypes.py_object_type, None), PyrexTypes.CFuncTypeArg("type", Builtin.type_type, None), - PyrexTypes.CFuncTypeArg("keysnames_tuple", PyrexTypes.py_object_type, None), + PyrexTypes.CFuncTypeArg("fixed_names", PyrexTypes.c_void_ptr_type, None), + PyrexTypes.CFuncTypeArg("n_fixed", PyrexTypes.c_py_ssize_t_type, None), PyrexTypes.CFuncTypeArg("match_self", PyrexTypes.c_int_type, None), - PyrexTypes.CFuncTypeArg("num_args", PyrexTypes.c_int_type, None), + PyrexTypes.CFuncTypeArg("subjects", PyrexTypes.c_void_ptr_ptr_type, None), + PyrexTypes.CFuncTypeArg("n_subjects", PyrexTypes.c_int_type, None), ], - has_varargs=True, exception_value="-1", ) @@ -1477,13 +1480,10 @@ class ClassPatternNode(PatternNode): def make_positional_args_call(self, subject_node, class_node): assert self.positional_patterns util_code = UtilityCode.load_cached("ClassPositionalPatterns", "MatchCase.c") - keynames = ExprNodes.TupleNode( - self.pos, - args=[ - ExprNodes.StringNode(n.pos, value=n.name) - for n in self.keyword_pattern_names - ], - ) + keynames = [ + ExprNodes.StringNode(n.pos, value=n.name) + for n in self.keyword_pattern_names + ] # -1 is "unknown" match_self = ( -1 @@ -1516,21 +1516,28 @@ class ClassPatternNode(PatternNode): match_self = 0 # I think... Relies on knowing the bases match_self = ExprNodes.IntNode(self.pos, value=str(match_self)) - len_ = ExprNodes.IntNode(self.pos, value=str(len(self.positional_patterns))) - subject_derefs = [ - ExprNodes.NullNode(self.pos) - if t is None - else AddressOfPyObjectNode(self.pos, obj=t) - for t in self.positional_subject_temps - ] - return ExprNodes.PythonCapiCallNode( + n_subjects = ExprNodes.IntNode(self.pos, value=str(len(self.positional_patterns))) + return MappingOrClassComparisonNode( self.pos, - "__Pyx_MatchCase_ClassPositional", - self.Pyx_positional_type, - utility_code=util_code, - args=[subject_node, class_node, keynames, match_self, len_] - + subject_derefs, + arg=ExprNodes.PythonCapiCallNode( + self.pos, + "__Pyx_MatchCase_ClassPositional", + self.Pyx_positional_type, + utility_code=util_code, + args=[ + subject_node, + class_node, + MappingOrClassComparisonNode.make_keys_node(self.pos), + ExprNodes.IntNode(self.pos, value=str(len(keynames))), + match_self, + MappingOrClassComparisonNode.make_subjects_node(self.pos), + n_subjects, + ] + ), + subjects_array=self.positional_subject_temps, + keys_array=keynames, ) + return def make_subpattern_checks(self): patterns = self.keyword_pattern_patterns + self.positional_patterns @@ -1945,27 +1952,6 @@ class CompilerDirectivesExprNode(ExprNodes.ProxyNode): self.arg.annotate(code) -class AddressOfPyObjectNode(ExprNodes.ExprNode): - """ - obj - some temp node - """ - - type = PyrexTypes.c_void_ptr_ptr_type - is_temp = False - subexprs = ["obj"] - - def analyse_types(self, env): - self.obj = self.obj.analyse_types(env) - assert self.obj.type.is_pyobject, repr(self.obj.type) - return self - - def generate_result_code(self, code): - self.obj.generate_result_code(code) - - def calculate_result_code(self): - return "&%s" % self.obj.result() - - class LazyCoerceToPyObject(ExprNodes.ExprNode): """ Just calls "self.arg.coerce_to_pyobject" when it's analysed, @@ -2018,13 +2004,14 @@ def generate_binop_tree_from_list(pos, operator, list_of_tests): ) -class MappingComparisonNode(ExprNodes.ExprNode): +class MappingOrClassComparisonNode(ExprNodes.ExprNode): """ - Combined with MappingComparisonNodeInner this is responsible - for setting up up the arrays of subjects and keys + Combined with MappingOrClassComparisonNodeInner this is responsible + for setting up up the arrays of subjects and keys that are used in + the function calls that handle these types of patterns Note that self.keys_array is owned by this but used by - MappingComparisonNodeInner - that's mainly to ensure that + MappingOrClassComparisonNodeInner - that's mainly to ensure that it gets evaluated in the correct order """ subexprs = ["keys_array", "inner"] @@ -2053,8 +2040,8 @@ class MappingComparisonNode(ExprNodes.ExprNode): ) def __init__(self, pos, arg, subjects_array, **kwds): - super(MappingComparisonNode, self).__init__(pos, **kwds) - self.inner = MappingComparisonNodeInner( + super(MappingOrClassComparisonNode, self).__init__(pos, **kwds) + self.inner = MappingOrClassComparisonNodeInner( pos, arg=arg, keys_array = self.keys_array, @@ -2075,7 +2062,7 @@ class MappingComparisonNode(ExprNodes.ExprNode): return self.inner.calculate_result_code() -class MappingComparisonNodeInner(ExprNodes.ExprNode): +class MappingOrClassComparisonNodeInner(ExprNodes.ExprNode): """ Sets up the arrays of subjects and keys @@ -2110,7 +2097,7 @@ class MappingComparisonNodeInner(ExprNodes.ExprNode): # a genuinely empty array keys_str = "NULL" code.putln("PyObject *%s[] = {%s};" % ( - MappingComparisonNode.keys_array_cname, + MappingOrClassComparisonNode.keys_array_cname, keys_str, )) subjects_str = ", ".join( @@ -2121,10 +2108,10 @@ class MappingComparisonNodeInner(ExprNodes.ExprNode): # a genuinely empty array subjects_str = "NULL" code.putln("PyObject **%s[] = {%s};" % ( - MappingComparisonNode.subjects_array_cname, + MappingOrClassComparisonNode.subjects_array_cname, subjects_str )) - super(MappingComparisonNodeInner, self).generate_evaluation_code(code) + super(MappingOrClassComparisonNodeInner, self).generate_evaluation_code(code) code.putln("}") diff --git a/Cython/Utility/MatchCase.c b/Cython/Utility/MatchCase.c index 7ea182f26..efdc4957e 100644 --- a/Cython/Utility/MatchCase.c +++ b/Cython/Utility/MatchCase.c @@ -703,18 +703,16 @@ static PyObject* __Pyx_MatchCase_DoubleStarCapture{{tag}}(PyObject *mapping, PyO ////////////////////////////// ClassPositionalPatterns.proto //////////////////////// -#include - #if CYTHON_REFNANNY #define __Pyx_MatchCase_ClassPositional(...) __Pyx__MatchCase_ClassPositional(__pyx_refnanny, __VA_ARGS__) #else #define __Pyx_MatchCase_ClassPositional(...) __Pyx__MatchCase_ClassPositional(NULL, __VA_ARGS__) #endif -static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subject, PyTypeObject *type, PyObject *keysnames_tuple, int match_self, int num_args, ...); /* proto */ +static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subject, PyTypeObject *type, PyObject *fixed_names[], Py_ssize_t n_fixed, int match_self, PyObject **subjects[], Py_ssize_t n_subjects); /* proto */ /////////////////////////////// ClassPositionalPatterns ////////////////////////////// -static int __Pyx_MatchCase_ClassCheckDuplicateAttrs(const char *tp_name, PyObject *fixed_names_tuple, PyObject *match_args, Py_ssize_t num_args) { +static int __Pyx_MatchCase_ClassCheckDuplicateAttrs(const char *tp_name, PyObject *fixed_names[], Py_ssize_t n_fixed, PyObject *match_args, Py_ssize_t num_args) { // a lot of the basic logic of this is shared with __Pyx_MatchCase_CheckMappingDuplicateKeys // but they take different input types so it isn't easy to actually share the code. @@ -743,8 +741,8 @@ static int __Pyx_MatchCase_ClassCheckDuplicateAttrs(const char *tp_name, PyObjec } } } - for (n=0; n < PyTuple_GET_SIZE(fixed_names_tuple); ++n) { - attr = PyTuple_GET_ITEM(fixed_names_tuple, n); + for (n=0; n < n_fixed; ++n) { + attr = fixed_names[n]; contains = PySet_Contains(attrs_set, attr); if (contains < 0) { goto bad; @@ -775,12 +773,11 @@ static int __Pyx_MatchCase_ClassCheckDuplicateAttrs(const char *tp_name, PyObjec // 0 for "known to be false" // -1 for "unknown", runtime test // nargs is >= 0 otherwise this function will be skipped -static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subject, PyTypeObject *type, PyObject *keysnames_tuple, int match_self, int num_args, ...) +static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subject, PyTypeObject *type, PyObject *fixed_names[], Py_ssize_t n_fixed, int match_self, PyObject **subjects[], Py_ssize_t n_subjects) { - PyObject *match_args, *dup_key; + PyObject *match_args; Py_ssize_t allowed, i; int result; - va_list subjects; match_args = PyObject_GetAttrString((PyObject*)type, "__match_args__"); if (!match_args) { @@ -824,18 +821,17 @@ static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subj allowed = match_self ? 1 : (match_args ? PyTuple_GET_SIZE(match_args) : 0); - if (allowed < num_args) { + if (allowed < n_subjects) { const char *plural = (allowed == 1) ? "" : "s"; PyErr_Format(PyExc_TypeError, "%s() accepts %d positional sub-pattern%s (%d given)", type->tp_name, - allowed, plural, num_args); + allowed, plural, n_subjects); Py_XDECREF(match_args); return -1; } - va_start(subjects, num_args); if (match_self) { - PyObject **self_subject = va_arg(subjects, PyObject**); + PyObject **self_subject = subjects[0]; if (self_subject) { // Easy. Copy the subject itself, and move on to kwargs. __Pyx_XDECREF_SET(*self_subject, subject); @@ -845,12 +841,12 @@ static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subj goto end_match_self; } // next stage is to check for duplicate attributes. - if (__Pyx_MatchCase_ClassCheckDuplicateAttrs(type->tp_name, keysnames_tuple, match_args, num_args)) { + if (__Pyx_MatchCase_ClassCheckDuplicateAttrs(type->tp_name, fixed_names, n_fixed, match_args, n_subjects)) { result = -1; goto end; } - for (i = 0; i < num_args; i++) { + for (i = 0; i < n_subjects; i++) { PyObject *attr; PyObject **subject_i; PyObject *name = PyTuple_GET_ITEM(match_args, i); @@ -868,7 +864,7 @@ static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subj result = 0; goto end; } - subject_i = va_arg(subjects, PyObject**); + subject_i = subjects[i]; if (subject_i) { __Pyx_XDECREF_SET(*subject_i, attr); __Pyx_GOTREF(attr); @@ -881,7 +877,6 @@ static int __Pyx__MatchCase_ClassPositional(void *__pyx_refnanny, PyObject *subj end: Py_DECREF(match_args); end_match_self: // because match_args isn't set - va_end(subjects); return result; } -- cgit v1.2.1 From 4ef0ba28862543b54e0986303dfce6643e893cf2 Mon Sep 17 00:00:00 2001 From: da-woods Date: Sat, 3 Sep 2022 20:36:17 +0100 Subject: Remove unneeded includes and silence a compiler warning --- Cython/Compiler/MatchCaseNodes.py | 8 ++++++++ Cython/Utility/MatchCase.c | 6 ------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Cython/Compiler/MatchCaseNodes.py b/Cython/Compiler/MatchCaseNodes.py index 39c1323a6..e704a331a 100644 --- a/Cython/Compiler/MatchCaseNodes.py +++ b/Cython/Compiler/MatchCaseNodes.py @@ -1755,6 +1755,10 @@ class MappingComparisonNodeInner(ExprNodes.ExprNode): def generate_evaluation_code(self, code): code.putln("{") keys_str = ", ".join(k.result() for k in self.keys_array) + if not keys_str: + # GCC gets worried about overflow if we pass + # a genuinely empty array + keys_str = "NULL" code.putln("PyObject *%s[] = {%s};" % ( MappingComparisonNode.keys_array_cname, keys_str, @@ -1762,6 +1766,10 @@ class MappingComparisonNodeInner(ExprNodes.ExprNode): subjects_str = ", ".join( "&"+subject.result() if subject is not None else "NULL" for subject in self.subjects_array ) + if not subjects_str: + # GCC gets worried about overflow if we pass + # a genuinely empty array + subjects_str = "NULL" code.putln("PyObject **%s[] = {%s};" % ( MappingComparisonNode.subjects_array_cname, subjects_str diff --git a/Cython/Utility/MatchCase.c b/Cython/Utility/MatchCase.c index 08eb50433..fd4139032 100644 --- a/Cython/Utility/MatchCase.c +++ b/Cython/Utility/MatchCase.c @@ -519,8 +519,6 @@ static int __Pyx_MatchCase_CheckDuplicateKeys(PyObject *keys[], Py_ssize_t nFixe /////////////////////////// ExtractExactDict.proto //////////////// -#include - // the variadic arguments are a list of PyObject** to subjects to be filled. They may be NULL // in which case they're ignored. // @@ -565,8 +563,6 @@ static CYTHON_INLINE int __Pyx__MatchCase_Mapping_ExtractDict(void *__pyx_refnan // // This is a specialized version for the rarer case when the type isn't an exact dict. -#include - #if CYTHON_REFNANNY #define __Pyx_MatchCase_Mapping_ExtractNonDict(...) __Pyx__MatchCase_Mapping_ExtractNonDict(__pyx_refnanny, __VA_ARGS__) #else @@ -629,8 +625,6 @@ static int __Pyx__MatchCase_Mapping_ExtractNonDict(void *__pyx_refnanny, PyObjec ///////////////////////// ExtractGeneric.proto //////////////////////////////// -#include - #if CYTHON_REFNANNY #define __Pyx_MatchCase_Mapping_Extract(...) __Pyx__MatchCase_Mapping_Extract(__pyx_refnanny, __VA_ARGS__) #else -- cgit v1.2.1 From 4d772106395f40ed83ed4bc8f6ff59a43a880756 Mon Sep 17 00:00:00 2001 From: da-woods Date: Fri, 16 Sep 2022 08:17:21 +0100 Subject: Simplify some type checking --- Cython/Utility/MatchCase.c | 89 +++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 52 deletions(-) diff --git a/Cython/Utility/MatchCase.c b/Cython/Utility/MatchCase.c index 6f9b83360..cea40d8b2 100644 --- a/Cython/Utility/MatchCase.c +++ b/Cython/Utility/MatchCase.c @@ -1,28 +1,14 @@ ///////////////////////////// ABCCheck ////////////////////////////// #if PY_VERSION_HEX < 0x030A0000 -static int __Pyx_MatchCase_IsExactSequence(PyObject *o) { +static CYTHON_INLINE int __Pyx_MatchCase_IsExactSequence(PyObject *o) { // is one of the small list of builtin types known to be a sequence - if (PyList_CheckExact(o) || PyTuple_CheckExact(o)) { + if (PyList_CheckExact(o) || PyTuple_CheckExact(o) || + PyType_CheckExact(o, PyRange_Type) || PyType_CheckExact(o, PyMemoryView_Type)) { // Use exact type match for these checks. I in the event of inheritence we need to make sure // that it isn't a mapping too return 1; } - if (PyRange_Check(o) || PyMemoryView_Check(o)) { - // Exact check isn't possible so do exact check in another way - PyObject *mro = PyObject_GetAttrString((PyObject*)Py_TYPE(o), "__mro__"); - if (mro) { - Py_ssize_t len = PyObject_Length(mro); - Py_DECREF(mro); - if (len < 0) { - PyErr_Clear(); // doesn't really matter, just proceed with other checks - } else if (len == 2) { - return 1; // the type and "object" and no other bases - } - } else { - PyErr_Clear(); // doesn't really matter, just proceed with other checks - } - } return 0; } @@ -34,10 +20,13 @@ static CYTHON_INLINE int __Pyx_MatchCase_IsExactMapping(PyObject *o) { } static int __Pyx_MatchCase_IsExactNeitherSequenceNorMapping(PyObject *o) { - if (PyUnicode_Check(o) || PyBytes_Check(o) || PyByteArray_Check(o)) { + if (PyType_GetFlags(Py_TYPE(o)) & (Py_TPFLAGS_BYTES_SUBCLASS | Py_TPFLAGS_UNICODE_SUBCLASS)) || + PyByteArray_Check(o)) { return 1; // these types are deliberately excluded from the sequence test // even though they look like sequences for most other purposes. - // They're therefore "inexact" checks + // Leave them as inexact checks since they do pass + // "isinstance(o, collections.abc.Sequence)" so it's very hard to + // reason about their subclasses } if (o == Py_None || PyLong_CheckExact(o) || PyFloat_CheckExact(o)) { return 1; @@ -73,6 +62,16 @@ static int __Pyx_MatchCase_IsExactNeitherSequenceNorMapping(PyObject *o) { #define __PYX_SEQUENCE_MAPPING_ERROR (1U<<4) // only used by the ABCCheck function #endif +static int __Pyx_MatchCase_InitAndIsInstanceAbc(PyObject *o, PyObject *abc_module, + PyObject **abc_type, PyObject *name) { + assert(!abc_type); + abc_type = PyObject_GetAttr(abc_module, name); + if (!abc_type) { + return -1; + } + return PyObject_IsInstance(o, abc_type); +} + // the result is defined using the specification for sequence_mapping_temp // (detailed in "is_sequence") static unsigned int __Pyx_MatchCase_ABCCheck(PyObject *o, int sequence_first, int definitely_not_sequence, int definitely_not_mapping) { @@ -101,12 +100,7 @@ static unsigned int __Pyx_MatchCase_ABCCheck(PyObject *o, int sequence_first, in result = __PYX_DEFINITELY_SEQUENCE_FLAG; goto end; } - sequence_type = PyObject_GetAttr(abc_module, PYIDENT("Sequence")); - if (!sequence_type) { - result = __PYX_SEQUENCE_MAPPING_ERROR; - goto end; - } - sequence_result = PyObject_IsInstance(o, sequence_type); + sequence_result = __Pyx_MatchCase_InitAndIsInstanceAbc(o, abc_module, &sequence_type, PYIDENT("Sequence")); if (sequence_result < 0) { result = __PYX_SEQUENCE_MAPPING_ERROR; goto end; @@ -114,41 +108,32 @@ static unsigned int __Pyx_MatchCase_ABCCheck(PyObject *o, int sequence_first, in result |= __PYX_DEFINITELY_NOT_SEQUENCE_FLAG; goto end; } - // else wait to see what mapping is + // else wait to see what mapping is } if (!definitely_not_mapping) { - mapping_type = PyObject_GetAttr(abc_module, PYIDENT("Mapping")); - if (!mapping_type) { + mapping_result = __Pyx_MatchCase_InitAndIsInstanceAbc(o, abc_module, &mapping_type, PYIDENT("Mapping")); + if (mapping_result < 0) { + result = __PYX_SEQUENCE_MAPPING_ERROR; goto end; - } - mapping_result = PyObject_IsInstance(o, mapping_type); - } - if (mapping_result < 0) { - result = __PYX_SEQUENCE_MAPPING_ERROR; - goto end; - } else if (mapping_result == 0) { - result |= __PYX_DEFINITELY_NOT_MAPPING_FLAG; - if (sequence_first) { - assert(sequence_result); - result |= __PYX_DEFINITELY_SEQUENCE_FLAG; - } - goto end; - } else /* mapping_result == 1 */ { - if (sequence_first && !sequence_result) { - result |= __PYX_DEFINITELY_MAPPING_FLAG; + } else if (mapping_result == 0) { + result |= __PYX_DEFINITELY_NOT_MAPPING_FLAG; + if (sequence_first) { + assert(sequence_result); + result |= __PYX_DEFINITELY_SEQUENCE_FLAG; + } goto end; + } else /* mapping_result == 1 */ { + if (sequence_first && !sequence_result) { + result |= __PYX_DEFINITELY_MAPPING_FLAG; + goto end; + } } } if (!sequence_first) { // here we know mapping_result is true because we'd have returned otherwise assert(mapping_result); if (!definitely_not_sequence) { - sequence_type = PyObject_GetAttr(abc_module, PYIDENT("Sequence")); - if (!sequence_type) { - result = __PYX_SEQUENCE_MAPPING_ERROR; - goto end; - } - sequence_result = PyObject_IsInstance(o, sequence_type); + sequence_result = __Pyx_MatchCase_InitAndIsInstanceAbc(o, abc_module, &sequence_type, PYIDENT("Sequence")); } if (sequence_result < 0) { result = __PYX_SEQUENCE_MAPPING_ERROR; @@ -167,7 +152,7 @@ static unsigned int __Pyx_MatchCase_ABCCheck(PyObject *o, int sequence_first, in if (!mro) { PyErr_Clear(); goto end; - } + } if (!PyTuple_Check(mro)) { Py_DECREF(mro); goto end; @@ -322,7 +307,7 @@ static PyObject *__Pyx_MatchCase_OtherSequenceSliceToList(PyObject *x, Py_ssize_ PyObject *list; ssizeargfunc slot; PyTypeObject *type = Py_TYPE(x); - + list = PyList_New(total); if (!list) { return NULL; -- cgit v1.2.1