From 54a1414500ee2f8f87647fc0ffe921498332446f Mon Sep 17 00:00:00 2001 From: George Marques Date: Mon, 31 Jul 2023 07:47:26 -0300 Subject: [PATCH] GDScript: Implement pattern guards for match statement Within a match statement, it is now possible to add guards in each branch: var a = 0 match a: 0 when false: print("does not run") 0 when true: print("but this does") This allows more complex logic for deciding which branch to take. --- modules/gdscript/gdscript.cpp | 2 + modules/gdscript/gdscript_analyzer.cpp | 4 ++ modules/gdscript/gdscript_compiler.cpp | 20 ++++++ modules/gdscript/gdscript_parser.cpp | 33 ++++++++- modules/gdscript/gdscript_parser.h | 1 + modules/gdscript/gdscript_tokenizer.cpp | 4 ++ modules/gdscript/gdscript_tokenizer.h | 1 + .../errors/match_guard_invalid_expression.gd | 4 ++ .../errors/match_guard_invalid_expression.out | 2 + .../errors/match_guard_with_assignment.gd | 5 ++ .../errors/match_guard_with_assignment.out | 2 + .../allowed_keywords_as_identifiers.gd | 4 ++ .../allowed_keywords_as_identifiers.out | 1 + .../features/match_with_pattern_guards.gd | 71 +++++++++++++++++++ .../features/match_with_pattern_guards.out | 10 +++ 15 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/match_guard_invalid_expression.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/match_guard_invalid_expression.out create mode 100644 modules/gdscript/tests/scripts/parser/errors/match_guard_with_assignment.gd create mode 100644 modules/gdscript/tests/scripts/parser/errors/match_guard_with_assignment.out create mode 100644 modules/gdscript/tests/scripts/runtime/features/match_with_pattern_guards.gd create mode 100644 modules/gdscript/tests/scripts/runtime/features/match_with_pattern_guards.out diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 22be26fdc115..4a7cd672bcc9 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -2415,6 +2415,7 @@ void GDScriptLanguage::get_reserved_words(List *p_words) const { "return", "match", "while", + "when", // These keywords are not implemented currently, but reserved for (potential) future use. // We highlight them as keywords to make errors easier to understand. "trait", @@ -2448,6 +2449,7 @@ bool GDScriptLanguage::is_control_flow_keyword(String p_keyword) const { p_keyword == "match" || p_keyword == "pass" || p_keyword == "return" || + p_keyword == "when" || p_keyword == "while"; } diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 55bb99133a2c..aac42b2c1fc1 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -2219,6 +2219,10 @@ void GDScriptAnalyzer::resolve_match_branch(GDScriptParser::MatchBranchNode *p_m resolve_match_pattern(p_match_branch->patterns[i], p_match_test); } + if (p_match_branch->guard_body) { + resolve_suite(p_match_branch->guard_body); + } + resolve_suite(p_match_branch->block); decide_suite_type(p_match_branch, p_match_branch->block); diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 97e02ac71600..f9bd0fd709b6 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -1928,6 +1928,26 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui } } + // If there's a guard, check its condition too. + if (branch->guard_body != nullptr) { + // Do this first so the guard does not run unless the pattern matched. + gen->write_and_left_operand(pattern_result); + + // Don't actually use the block for the guard. + // The binds are already in the locals and we don't want to clear the result of the guard condition before we check the actual match. + GDScriptCodeGenerator::Address guard_result = _parse_expression(codegen, err, static_cast(branch->guard_body->statements[0])); + if (err) { + return err; + } + + gen->write_and_right_operand(guard_result); + gen->write_end_and(pattern_result); + + if (guard_result.mode == GDScriptCodeGenerator::Address::TEMPORARY) { + codegen.generator->pop_temporary(); + } + } + // Check if pattern did match. gen->write_if(pattern_result); diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 0801582dbd8d..02c1acfb67d5 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -2035,7 +2035,37 @@ GDScriptParser::MatchBranchNode *GDScriptParser::parse_match_branch() { push_error(R"(No pattern found for "match" branch.)"); } - if (!consume(GDScriptTokenizer::Token::COLON, R"(Expected ":" after "match" patterns.)")) { + bool has_guard = false; + if (match(GDScriptTokenizer::Token::WHEN)) { + // Pattern guard. + // Create block for guard because it also needs to access the bound variables from patterns, and we don't want to add them to the outer scope. + branch->guard_body = alloc_node(); + if (branch->patterns.size() > 0) { + for (const KeyValue &E : branch->patterns[0]->binds) { + SuiteNode::Local local(E.value, current_function); + local.type = SuiteNode::Local::PATTERN_BIND; + branch->guard_body->add_local(local); + } + } + + SuiteNode *parent_block = current_suite; + branch->guard_body->parent_block = parent_block; + current_suite = branch->guard_body; + + ExpressionNode *guard = parse_expression(false); + if (guard == nullptr) { + push_error(R"(Expected expression for pattern guard after "when".)"); + } else { + branch->guard_body->statements.append(guard); + } + current_suite = parent_block; + complete_extents(branch->guard_body); + + has_guard = true; + branch->has_wildcard = false; // If it has a guard, the wildcard might still not match. + } + + if (!consume(GDScriptTokenizer::Token::COLON, vformat(R"(Expected ":"%s after "match" %s.)", has_guard ? "" : R"( or "when")", has_guard ? "pattern guard" : "patterns"))) { complete_extents(branch); return nullptr; } @@ -3674,6 +3704,7 @@ GDScriptParser::ParseRule *GDScriptParser::get_rule(GDScriptTokenizer::Token::Ty { nullptr, nullptr, PREC_NONE }, // PASS, { nullptr, nullptr, PREC_NONE }, // RETURN, { nullptr, nullptr, PREC_NONE }, // MATCH, + { nullptr, nullptr, PREC_NONE }, // WHEN, // Keywords { nullptr, &GDScriptParser::parse_cast, PREC_CAST }, // AS, { nullptr, nullptr, PREC_NONE }, // ASSERT, diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 3bd3696e99d3..ec6ea5e78f6a 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -948,6 +948,7 @@ public: Vector patterns; SuiteNode *block = nullptr; bool has_wildcard = false; + SuiteNode *guard_body = nullptr; MatchBranchNode() { type = MATCH_BRANCH; diff --git a/modules/gdscript/gdscript_tokenizer.cpp b/modules/gdscript/gdscript_tokenizer.cpp index 07f2b8b406df..98a3a1268f28 100644 --- a/modules/gdscript/gdscript_tokenizer.cpp +++ b/modules/gdscript/gdscript_tokenizer.cpp @@ -99,6 +99,7 @@ static const char *token_names[] = { "pass", // PASS, "return", // RETURN, "match", // MATCH, + "when", // WHEN, // Keywords "as", // AS, "assert", // ASSERT, @@ -187,6 +188,7 @@ bool GDScriptTokenizer::Token::is_identifier() const { switch (type) { case IDENTIFIER: case MATCH: // Used in String.match(). + case WHEN: // New keyword, avoid breaking existing code. // Allow constants to be treated as regular identifiers. case CONST_PI: case CONST_INF: @@ -241,6 +243,7 @@ bool GDScriptTokenizer::Token::is_node_name() const { case VAR: case VOID: case WHILE: + case WHEN: case YIELD: return true; default: @@ -531,6 +534,7 @@ GDScriptTokenizer::Token GDScriptTokenizer::annotation() { KEYWORD("void", Token::VOID) \ KEYWORD_GROUP('w') \ KEYWORD("while", Token::WHILE) \ + KEYWORD("when", Token::WHEN) \ KEYWORD_GROUP('y') \ KEYWORD("yield", Token::YIELD) \ KEYWORD_GROUP('I') \ diff --git a/modules/gdscript/gdscript_tokenizer.h b/modules/gdscript/gdscript_tokenizer.h index f916407b18b7..6dd8a9865260 100644 --- a/modules/gdscript/gdscript_tokenizer.h +++ b/modules/gdscript/gdscript_tokenizer.h @@ -105,6 +105,7 @@ public: PASS, RETURN, MATCH, + WHEN, // Keywords AS, ASSERT, diff --git a/modules/gdscript/tests/scripts/analyzer/errors/match_guard_invalid_expression.gd b/modules/gdscript/tests/scripts/analyzer/errors/match_guard_invalid_expression.gd new file mode 100644 index 000000000000..1dcb9fc36a05 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/match_guard_invalid_expression.gd @@ -0,0 +1,4 @@ +func test(): + match 0: + _ when a == 0: + print("a does not exist") diff --git a/modules/gdscript/tests/scripts/analyzer/errors/match_guard_invalid_expression.out b/modules/gdscript/tests/scripts/analyzer/errors/match_guard_invalid_expression.out new file mode 100644 index 000000000000..c5f0a90d6aaa --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/match_guard_invalid_expression.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Identifier "a" not declared in the current scope. diff --git a/modules/gdscript/tests/scripts/parser/errors/match_guard_with_assignment.gd b/modules/gdscript/tests/scripts/parser/errors/match_guard_with_assignment.gd new file mode 100644 index 000000000000..d88b4a37c40a --- /dev/null +++ b/modules/gdscript/tests/scripts/parser/errors/match_guard_with_assignment.gd @@ -0,0 +1,5 @@ +func test(): + var a = 0 + match a: + 0 when a = 1: + print("assignment not allowed on pattern guard") diff --git a/modules/gdscript/tests/scripts/parser/errors/match_guard_with_assignment.out b/modules/gdscript/tests/scripts/parser/errors/match_guard_with_assignment.out new file mode 100644 index 000000000000..e8f9130706f6 --- /dev/null +++ b/modules/gdscript/tests/scripts/parser/errors/match_guard_with_assignment.out @@ -0,0 +1,2 @@ +GDTEST_PARSER_ERROR +Assignment is not allowed inside an expression. diff --git a/modules/gdscript/tests/scripts/parser/features/allowed_keywords_as_identifiers.gd b/modules/gdscript/tests/scripts/parser/features/allowed_keywords_as_identifiers.gd index 7e1982597c8d..0c8a5d1367de 100644 --- a/modules/gdscript/tests/scripts/parser/features/allowed_keywords_as_identifiers.gd +++ b/modules/gdscript/tests/scripts/parser/features/allowed_keywords_as_identifiers.gd @@ -14,3 +14,7 @@ func test(): var TAU = "TAU" print(TAU) + + # New keyword for pattern guards. + var when = "when" + print(when) diff --git a/modules/gdscript/tests/scripts/parser/features/allowed_keywords_as_identifiers.out b/modules/gdscript/tests/scripts/parser/features/allowed_keywords_as_identifiers.out index aae2ae13d5cb..8ac8e92ef7a1 100644 --- a/modules/gdscript/tests/scripts/parser/features/allowed_keywords_as_identifiers.out +++ b/modules/gdscript/tests/scripts/parser/features/allowed_keywords_as_identifiers.out @@ -4,3 +4,4 @@ PI INF NAN TAU +when diff --git a/modules/gdscript/tests/scripts/runtime/features/match_with_pattern_guards.gd b/modules/gdscript/tests/scripts/runtime/features/match_with_pattern_guards.gd new file mode 100644 index 000000000000..4cb51f8512db --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/features/match_with_pattern_guards.gd @@ -0,0 +1,71 @@ +var global := 0 + +func test(): + var a = 0 + var b = 1 + + match a: + 0 when b == 0: + print("does not run" if true else "") + 0 when b == 1: + print("guards work") + _: + print("does not run") + + match a: + var a_bind when b == 0: + prints("a is", a_bind, "and b is 0") + var a_bind when b == 1: + prints("a is", a_bind, "and b is 1") + _: + print("does not run") + + match a: + var a_bind when a_bind < 0: + print("a is less than zero") + var a_bind when a_bind == 0: + print("a is equal to zero") + _: + print("a is more than zero") + + match [1, 2, 3]: + [1, 2, var element] when element == 0: + print("does not run") + [1, 2, var element] when element == 3: + print("3rd element is 3") + + match a: + _ when b == 0: + print("does not run") + _ when b == 1: + print("works with wildcard too.") + _: + print("does not run") + + match a: + 0, 1 when b == 0: + print("does not run") + 0, 1 when b == 1: + print("guard with multiple patterns") + _: + print("does not run") + + match a: + 0 when b == 0: + print("does not run") + 0: + print("regular pattern after guard mismatch") + + match a: + 1 when side_effect(): + print("should not run the side effect call") + 0 when side_effect(): + print("will run the side effect call, but not this") + _: + assert(global == 1) + print("side effect only ran once") + +func side_effect(): + print("side effect") + global += 1 + return false diff --git a/modules/gdscript/tests/scripts/runtime/features/match_with_pattern_guards.out b/modules/gdscript/tests/scripts/runtime/features/match_with_pattern_guards.out new file mode 100644 index 000000000000..452d1ff4bf4a --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/features/match_with_pattern_guards.out @@ -0,0 +1,10 @@ +GDTEST_OK +guards work +a is 0 and b is 1 +a is equal to zero +3rd element is 3 +works with wildcard too. +guard with multiple patterns +regular pattern after guard mismatch +side effect +side effect only ran once