From d36213bab8d71575b39c4d5d96622f3e91dc09ee Mon Sep 17 00:00:00 2001 From: George Marques Date: Thu, 14 Oct 2021 19:55:45 -0300 Subject: [PATCH 1/4] GDScript: Make sure calls don't use return when not needed --- modules/gdscript/gdscript_compiler.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 1f9aad40af..ab0fe5c37d 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -488,6 +488,9 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code const GDScriptParser::CallNode *call = static_cast(p_expression); GDScriptDataType type = _gdtype_from_datatype(call->get_datatype()); GDScriptCodeGenerator::Address result = codegen.add_temporary(type); + GDScriptCodeGenerator::Address nil = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::NIL); + + GDScriptCodeGenerator::Address return_addr = p_root ? nil : result; Vector arguments; for (int i = 0; i < call->arguments.size(); i++) { @@ -538,13 +541,13 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code if (within_await) { gen->write_call_async(result, self, call->function_name, arguments); } else { - gen->write_call(result, self, call->function_name, arguments); + gen->write_call(return_addr, self, call->function_name, arguments); } } else { if (within_await) { gen->write_call_self_async(result, call->function_name, arguments); } else { - gen->write_call_self(result, call->function_name, arguments); + gen->write_call_self(return_addr, call->function_name, arguments); } } } else if (callee->type == GDScriptParser::Node::SUBSCRIPT) { @@ -579,12 +582,12 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code gen->write_call_method_bind(result, base, method, arguments); } } else { - gen->write_call(result, base, call->function_name, arguments); + gen->write_call(return_addr, base, call->function_name, arguments); } } else if (base.type.has_type && base.type.kind == GDScriptDataType::BUILTIN) { gen->write_call_builtin_type(result, base, base.type.builtin_type, call->function_name, arguments); } else { - gen->write_call(result, base, call->function_name, arguments); + gen->write_call(return_addr, base, call->function_name, arguments); } if (base.mode == GDScriptCodeGenerator::Address::TEMPORARY) { gen->pop_temporary(); From 056a54db7be5b63685f221e34c17cc4b72964080 Mon Sep 17 00:00:00 2001 From: George Marques Date: Thu, 14 Oct 2021 19:58:10 -0300 Subject: [PATCH 2/4] GDScript: Properly return value with await on non-coroutine If the keyword `await` is used without a coroutine, it should still return the value synchronally. --- modules/gdscript/gdscript_vm.cpp | 4 +++- .../scripts/runtime/features/await_without_coroutine.gd | 8 ++++++++ .../scripts/runtime/features/await_without_coroutine.out | 6 ++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 modules/gdscript/tests/scripts/runtime/features/await_without_coroutine.gd create mode 100644 modules/gdscript/tests/scripts/runtime/features/await_without_coroutine.out diff --git a/modules/gdscript/gdscript_vm.cpp b/modules/gdscript/gdscript_vm.cpp index 1bc7ae086f..7018c339d7 100644 --- a/modules/gdscript/gdscript_vm.cpp +++ b/modules/gdscript/gdscript_vm.cpp @@ -2098,8 +2098,10 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a } if (result.get_type() != Variant::SIGNAL) { + // Not async, return immediately using the target from OPCODE_AWAIT_RESUME. + GET_VARIANT_PTR(target, 3); + *target = result; ip += 4; // Skip OPCODE_AWAIT_RESUME and its data. - // The stack pointer should be the same, so we don't need to set a return value. is_signal = false; } else { sig = result; diff --git a/modules/gdscript/tests/scripts/runtime/features/await_without_coroutine.gd b/modules/gdscript/tests/scripts/runtime/features/await_without_coroutine.gd new file mode 100644 index 0000000000..9da61ab184 --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/features/await_without_coroutine.gd @@ -0,0 +1,8 @@ +# https://github.com/godotengine/godot/issues/50894 + +func test(): + print(await not_coroutine()) + + +func not_coroutine(): + return "awaited" diff --git a/modules/gdscript/tests/scripts/runtime/features/await_without_coroutine.out b/modules/gdscript/tests/scripts/runtime/features/await_without_coroutine.out new file mode 100644 index 0000000000..c2ac488e9b --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/features/await_without_coroutine.out @@ -0,0 +1,6 @@ +GDTEST_OK +>> WARNING +>> Line: 4 +>> REDUNDANT_AWAIT +>> "await" keyword not needed in this case, because the expression isn't a coroutine nor a signal. +awaited From 749d89ae386d27da78ffb28a7cfe11ad5c4a3ae0 Mon Sep 17 00:00:00 2001 From: George Marques Date: Thu, 14 Oct 2021 20:12:01 -0300 Subject: [PATCH 3/4] GDScript: Remove error when coroutine is called without await In the case the call happens as a statement, since the return value isn't used in this case. --- modules/gdscript/gdscript_analyzer.cpp | 10 +++++----- modules/gdscript/gdscript_analyzer.h | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 6b57784b1c..eab9109955 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -1069,7 +1069,7 @@ void GDScriptAnalyzer::resolve_node(GDScriptParser::Node *p_node) { case GDScriptParser::Node::SUBSCRIPT: case GDScriptParser::Node::TERNARY_OPERATOR: case GDScriptParser::Node::UNARY_OPERATOR: - reduce_expression(static_cast(p_node)); + reduce_expression(static_cast(p_node), true); break; case GDScriptParser::Node::BREAK: case GDScriptParser::Node::BREAKPOINT: @@ -1658,7 +1658,7 @@ void GDScriptAnalyzer::resolve_return(GDScriptParser::ReturnNode *p_return) { p_return->set_datatype(result); } -void GDScriptAnalyzer::reduce_expression(GDScriptParser::ExpressionNode *p_expression) { +void GDScriptAnalyzer::reduce_expression(GDScriptParser::ExpressionNode *p_expression, bool p_is_root) { // This one makes some magic happen. if (p_expression == nullptr) { @@ -1686,7 +1686,7 @@ void GDScriptAnalyzer::reduce_expression(GDScriptParser::ExpressionNode *p_expre reduce_binary_op(static_cast(p_expression)); break; case GDScriptParser::Node::CALL: - reduce_call(static_cast(p_expression)); + reduce_call(static_cast(p_expression), p_is_root); break; case GDScriptParser::Node::CAST: reduce_cast(static_cast(p_expression)); @@ -2056,7 +2056,7 @@ void GDScriptAnalyzer::reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_o p_binary_op->set_datatype(result); } -void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool is_await) { +void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_await, bool p_is_root) { bool all_is_constant = true; Map arrays; // For array literal to potentially type when passing. for (int i = 0; i < p_call->arguments.size(); i++) { @@ -2415,7 +2415,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool is_awa } } - if (call_type.is_coroutine && !is_await) { + if (call_type.is_coroutine && !p_is_await && !p_is_root) { push_error(vformat(R"*(Function "%s()" is a coroutine, so it must be called with "await".)*", p_call->function_name), p_call->callee); } diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h index 2e17e15452..ce4525190b 100644 --- a/modules/gdscript/gdscript_analyzer.h +++ b/modules/gdscript/gdscript_analyzer.h @@ -78,12 +78,12 @@ class GDScriptAnalyzer { void resolve_return(GDScriptParser::ReturnNode *p_return); // Reduction functions. - void reduce_expression(GDScriptParser::ExpressionNode *p_expression); + void reduce_expression(GDScriptParser::ExpressionNode *p_expression, bool p_is_root = false); void reduce_array(GDScriptParser::ArrayNode *p_array); void reduce_assignment(GDScriptParser::AssignmentNode *p_assignment); void reduce_await(GDScriptParser::AwaitNode *p_await); void reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_op); - void reduce_call(GDScriptParser::CallNode *p_call, bool is_await = false); + void reduce_call(GDScriptParser::CallNode *p_call, bool p_is_await = false, bool p_is_root = false); void reduce_cast(GDScriptParser::CastNode *p_cast); void reduce_dictionary(GDScriptParser::DictionaryNode *p_dictionary); void reduce_get_node(GDScriptParser::GetNodeNode *p_get_node); From 84abb9a76c9bd1c797fedafbaf52e582fb2a8238 Mon Sep 17 00:00:00 2001 From: George Marques Date: Thu, 14 Oct 2021 20:30:06 -0300 Subject: [PATCH 4/4] GDScript: Fix typing for await expression Don't grab the type of the awaited value unless it's constant (which makes it synchronous) or call (which always use the proper return type). --- modules/gdscript/gdscript_analyzer.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index eab9109955..47f81b455f 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -1927,16 +1927,25 @@ void GDScriptAnalyzer::reduce_await(GDScriptParser::AwaitNode *p_await) { p_await->set_datatype(await_type); return; } + + GDScriptParser::DataType awaiting_type; + if (p_await->to_await->type == GDScriptParser::Node::CALL) { reduce_call(static_cast(p_await->to_await), true); + awaiting_type = p_await->to_await->get_datatype(); } else { reduce_expression(p_await->to_await); } - p_await->is_constant = p_await->to_await->is_constant; - p_await->reduced_value = p_await->to_await->reduced_value; + if (p_await->to_await->is_constant) { + p_await->is_constant = p_await->to_await->is_constant; + p_await->reduced_value = p_await->to_await->reduced_value; - GDScriptParser::DataType awaiting_type = p_await->to_await->get_datatype(); + awaiting_type = p_await->to_await->get_datatype(); + } else { + awaiting_type.kind = GDScriptParser::DataType::VARIANT; + awaiting_type.type_source = GDScriptParser::DataType::UNDETECTED; + } p_await->set_datatype(awaiting_type);