From 2a47a01ad7ba91270697e5c7907ea06c4e5e0e6a Mon Sep 17 00:00:00 2001 From: zjin <184326668+zjin123@users.noreply.github.com> Date: Thu, 24 Oct 2024 11:23:01 +0800 Subject: [PATCH 1/2] GDScript: support variable definition as condition in if-statement (godotengine/godot-proposals#2727) --- .../gdscript_translation_parser_plugin.cpp | 3 + modules/gdscript/gdscript_analyzer.cpp | 3 + modules/gdscript/gdscript_compiler.cpp | 147 +++++++++++------- modules/gdscript/gdscript_compiler.h | 2 + modules/gdscript/gdscript_parser.cpp | 98 ++++++++++-- modules/gdscript/gdscript_parser.h | 4 +- .../analyzer/errors/if_var_use_in_elif.gd | 5 + .../analyzer/errors/if_var_use_in_elif.out | 2 + .../analyzer/errors/if_var_use_in_else.gd | 5 + .../analyzer/errors/if_var_use_in_else.out | 2 + .../analyzer/errors/if_var_use_outside.gd | 5 + .../analyzer/errors/if_var_use_outside.out | 2 + .../parser/errors/assignment_in_var_if.gd | 4 - .../parser/errors/assignment_in_var_if.out | 2 - .../parser/errors/if_var_miss_initializer.gd | 3 + .../parser/errors/if_var_miss_initializer.out | 2 + .../gdscript/tests/scripts/runtime/if_var.gd | 22 +++ .../gdscript/tests/scripts/runtime/if_var.out | 5 + 18 files changed, 241 insertions(+), 75 deletions(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/if_var_use_outside.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/if_var_use_outside.out delete mode 100644 modules/gdscript/tests/scripts/parser/errors/assignment_in_var_if.gd delete mode 100644 modules/gdscript/tests/scripts/parser/errors/assignment_in_var_if.out create mode 100644 modules/gdscript/tests/scripts/parser/errors/if_var_miss_initializer.gd create mode 100644 modules/gdscript/tests/scripts/parser/errors/if_var_miss_initializer.out create mode 100644 modules/gdscript/tests/scripts/runtime/if_var.gd create mode 100644 modules/gdscript/tests/scripts/runtime/if_var.out diff --git a/modules/gdscript/editor/gdscript_translation_parser_plugin.cpp b/modules/gdscript/editor/gdscript_translation_parser_plugin.cpp index b31ae878cef6..49148e4b3245 100644 --- a/modules/gdscript/editor/gdscript_translation_parser_plugin.cpp +++ b/modules/gdscript/editor/gdscript_translation_parser_plugin.cpp @@ -135,6 +135,9 @@ void GDScriptEditorTranslationParserPlugin::_traverse_block(const GDScriptParser } break; case GDScriptParser::Node::IF: { const GDScriptParser::IfNode *if_node = static_cast(statement); + if (if_node->variable) { + _assess_expression(if_node->variable->initializer); + } _assess_expression(if_node->condition); _traverse_block(if_node->true_block); _traverse_block(if_node->false_block); diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 4a3a3a4b6152..1bf47863b7ca 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -2096,6 +2096,9 @@ void GDScriptAnalyzer::resolve_parameter(GDScriptParser::ParameterNode *p_parame } void GDScriptAnalyzer::resolve_if(GDScriptParser::IfNode *p_if) { + if (p_if->variable) { + resolve_variable(p_if->variable, true); + } reduce_expression(p_if->condition); resolve_suite(p_if->true_block); diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index f4f445e09667..922a4e2a72b2 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -251,6 +251,45 @@ static bool _can_use_validate_call(const MethodBind *p_method, const Vectoridentifier->name]; + GDScriptDataType local_type = _gdtype_from_datatype(p_variable->get_datatype(), codegen.script); + + Error err = OK; + + bool initialized = false; + if (p_variable->initializer != nullptr) { + GDScriptCodeGenerator::Address src_address = _parse_expression(codegen, err, p_variable->initializer); + if (err) { + return err; + } + if (p_variable->use_conversion_assign) { + gen->write_assign_with_conversion(local, src_address); + } else { + gen->write_assign(local, src_address); + } + if (src_address.mode == GDScriptCodeGenerator::Address::TEMPORARY) { + codegen.generator->pop_temporary(); + } + initialized = true; + } else if ((local_type.has_type && local_type.kind == GDScriptDataType::BUILTIN) || codegen.generator->is_local_dirty(local)) { + // Initialize with default for the type. Built-in types must always be cleared (they cannot be `null`). + // Objects and untyped variables are assigned to `null` only if the stack address has been re-used and not cleared. + codegen.generator->clear_address(local); + initialized = true; + } + + // Don't check `is_local_dirty()` since the variable must be assigned to `null` **on each iteration**. + if (!initialized && p_block->is_in_loop) { + codegen.generator->clear_address(local); + } + + return err; +} + GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::ExpressionNode *p_expression, bool p_root, bool p_initializer) { if (p_expression->is_constant && !(p_expression->get_datatype().is_meta_type && p_expression->get_datatype().kind == GDScriptParser::DataType::CLASS)) { return codegen.add_constant(p_expression->reduced_value); @@ -1878,6 +1917,56 @@ void GDScriptCompiler::_clear_block_locals(CodeGen &codegen, const List block_locals; + + gen->clear_temporaries(); + codegen.start_block(); + block_locals = _add_block_locals(codegen, if_n->condition_block); + + if (if_n->variable) { + err = _parse_variable(codegen, if_n->variable, if_n->condition_block); + if (err) { + return err; + } + + gen->clear_temporaries(); + } + + GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, if_n->condition); + if (err) { + return err; + } + gen->write_if(condition); + + if (condition.mode == GDScriptCodeGenerator::Address::TEMPORARY) { + codegen.generator->pop_temporary(); + } + + err = _parse_block(codegen, if_n->true_block); + if (err) { + return err; + } + + _clear_block_locals(codegen, block_locals); + codegen.end_block(); + + if (if_n->false_block) { + gen->write_else(); + + err = _parse_block(codegen, if_n->false_block); + if (err) { + return err; + } + } + + gen->write_endif(); + + return OK; +} + Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block, bool p_add_locals, bool p_clear_locals) { Error err = OK; GDScriptCodeGenerator *gen = codegen.generator; @@ -2004,32 +2093,10 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui } break; case GDScriptParser::Node::IF: { const GDScriptParser::IfNode *if_n = static_cast(s); - GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, if_n->condition); + err = _parse_if(codegen, if_n); if (err) { return err; } - - gen->write_if(condition); - - if (condition.mode == GDScriptCodeGenerator::Address::TEMPORARY) { - codegen.generator->pop_temporary(); - } - - err = _parse_block(codegen, if_n->true_block); - if (err) { - return err; - } - - if (if_n->false_block) { - gen->write_else(); - - err = _parse_block(codegen, if_n->false_block); - if (err) { - return err; - } - } - - gen->write_endif(); } break; case GDScriptParser::Node::FOR: { const GDScriptParser::ForNode *for_n = static_cast(s); @@ -2166,36 +2233,10 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui #endif } break; case GDScriptParser::Node::VARIABLE: { - const GDScriptParser::VariableNode *lv = static_cast(s); - // Should be already in stack when the block began. - GDScriptCodeGenerator::Address local = codegen.locals[lv->identifier->name]; - GDScriptDataType local_type = _gdtype_from_datatype(lv->get_datatype(), codegen.script); - - bool initialized = false; - if (lv->initializer != nullptr) { - GDScriptCodeGenerator::Address src_address = _parse_expression(codegen, err, lv->initializer); - if (err) { - return err; - } - if (lv->use_conversion_assign) { - gen->write_assign_with_conversion(local, src_address); - } else { - gen->write_assign(local, src_address); - } - if (src_address.mode == GDScriptCodeGenerator::Address::TEMPORARY) { - codegen.generator->pop_temporary(); - } - initialized = true; - } else if ((local_type.has_type && local_type.kind == GDScriptDataType::BUILTIN) || codegen.generator->is_local_dirty(local)) { - // Initialize with default for the type. Built-in types must always be cleared (they cannot be `null`). - // Objects and untyped variables are assigned to `null` only if the stack address has been re-used and not cleared. - codegen.generator->clear_address(local); - initialized = true; - } - - // Don't check `is_local_dirty()` since the variable must be assigned to `null` **on each iteration**. - if (!initialized && p_block->is_in_loop) { - codegen.generator->clear_address(local); + const GDScriptParser::VariableNode *variable_n = static_cast(s); + err = _parse_variable(codegen, variable_n, p_block); + if (err) { + return err; } } break; case GDScriptParser::Node::CONSTANT: { diff --git a/modules/gdscript/gdscript_compiler.h b/modules/gdscript/gdscript_compiler.h index 45f0f9e19b97..8fbe44585ad7 100644 --- a/modules/gdscript/gdscript_compiler.h +++ b/modules/gdscript/gdscript_compiler.h @@ -155,6 +155,8 @@ class GDScriptCompiler { GDScriptCodeGenerator::Address _parse_match_pattern(CodeGen &codegen, Error &r_error, const GDScriptParser::PatternNode *p_pattern, const GDScriptCodeGenerator::Address &p_value_addr, const GDScriptCodeGenerator::Address &p_type_addr, const GDScriptCodeGenerator::Address &p_previous_test, bool p_is_first, bool p_is_nested); List _add_block_locals(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block); void _clear_block_locals(CodeGen &codegen, const List &p_locals); + Error _parse_variable(CodeGen &codegen, const GDScriptParser::VariableNode *p_variable, const GDScriptParser::SuiteNode *p_block); + Error _parse_if(CodeGen &codegen, const GDScriptParser::IfNode *if_n); Error _parse_block(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block, bool p_add_locals = true, bool p_clear_locals = true); GDScriptFunction *_parse_function(Error &r_error, GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready = false, bool p_for_lambda = false); GDScriptFunction *_make_static_initializer(Error &r_error, GDScript *p_script, const GDScriptParser::ClassNode *p_class); diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index e30f03afad12..7ad4adf474e3 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -1076,7 +1076,7 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static) { return parse_variable(p_is_static, true); } -GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static, bool p_allow_property) { +GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static, bool p_allow_property, bool p_is_if_condition) { VariableNode *variable = alloc_node(); if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected variable name after "var".)")) { @@ -1135,7 +1135,10 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static, b } complete_extents(variable); - end_statement("variable declaration"); + + if (p_is_if_condition == false) { + end_statement("variable declaration"); + } return variable; } @@ -2087,21 +2090,68 @@ GDScriptParser::ForNode *GDScriptParser::parse_for() { } GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) { - IfNode *n_if = alloc_node(); + SuiteNode *condition_block = alloc_node(); + condition_block->parent_block = current_suite; + condition_block->parent_function = current_function; + SuiteNode *saved_suite = current_suite; + current_suite = condition_block; + + VariableNode *variable = nullptr; + ExpressionNode *condition = nullptr; + if (match(GDScriptTokenizer::Token::VAR)) { + // Variable declaration + variable = parse_variable(false, false, true); + if (variable == nullptr) { + push_error(vformat(R"(Expected variable definition after "%s".)", p_token)); + } else if (variable->initializer == nullptr) { + push_error(R"(Expected expression for variable initial value.)"); + } else { + const SuiteNode::Local &local = current_suite->get_local(variable->identifier->name); + if (local.type != SuiteNode::Local::UNDEFINED) { + push_error(vformat(R"(There is already a %s named "%s" declared in this scope.)", local.get_name(), variable->identifier->name), variable->identifier); + } + condition_block->add_local(variable, current_function); + + IdentifierNode *identifier = alloc_node(); + identifier->name = variable->identifier->name; + identifier->suite = condition_block; + identifier->source = IdentifierNode::Source::LOCAL_VARIABLE; + const SuiteNode::Local &declaration = condition_block->get_local(identifier->name); + identifier->variable_source = declaration.variable; + declaration.variable->usages++; + complete_extents(identifier); + + condition_block->statements.push_back(variable); - n_if->condition = parse_expression(false); - if (n_if->condition == nullptr) { - push_error(vformat(R"(Expected conditional expression after "%s".)", p_token)); + condition = identifier; + } + } else { + // Expression + ExpressionNode *expression = parse_expression(false); + if (expression == nullptr) { + push_error(vformat(R"(Expected conditional expression after "%s".)", p_token)); + } else { + condition = expression; + } } consume(GDScriptTokenizer::Token::COLON, vformat(R"(Expected ":" after "%s" condition.)", p_token)); - n_if->true_block = parse_suite(vformat(R"("%s" block)", p_token)); - n_if->true_block->parent_if = n_if; + SuiteNode *true_block = parse_suite(vformat(R"("%s" block)", p_token)); - if (n_if->true_block->has_continue) { - current_suite->has_continue = true; - } + complete_extents(condition_block); + current_suite = saved_suite; + + IfNode *n_if = alloc_node(); + + true_block->parent_function = current_function; + true_block->parent_block = condition_block; + true_block->parent_if = n_if; + + n_if->variable = variable; + n_if->condition = condition; + n_if->condition_block = condition_block; + n_if->true_block = true_block; if (match(GDScriptTokenizer::Token::ELIF)) { SuiteNode *else_block = alloc_node(); @@ -2123,10 +2173,10 @@ GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) { } complete_extents(n_if); - if (n_if->false_block != nullptr && n_if->false_block->has_return && n_if->true_block->has_return) { + if ((n_if->false_block != nullptr && n_if->false_block->has_return) && n_if->true_block->has_return) { current_suite->has_return = true; } - if (n_if->false_block != nullptr && n_if->false_block->has_continue) { + if ((n_if->false_block != nullptr && n_if->false_block->has_continue) || n_if->true_block->has_continue) { current_suite->has_continue = true; } @@ -4787,11 +4837,25 @@ bool GDScriptParser::warning_annotations(AnnotationNode *p_annotation, Node *p_t // Contain bodies. SIMPLE_CASE(Node::FOR, ForNode, list) - SIMPLE_CASE(Node::IF, IfNode, condition) SIMPLE_CASE(Node::MATCH, MatchNode, test) SIMPLE_CASE(Node::WHILE, WhileNode, condition) #undef SIMPLE_CASE + case Node::IF: { + IfNode *if_n = static_cast(p_target); + if (if_n->variable) { + if (if_n->variable->initializer) { + end_line = if_n->variable->initializer->end_line; + } else { + end_line = if_n->start_line; + } + } else if (if_n->condition) { + end_line = if_n->condition->end_line; + } else { + end_line = if_n->start_line; + } + } break; + case Node::CLASS: { end_line = p_target->start_line; for (const AnnotationNode *annotation : p_target->annotations) { @@ -5744,7 +5808,11 @@ void GDScriptParser::TreePrinter::print_if(IfNode *p_if, bool p_is_elif) { } else { push_text("If "); } - print_expression(p_if->condition); + if (p_if->variable) { + print_variable(p_if->variable); + } else { + print_expression(p_if->condition); + } push_line(" :"); increase_indent(); diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 7f64ae902b03..3dd62ab14504 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -918,7 +918,9 @@ class GDScriptParser { }; struct IfNode : public Node { + VariableNode *variable = nullptr; ExpressionNode *condition = nullptr; + SuiteNode *condition_block = nullptr; SuiteNode *true_block = nullptr; SuiteNode *false_block = nullptr; @@ -1516,7 +1518,7 @@ class GDScriptParser { // Statements. Node *parse_statement(); VariableNode *parse_variable(bool p_is_static); - VariableNode *parse_variable(bool p_is_static, bool p_allow_property); + VariableNode *parse_variable(bool p_is_static, bool p_allow_property, bool p_is_if_condition = false); VariableNode *parse_property(VariableNode *p_variable, bool p_need_indent); void parse_property_getter(VariableNode *p_variable); void parse_property_setter(VariableNode *p_variable); diff --git a/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.gd b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.gd new file mode 100644 index 000000000000..947fed099698 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.gd @@ -0,0 +1,5 @@ +func test(): + if var x = 100: + print("t") + elif x > 0: + print("f") diff --git a/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.out b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.out new file mode 100644 index 000000000000..ef7c66078e5a --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Identifier "x" not declared in the current scope. diff --git a/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.gd b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.gd new file mode 100644 index 000000000000..6ad281033779 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.gd @@ -0,0 +1,5 @@ +func test(): + if var x = 100: + print("t") + else: + print(x) diff --git a/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.out b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.out new file mode 100644 index 000000000000..ef7c66078e5a --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Identifier "x" not declared in the current scope. diff --git a/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_outside.gd b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_outside.gd new file mode 100644 index 000000000000..3afec0d3673d --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_outside.gd @@ -0,0 +1,5 @@ +func test(): + if var x = 100: + print("t") + + print(x) diff --git a/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_outside.out b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_outside.out new file mode 100644 index 000000000000..ef7c66078e5a --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/if_var_use_outside.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Identifier "x" not declared in the current scope. diff --git a/modules/gdscript/tests/scripts/parser/errors/assignment_in_var_if.gd b/modules/gdscript/tests/scripts/parser/errors/assignment_in_var_if.gd deleted file mode 100644 index a99557fa3c5a..000000000000 --- a/modules/gdscript/tests/scripts/parser/errors/assignment_in_var_if.gd +++ /dev/null @@ -1,4 +0,0 @@ -func test(): - # Error here. - if var foo = 25: - print(foo) diff --git a/modules/gdscript/tests/scripts/parser/errors/assignment_in_var_if.out b/modules/gdscript/tests/scripts/parser/errors/assignment_in_var_if.out deleted file mode 100644 index e84f4652acd6..000000000000 --- a/modules/gdscript/tests/scripts/parser/errors/assignment_in_var_if.out +++ /dev/null @@ -1,2 +0,0 @@ -GDTEST_PARSER_ERROR -Expected conditional expression after "if". diff --git a/modules/gdscript/tests/scripts/parser/errors/if_var_miss_initializer.gd b/modules/gdscript/tests/scripts/parser/errors/if_var_miss_initializer.gd new file mode 100644 index 000000000000..3e85f36a76d8 --- /dev/null +++ b/modules/gdscript/tests/scripts/parser/errors/if_var_miss_initializer.gd @@ -0,0 +1,3 @@ +func test(): + if var x: + pass diff --git a/modules/gdscript/tests/scripts/parser/errors/if_var_miss_initializer.out b/modules/gdscript/tests/scripts/parser/errors/if_var_miss_initializer.out new file mode 100644 index 000000000000..df6b9a80bd30 --- /dev/null +++ b/modules/gdscript/tests/scripts/parser/errors/if_var_miss_initializer.out @@ -0,0 +1,2 @@ +GDTEST_PARSER_ERROR +Expected type after ":" diff --git a/modules/gdscript/tests/scripts/runtime/if_var.gd b/modules/gdscript/tests/scripts/runtime/if_var.gd new file mode 100644 index 000000000000..70cfbe0819f6 --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/if_var.gd @@ -0,0 +1,22 @@ +func foo(x: int): + return x > 0 + +func test(): + if var x = 25: + print(x) + + if var x = 25: + print(x) + else: + print("ff") + + if var x = foo(0): + print(x) + + if var x = foo(0): + print(x) + else: + print("fff") + + if var x := signi(100): + print(x) diff --git a/modules/gdscript/tests/scripts/runtime/if_var.out b/modules/gdscript/tests/scripts/runtime/if_var.out new file mode 100644 index 000000000000..843b5baa2cff --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/if_var.out @@ -0,0 +1,5 @@ +GDTEST_OK +25 +25 +fff +1 From fa201b2eb09e81253c2aa4dec505cdfb85837692 Mon Sep 17 00:00:00 2001 From: zjin <184326668+zjin123@users.noreply.github.com> Date: Thu, 24 Oct 2024 13:47:39 +0800 Subject: [PATCH 2/2] GDScript: support multiple variable definitions and conditions in if-statement --- .../gdscript_translation_parser_plugin.cpp | 12 +- modules/gdscript/gdscript_analyzer.cpp | 12 +- modules/gdscript/gdscript_byte_codegen.cpp | 9 +- modules/gdscript/gdscript_byte_codegen.h | 2 +- modules/gdscript/gdscript_codegen.h | 2 +- modules/gdscript/gdscript_compiler.cpp | 52 +++++--- modules/gdscript/gdscript_editor.cpp | 42 ++++--- modules/gdscript/gdscript_parser.cpp | 119 +++++++++++------- modules/gdscript/gdscript_parser.h | 3 +- .../errors/if_var_duplicated_definitions.gd | 3 + .../errors/if_var_duplicated_definitions.out | 2 + .../runtime/if_var_multiple_conditions.gd | 37 ++++++ .../runtime/if_var_multiple_conditions.out | 40 ++++++ 13 files changed, 241 insertions(+), 94 deletions(-) create mode 100644 modules/gdscript/tests/scripts/parser/errors/if_var_duplicated_definitions.gd create mode 100644 modules/gdscript/tests/scripts/parser/errors/if_var_duplicated_definitions.out create mode 100644 modules/gdscript/tests/scripts/runtime/if_var_multiple_conditions.gd create mode 100644 modules/gdscript/tests/scripts/runtime/if_var_multiple_conditions.out diff --git a/modules/gdscript/editor/gdscript_translation_parser_plugin.cpp b/modules/gdscript/editor/gdscript_translation_parser_plugin.cpp index 49148e4b3245..554208236b2e 100644 --- a/modules/gdscript/editor/gdscript_translation_parser_plugin.cpp +++ b/modules/gdscript/editor/gdscript_translation_parser_plugin.cpp @@ -135,10 +135,16 @@ void GDScriptEditorTranslationParserPlugin::_traverse_block(const GDScriptParser } break; case GDScriptParser::Node::IF: { const GDScriptParser::IfNode *if_node = static_cast(statement); - if (if_node->variable) { - _assess_expression(if_node->variable->initializer); + const List::Element *E = if_node->conditions.front(); + while (E) { + const GDScriptParser::Node *node = E->get(); + if (node->is_expression()) { + _assess_expression(static_cast(node)); + } else if (node->type == GDScriptParser::Node::VARIABLE) { + _assess_expression(static_cast(node)->initializer); + } + E = E->next(); } - _assess_expression(if_node->condition); _traverse_block(if_node->true_block); _traverse_block(if_node->false_block); } break; diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 1bf47863b7ca..8f074e955b0f 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -2096,10 +2096,16 @@ void GDScriptAnalyzer::resolve_parameter(GDScriptParser::ParameterNode *p_parame } void GDScriptAnalyzer::resolve_if(GDScriptParser::IfNode *p_if) { - if (p_if->variable) { - resolve_variable(p_if->variable, true); + List::Element *E = p_if->conditions.front(); + while (E) { + GDScriptParser::Node *condition = E->get(); + if (condition->is_expression()) { + reduce_expression(static_cast(condition)); + } else if (condition->type == GDScriptParser::Node::VARIABLE) { + resolve_variable(static_cast(condition), true); + } + E = E->next(); } - reduce_expression(p_if->condition); resolve_suite(p_if->true_block); p_if->set_datatype(p_if->true_block->get_datatype()); diff --git a/modules/gdscript/gdscript_byte_codegen.cpp b/modules/gdscript/gdscript_byte_codegen.cpp index b77c641eb550..15832256ade1 100644 --- a/modules/gdscript/gdscript_byte_codegen.cpp +++ b/modules/gdscript/gdscript_byte_codegen.cpp @@ -1501,13 +1501,16 @@ void GDScriptByteCodeGenerator::write_if(const Address &p_condition) { append(0); // Jump destination, will be patched. } -void GDScriptByteCodeGenerator::write_else() { +void GDScriptByteCodeGenerator::write_else(int count) { append_opcode(GDScriptFunction::OPCODE_JUMP); // Jump from true if block; int else_jmp_addr = opcodes.size(); append(0); // Jump destination, will be patched. - patch_jump(if_jmp_addrs.back()->get()); - if_jmp_addrs.pop_back(); + for (int i = 0; i < count; i++) { + patch_jump(if_jmp_addrs.back()->get()); + if_jmp_addrs.pop_back(); + } + if_jmp_addrs.push_back(else_jmp_addr); } diff --git a/modules/gdscript/gdscript_byte_codegen.h b/modules/gdscript/gdscript_byte_codegen.h index 6303db71fd70..cb724d895236 100644 --- a/modules/gdscript/gdscript_byte_codegen.h +++ b/modules/gdscript/gdscript_byte_codegen.h @@ -532,7 +532,7 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator { virtual void write_construct_typed_dictionary(const Address &p_target, const GDScriptDataType &p_key_type, const GDScriptDataType &p_value_type, const Vector
&p_arguments) override; virtual void write_await(const Address &p_target, const Address &p_operand) override; virtual void write_if(const Address &p_condition) override; - virtual void write_else() override; + virtual void write_else(int count) override; virtual void write_endif() override; virtual void write_jump_if_shared(const Address &p_value) override; virtual void write_end_jump_if_shared() override; diff --git a/modules/gdscript/gdscript_codegen.h b/modules/gdscript/gdscript_codegen.h index f3c4acf1c344..a30e5db9e044 100644 --- a/modules/gdscript/gdscript_codegen.h +++ b/modules/gdscript/gdscript_codegen.h @@ -145,7 +145,7 @@ class GDScriptCodeGenerator { virtual void write_construct_typed_dictionary(const Address &p_target, const GDScriptDataType &p_key_type, const GDScriptDataType &p_value_type, const Vector
&p_arguments) = 0; virtual void write_await(const Address &p_target, const Address &p_operand) = 0; virtual void write_if(const Address &p_condition) = 0; - virtual void write_else() = 0; + virtual void write_else(int count) = 0; virtual void write_endif() = 0; virtual void write_jump_if_shared(const Address &p_value) = 0; virtual void write_end_jump_if_shared() = 0; diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 922a4e2a72b2..0c928aa186fe 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -1926,23 +1926,35 @@ Error GDScriptCompiler::_parse_if(CodeGen &codegen, const GDScriptParser::IfNode codegen.start_block(); block_locals = _add_block_locals(codegen, if_n->condition_block); - if (if_n->variable) { - err = _parse_variable(codegen, if_n->variable, if_n->condition_block); - if (err) { - return err; - } + int count = 0; + const List::Element *E = if_n->conditions.front(); + while (E) { + const GDScriptParser::Node *condition_n = E->get(); + if (condition_n->is_expression()) { + const GDScriptParser::ExpressionNode *expression_n = static_cast(condition_n); + + GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, expression_n); + if (err) { + return err; + } + gen->write_if(condition); - gen->clear_temporaries(); - } + if (condition.mode == GDScriptCodeGenerator::Address::TEMPORARY) { + codegen.generator->pop_temporary(); + } - GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, if_n->condition); - if (err) { - return err; - } - gen->write_if(condition); + count++; + } else if (condition_n->type == GDScriptParser::Node::VARIABLE) { + const GDScriptParser::VariableNode *variable_n = static_cast(condition_n); + + err = _parse_variable(codegen, variable_n, if_n->condition_block); + if (err) { + return err; + } - if (condition.mode == GDScriptCodeGenerator::Address::TEMPORARY) { - codegen.generator->pop_temporary(); + gen->clear_temporaries(); + } + E = E->next(); } err = _parse_block(codegen, if_n->true_block); @@ -1954,15 +1966,19 @@ Error GDScriptCompiler::_parse_if(CodeGen &codegen, const GDScriptParser::IfNode codegen.end_block(); if (if_n->false_block) { - gen->write_else(); + gen->write_else(count); err = _parse_block(codegen, if_n->false_block); if (err) { return err; } - } - gen->write_endif(); + gen->write_endif(); + } else { + for (int i = 0; i < count; i++) { + gen->write_endif(); + } + } return OK; } @@ -2024,7 +2040,7 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui for (int j = 0; j < match->branches.size(); j++) { if (j > 0) { // Use `else` to not check the next branch after matching. - gen->write_else(); + gen->write_else(1); } const GDScriptParser::MatchBranchNode *branch = match->branches[j]; diff --git a/modules/gdscript/gdscript_editor.cpp b/modules/gdscript/gdscript_editor.cpp index 3de1decc1800..60c238075817 100644 --- a/modules/gdscript/gdscript_editor.cpp +++ b/modules/gdscript/gdscript_editor.cpp @@ -2205,25 +2205,33 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context, } } - if (suite->parent_if && suite->parent_if->condition && suite->parent_if->condition->type == GDScriptParser::Node::TYPE_TEST) { - // Operator `is` used, check if identifier is in there! this helps resolve in blocks that are (if (identifier is value)): which are very common.. - // Super dirty hack, but very useful. - // Credit: Zylann. - // TODO: this could be hacked to detect ANDed conditions too... - const GDScriptParser::TypeTestNode *type_test = static_cast(suite->parent_if->condition); - if (type_test->operand && type_test->test_type && type_test->operand->type == GDScriptParser::Node::IDENTIFIER && static_cast(type_test->operand)->name == p_identifier->name && static_cast(type_test->operand)->source == p_identifier->source) { - // Bingo. - GDScriptParser::CompletionContext c = p_context; - c.current_line = type_test->operand->start_line; - c.current_suite = suite; - if (type_test->test_datatype.is_hard_type()) { - id_type.type = type_test->test_datatype; - if (last_assign_line < c.current_line) { - // Override last assignment. - last_assign_line = c.current_line; - last_assigned_expression = nullptr; + if (suite->parent_if) { + List::Element *E = suite->parent_if->conditions.front(); + while (E) { + GDScriptParser::Node *condition = E->get(); + if (condition->is_expression() && condition->type == GDScriptParser::Node::TYPE_TEST) { + // Operator `is` used, check if identifier is in there! this helps resolve in blocks that are (if (identifier is value)): which are very common.. + // Super dirty hack, but very useful. + // Credit: Zylann. + // TODO: this could be hacked to detect ANDed conditions too... + const GDScriptParser::TypeTestNode *type_test = static_cast(condition); + if (type_test->operand && type_test->test_type && type_test->operand->type == GDScriptParser::Node::IDENTIFIER && static_cast(type_test->operand)->name == p_identifier->name && static_cast(type_test->operand)->source == p_identifier->source) { + // Bingo. + GDScriptParser::CompletionContext c = p_context; + c.current_line = type_test->operand->start_line; + c.current_suite = suite; + if (type_test->test_datatype.is_hard_type()) { + id_type.type = type_test->test_datatype; + if (last_assign_line < c.current_line) { + // Override last assignment. + last_assign_line = c.current_line; + last_assigned_expression = nullptr; + } + } + break; } } + E = E->next(); } } diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 7ad4adf474e3..59d0e0b3c1da 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -2096,44 +2096,50 @@ GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) { SuiteNode *saved_suite = current_suite; current_suite = condition_block; - VariableNode *variable = nullptr; - ExpressionNode *condition = nullptr; - if (match(GDScriptTokenizer::Token::VAR)) { - // Variable declaration - variable = parse_variable(false, false, true); - if (variable == nullptr) { - push_error(vformat(R"(Expected variable definition after "%s".)", p_token)); - } else if (variable->initializer == nullptr) { - push_error(R"(Expected expression for variable initial value.)"); - } else { - const SuiteNode::Local &local = current_suite->get_local(variable->identifier->name); - if (local.type != SuiteNode::Local::UNDEFINED) { - push_error(vformat(R"(There is already a %s named "%s" declared in this scope.)", local.get_name(), variable->identifier->name), variable->identifier); - } - condition_block->add_local(variable, current_function); + List conditions; + do { + if (match(GDScriptTokenizer::Token::VAR)) { + // Variable declaration + VariableNode *variable = parse_variable(false, false, true); + if (variable == nullptr) { + push_error(vformat(R"(Expected variable definition after "%s".)", p_token)); + break; + } else if (variable->initializer == nullptr) { + push_error(R"(Expected expression for variable initial value.)"); + break; + } else { + const SuiteNode::Local &local = current_suite->get_local(variable->identifier->name); + if (local.type != SuiteNode::Local::UNDEFINED) { + push_error(vformat(R"(There is already a %s named "%s" declared in this scope.)", local.get_name(), variable->identifier->name), variable->identifier); + break; + } + condition_block->add_local(variable, current_function); - IdentifierNode *identifier = alloc_node(); - identifier->name = variable->identifier->name; - identifier->suite = condition_block; - identifier->source = IdentifierNode::Source::LOCAL_VARIABLE; - const SuiteNode::Local &declaration = condition_block->get_local(identifier->name); - identifier->variable_source = declaration.variable; - declaration.variable->usages++; - complete_extents(identifier); + IdentifierNode *identifier = alloc_node(); + identifier->name = variable->identifier->name; + identifier->suite = condition_block; + identifier->source = IdentifierNode::Source::LOCAL_VARIABLE; + const SuiteNode::Local &declaration = condition_block->get_local(identifier->name); + identifier->variable_source = declaration.variable; + declaration.variable->usages++; + complete_extents(identifier); - condition_block->statements.push_back(variable); + condition_block->statements.push_back(variable); - condition = identifier; - } - } else { - // Expression - ExpressionNode *expression = parse_expression(false); - if (expression == nullptr) { - push_error(vformat(R"(Expected conditional expression after "%s".)", p_token)); + conditions.push_back(variable); + conditions.push_back(identifier); + } } else { - condition = expression; + // Expression + ExpressionNode *expression = parse_expression(false); + if (expression == nullptr) { + push_error(vformat(R"(Expected conditional expression after "%s".)", p_token)); + break; + } else { + conditions.push_back(expression); + } } - } + } while (match(GDScriptTokenizer::Token::COMMA)); consume(GDScriptTokenizer::Token::COLON, vformat(R"(Expected ":" after "%s" condition.)", p_token)); @@ -2148,8 +2154,7 @@ GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) { true_block->parent_block = condition_block; true_block->parent_if = n_if; - n_if->variable = variable; - n_if->condition = condition; + n_if->conditions = conditions; n_if->condition_block = condition_block; n_if->true_block = true_block; @@ -4843,14 +4848,20 @@ bool GDScriptParser::warning_annotations(AnnotationNode *p_annotation, Node *p_t case Node::IF: { IfNode *if_n = static_cast(p_target); - if (if_n->variable) { - if (if_n->variable->initializer) { - end_line = if_n->variable->initializer->end_line; - } else { - end_line = if_n->start_line; + GDScriptParser::ExpressionNode *expression = nullptr; + List::Element *E = if_n->conditions.front(); + while (E) { + GDScriptParser::Node *condition = E->get(); + if (condition->is_expression()) { + expression = static_cast(condition); + } else if (condition->type == GDScriptParser::Node::VARIABLE) { + expression = static_cast(condition)->initializer; + E = E->next(); } - } else if (if_n->condition) { - end_line = if_n->condition->end_line; + E = E->next(); + } + if (expression) { + end_line = expression->end_line; } else { end_line = if_n->start_line; } @@ -5808,11 +5819,27 @@ void GDScriptParser::TreePrinter::print_if(IfNode *p_if, bool p_is_elif) { } else { push_text("If "); } - if (p_if->variable) { - print_variable(p_if->variable); - } else { - print_expression(p_if->condition); + List::Element *E = p_if->conditions.front(); + bool first = true; + while (E) { + if (first) { + first = false; + } else { + push_text(", "); + } + Node *node = E->get(); + if (node->is_expression()) { + print_expression(static_cast(node)); + } else if (node->type == Node::VARIABLE) { + print_variable(static_cast(node)); + // Skip next identifier condition + E = E->next(); + } else { + ERR_PRINT("BUG: invalid condition"); + } + E = E->next(); } + push_line(" :"); increase_indent(); diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 3dd62ab14504..5aa50fc89654 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -918,8 +918,7 @@ class GDScriptParser { }; struct IfNode : public Node { - VariableNode *variable = nullptr; - ExpressionNode *condition = nullptr; + List conditions; SuiteNode *condition_block = nullptr; SuiteNode *true_block = nullptr; SuiteNode *false_block = nullptr; diff --git a/modules/gdscript/tests/scripts/parser/errors/if_var_duplicated_definitions.gd b/modules/gdscript/tests/scripts/parser/errors/if_var_duplicated_definitions.gd new file mode 100644 index 000000000000..7f005adb5d21 --- /dev/null +++ b/modules/gdscript/tests/scripts/parser/errors/if_var_duplicated_definitions.gd @@ -0,0 +1,3 @@ +func test(): + if var x := 1, var x := 2: + print("t") diff --git a/modules/gdscript/tests/scripts/parser/errors/if_var_duplicated_definitions.out b/modules/gdscript/tests/scripts/parser/errors/if_var_duplicated_definitions.out new file mode 100644 index 000000000000..4d606f9faa14 --- /dev/null +++ b/modules/gdscript/tests/scripts/parser/errors/if_var_duplicated_definitions.out @@ -0,0 +1,2 @@ +GDTEST_PARSER_ERROR +There is already a variable named "x" declared in this scope. diff --git a/modules/gdscript/tests/scripts/runtime/if_var_multiple_conditions.gd b/modules/gdscript/tests/scripts/runtime/if_var_multiple_conditions.gd new file mode 100644 index 000000000000..ed1d831121de --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/if_var_multiple_conditions.gd @@ -0,0 +1,37 @@ +func foo(n: int) -> bool: + print("foo") + return n > 0 + +func bar(n: int) -> int: + print("bar") + return n + +func add(a: int, b: int) -> int: + print("add") + return a + b + +func test_if(f: bool, a: int, b: int) -> void: + print("--") + if f, foo(a), var n := bar(b): + print("t:%s" % n) + elif var n = add(a, b), n >= 1: + print("tt:%s" % n) + else: + print("f") + +func test(): + if var x := 100: + if x >= 1, var y := "ttt": + print(y) + print(100 + signi(x)) + + test_if(false, 0, 0); + test_if(false, 0, 1); + test_if(false, 1, 0); + test_if(false, 1, 1); + test_if(true, 0, 0); + test_if(true, 0, 1); + test_if(true, 0, 2); + test_if(true, 1, 0); + test_if(true, 1, 1); + test_if(true, 1, 2); diff --git a/modules/gdscript/tests/scripts/runtime/if_var_multiple_conditions.out b/modules/gdscript/tests/scripts/runtime/if_var_multiple_conditions.out new file mode 100644 index 000000000000..d0a940476da4 --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/if_var_multiple_conditions.out @@ -0,0 +1,40 @@ +GDTEST_OK +ttt +101 +-- +add +f +-- +add +tt:1 +-- +add +tt:1 +-- +add +tt:2 +-- +foo +add +f +-- +foo +add +tt:1 +-- +foo +add +tt:2 +-- +foo +bar +add +tt:1 +-- +foo +bar +t:1 +-- +foo +bar +t:2