Merge pull request #57151 from cdemirer/fix-match-array-dict-pattern-logic-error

Fix logic errors in match-statement Array & Dictionary patterns

modules/gdscript/gdscript_compiler.cpp

@@ -1389,6 +1389,44 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c
 			codegen.generator->pop_temporary();
 			codegen.generator->pop_temporary();
 
+			// Create temporaries outside the loop so they can be reused.
+			GDScriptCodeGenerator::Address element_addr = codegen.add_temporary();
+			GDScriptCodeGenerator::Address element_type_addr = codegen.add_temporary();
+
+			// Evaluate element by element.
+			for (int i = 0; i < p_pattern->array.size(); i++) {
+				if (p_pattern->array[i]->pattern_type == GDScriptParser::PatternNode::PT_REST) {
+					// Don't want to access an extra element of the user array.
+					break;
+				}
+
+				// Use AND here too, as we don't want to be checking elements if previous test failed (which means this might be an invalid get).
+				codegen.generator->write_and_left_operand(result_addr);
+
+				// Add index to constant map.
+				GDScriptCodeGenerator::Address index_addr = codegen.add_constant(i);
+
+				// Get the actual element from the user-sent array.
+				codegen.generator->write_get(element_addr, index_addr, p_value_addr);
+
+				// Also get type of element.
+				Vector<GDScriptCodeGenerator::Address> typeof_args;
+				typeof_args.push_back(element_addr);
+				codegen.generator->write_call_utility(element_type_addr, "typeof", typeof_args);
+
+				// Try the pattern inside the element.
+				result_addr = _parse_match_pattern(codegen, r_error, p_pattern->array[i], element_addr, element_type_addr, result_addr, false, true);
+				if (r_error != OK) {
+					return GDScriptCodeGenerator::Address();
+				}
+
+				codegen.generator->write_and_right_operand(result_addr);
+				codegen.generator->write_end_and(result_addr);
+			}
+			// Remove element temporaries.
+			codegen.generator->pop_temporary();
+			codegen.generator->pop_temporary();
+
 			// If this isn't the first, we need to OR with the previous pattern. If it's nested, we use AND instead.
 			if (p_is_nested) {
 				// Use the previous value as target, since we only need one temporary variable.
@@ -1404,46 +1442,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c
 			}
 			codegen.generator->pop_temporary(); // Remove temp result addr.
 
-			// Create temporaries outside the loop so they can be reused.
+			return p_previous_test;
-			GDScriptCodeGenerator::Address element_addr = codegen.add_temporary();
-			GDScriptCodeGenerator::Address element_type_addr = codegen.add_temporary();
-			GDScriptCodeGenerator::Address test_addr = p_previous_test;
-
-			// Evaluate element by element.
-			for (int i = 0; i < p_pattern->array.size(); i++) {
-				if (p_pattern->array[i]->pattern_type == GDScriptParser::PatternNode::PT_REST) {
-					// Don't want to access an extra element of the user array.
-					break;
-				}
-
-				// Use AND here too, as we don't want to be checking elements if previous test failed (which means this might be an invalid get).
-				codegen.generator->write_and_left_operand(test_addr);
-
-				// Add index to constant map.
-				GDScriptCodeGenerator::Address index_addr = codegen.add_constant(i);
-
-				// Get the actual element from the user-sent array.
-				codegen.generator->write_get(element_addr, index_addr, p_value_addr);
-
-				// Also get type of element.
-				Vector<GDScriptCodeGenerator::Address> typeof_args;
-				typeof_args.push_back(element_addr);
-				codegen.generator->write_call_utility(element_type_addr, "typeof", typeof_args);
-
-				// Try the pattern inside the element.
-				test_addr = _parse_match_pattern(codegen, r_error, p_pattern->array[i], element_addr, element_type_addr, p_previous_test, false, true);
-				if (r_error != OK) {
-					return GDScriptCodeGenerator::Address();
-				}
-
-				codegen.generator->write_and_right_operand(test_addr);
-				codegen.generator->write_end_and(test_addr);
-			}
-			// Remove element temporaries.
-			codegen.generator->pop_temporary();
-			codegen.generator->pop_temporary();
-
-			return test_addr;
 		} break;
 		case GDScriptParser::PatternNode::PT_DICTIONARY: {
 			if (p_is_nested) {
@@ -1488,6 +1487,66 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c
 			codegen.generator->pop_temporary();
 			codegen.generator->pop_temporary();
 
+			// Create temporaries outside the loop so they can be reused.
+			GDScriptCodeGenerator::Address element_addr = codegen.add_temporary();
+			GDScriptCodeGenerator::Address element_type_addr = codegen.add_temporary();
+
+			// Evaluate element by element.
+			for (int i = 0; i < p_pattern->dictionary.size(); i++) {
+				const GDScriptParser::PatternNode::Pair &element = p_pattern->dictionary[i];
+				if (element.value_pattern && element.value_pattern->pattern_type == GDScriptParser::PatternNode::PT_REST) {
+					// Ignore rest pattern.
+					break;
+				}
+
+				// Use AND here too, as we don't want to be checking elements if previous test failed (which means this might be an invalid get).
+				codegen.generator->write_and_left_operand(result_addr);
+
+				// Get the pattern key.
+				GDScriptCodeGenerator::Address pattern_key_addr = _parse_expression(codegen, r_error, element.key);
+				if (r_error) {
+					return GDScriptCodeGenerator::Address();
+				}
+
+				// Check if pattern key exists in user's dictionary. This will be AND-ed with next result.
+				func_args.clear();
+				func_args.push_back(pattern_key_addr);
+				codegen.generator->write_call(result_addr, p_value_addr, "has", func_args);
+
+				if (element.value_pattern != nullptr) {
+					// Use AND here too, as we don't want to be checking elements if previous test failed (which means this might be an invalid get).
+					codegen.generator->write_and_left_operand(result_addr);
+
+					// Get actual value from user dictionary.
+					codegen.generator->write_get(element_addr, pattern_key_addr, p_value_addr);
+
+					// Also get type of value.
+					func_args.clear();
+					func_args.push_back(element_addr);
+					codegen.generator->write_call_utility(element_type_addr, "typeof", func_args);
+
+					// Try the pattern inside the value.
+					result_addr = _parse_match_pattern(codegen, r_error, element.value_pattern, element_addr, element_type_addr, result_addr, false, true);
+					if (r_error != OK) {
+						return GDScriptCodeGenerator::Address();
+					}
+					codegen.generator->write_and_right_operand(result_addr);
+					codegen.generator->write_end_and(result_addr);
+				}
+
+				codegen.generator->write_and_right_operand(result_addr);
+				codegen.generator->write_end_and(result_addr);
+
+				// Remove pattern key temporary.
+				if (pattern_key_addr.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
+					codegen.generator->pop_temporary();
+				}
+			}
+
+			// Remove element temporaries.
+			codegen.generator->pop_temporary();
+			codegen.generator->pop_temporary();
+
 			// If this isn't the first, we need to OR with the previous pattern. If it's nested, we use AND instead.
 			if (p_is_nested) {
 				// Use the previous value as target, since we only need one temporary variable.
@@ -1503,71 +1562,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c
 			}
 			codegen.generator->pop_temporary(); // Remove temp result addr.
 
-			// Create temporaries outside the loop so they can be reused.
+			return p_previous_test;
-			temp_type.builtin_type = Variant::BOOL;
-			GDScriptCodeGenerator::Address test_result = codegen.add_temporary(temp_type);
-			GDScriptCodeGenerator::Address element_addr = codegen.add_temporary();
-			GDScriptCodeGenerator::Address element_type_addr = codegen.add_temporary();
-			GDScriptCodeGenerator::Address test_addr = p_previous_test;
-
-			// Evaluate element by element.
-			for (int i = 0; i < p_pattern->dictionary.size(); i++) {
-				const GDScriptParser::PatternNode::Pair &element = p_pattern->dictionary[i];
-				if (element.value_pattern && element.value_pattern->pattern_type == GDScriptParser::PatternNode::PT_REST) {
-					// Ignore rest pattern.
-					break;
-				}
-
-				// Use AND here too, as we don't want to be checking elements if previous test failed (which means this might be an invalid get).
-				codegen.generator->write_and_left_operand(test_addr);
-
-				// Get the pattern key.
-				GDScriptCodeGenerator::Address pattern_key_addr = _parse_expression(codegen, r_error, element.key);
-				if (r_error) {
-					return GDScriptCodeGenerator::Address();
-				}
-
-				// Check if pattern key exists in user's dictionary. This will be AND-ed with next result.
-				func_args.clear();
-				func_args.push_back(pattern_key_addr);
-				codegen.generator->write_call(test_result, p_value_addr, "has", func_args);
-
-				if (element.value_pattern != nullptr) {
-					// Use AND here too, as we don't want to be checking elements if previous test failed (which means this might be an invalid get).
-					codegen.generator->write_and_left_operand(test_result);
-
-					// Get actual value from user dictionary.
-					codegen.generator->write_get(element_addr, pattern_key_addr, p_value_addr);
-
-					// Also get type of value.
-					func_args.clear();
-					func_args.push_back(element_addr);
-					codegen.generator->write_call_utility(element_type_addr, "typeof", func_args);
-
-					// Try the pattern inside the value.
-					test_addr = _parse_match_pattern(codegen, r_error, element.value_pattern, element_addr, element_type_addr, test_addr, false, true);
-					if (r_error != OK) {
-						return GDScriptCodeGenerator::Address();
-					}
-					codegen.generator->write_and_right_operand(test_addr);
-					codegen.generator->write_end_and(test_addr);
-				}
-
-				codegen.generator->write_and_right_operand(test_addr);
-				codegen.generator->write_end_and(test_addr);
-
-				// Remove pattern key temporary.
-				if (pattern_key_addr.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
-					codegen.generator->pop_temporary();
-				}
-			}
-
-			// Remove element temporaries.
-			codegen.generator->pop_temporary();
-			codegen.generator->pop_temporary();
-			codegen.generator->pop_temporary();
-
-			return test_addr;
 		} break;
 		case GDScriptParser::PatternNode::PT_REST:
 			// Do nothing.

modules/gdscript/tests/scripts/parser/features/match_dictionary.gd

@@ -0,0 +1,43 @@
+func foo(x):
+    match x:
+        {"key1": "value1", "key2": "value2"}:
+            print('{"key1": "value1", "key2": "value2"}')
+        {"key1": "value1", "key2"}:
+            print('{"key1": "value1", "key2"}')
+        {"key1", "key2": "value2"}:
+            print('{"key1", "key2": "value2"}')
+        {"key1", "key2"}:
+            print('{"key1", "key2"}')
+        {"key1": "value1"}:
+            print('{"key1": "value1"}')
+        {"key1"}:
+            print('{"key1"}')
+        _:
+            print("wildcard")
+
+func bar(x):
+    match x:
+        {0}:
+            print("0")
+        {1}:
+            print("1")
+        {2}:
+            print("2")
+        _:
+            print("wildcard")
+
+func test():
+    foo({"key1": "value1", "key2": "value2"})
+    foo({"key1": "value1", "key2": ""})
+    foo({"key1": "", "key2": "value2"})
+    foo({"key1": "", "key2": ""})
+    foo({"key1": "value1"})
+    foo({"key1": ""})
+    foo({"key1": "value1", "key2": "value2", "key3": "value3"})
+    foo({"key1": "value1", "key3": ""})
+    foo({"key2": "value2"})
+    foo({"key3": ""})
+    bar({0: "0"})
+    bar({1: "1"})
+    bar({2: "2"})
+    bar({3: "3"})

modules/gdscript/tests/scripts/parser/features/match_dictionary.out

@@ -0,0 +1,15 @@
+GDTEST_OK
+{"key1": "value1", "key2": "value2"}
+{"key1": "value1", "key2"}
+{"key1", "key2": "value2"}
+{"key1", "key2"}
+{"key1": "value1"}
+{"key1"}
+wildcard
+wildcard
+wildcard
+wildcard
+0
+1
+2
+wildcard

modules/gdscript/tests/scripts/parser/features/match_multiple_patterns_with_array.gd

@@ -0,0 +1,26 @@
+func foo(x):
+    match x:
+        1, [2]:
+            print('1, [2]')
+        _:
+            print('wildcard')
+
+func bar(x):
+    match x:
+        [1], [2], [3]:
+            print('[1], [2], [3]')
+        [4]:
+            print('[4]')
+        _:
+            print('wildcard')
+
+func test():
+    foo(1)
+    foo([2])
+    foo(2)
+    bar([1])
+    bar([2])
+    bar([3])
+    bar([4])
+    bar([5])
+

modules/gdscript/tests/scripts/parser/features/match_multiple_patterns_with_array.out

@@ -0,0 +1,9 @@
+GDTEST_OK
+1, [2]
+1, [2]
+wildcard
+[1], [2], [3]
+[1], [2], [3]
+[1], [2], [3]
+[4]
+wildcard