Merge pull request #71107 from vnen/gdscript-fix-nil-address-assign

GDScript: Don't use the NIL address to hold return value of functions

modules/gdscript/gdscript_analyzer.cpp

@@ -2466,7 +2466,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 		} else if (GDScriptUtilityFunctions::function_exists(function_name)) {
 			MethodInfo function_info = GDScriptUtilityFunctions::get_function_info(function_name);
 
-			if (!p_is_root && function_info.return_val.type == Variant::NIL && ((function_info.return_val.usage & PROPERTY_USAGE_NIL_IS_VARIANT) == 0)) {
+			if (!p_is_root && !p_is_await && function_info.return_val.type == Variant::NIL && ((function_info.return_val.usage & PROPERTY_USAGE_NIL_IS_VARIANT) == 0)) {
 				push_error(vformat(R"*(Cannot get return value of call to "%s()" because it returns "void".)*", function_name), p_call);
 			}
 
@@ -2513,7 +2513,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 		} else if (Variant::has_utility_function(function_name)) {
 			MethodInfo function_info = info_from_utility_func(function_name);
 
-			if (!p_is_root && function_info.return_val.type == Variant::NIL && ((function_info.return_val.usage & PROPERTY_USAGE_NIL_IS_VARIANT) == 0)) {
+			if (!p_is_root && !p_is_await && function_info.return_val.type == Variant::NIL && ((function_info.return_val.usage & PROPERTY_USAGE_NIL_IS_VARIANT) == 0)) {
 				push_error(vformat(R"*(Cannot get return value of call to "%s()" because it returns "void".)*", function_name), p_call);
 			}
 
@@ -2659,7 +2659,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 			mark_lambda_use_self();
 		}
 
-		if (!p_is_root && return_type.is_hard_type() && return_type.kind == GDScriptParser::DataType::BUILTIN && return_type.builtin_type == Variant::NIL) {
+		if (!p_is_root && !p_is_await && return_type.is_hard_type() && return_type.kind == GDScriptParser::DataType::BUILTIN && return_type.builtin_type == Variant::NIL) {
 			push_error(vformat(R"*(Cannot get return value of call to "%s()" because it returns "void".)*", p_call->function_name), p_call);
 		}
 

modules/gdscript/gdscript_byte_codegen.cpp

@@ -920,13 +920,23 @@ void GDScriptByteCodeGenerator::write_cast(const Address &p_target, const Addres
 	append(index);
 }
 
+GDScriptCodeGenerator::Address GDScriptByteCodeGenerator::get_call_target(const GDScriptCodeGenerator::Address &p_target) {
+	if (p_target.mode == Address::NIL) {
+		uint32_t addr = add_temporary(p_target.type);
+		pop_temporary();
+		return Address(Address::TEMPORARY, addr, p_target.type);
+	} else {
+		return p_target;
+	}
+}
+
 void GDScriptByteCodeGenerator::write_call(const Address &p_target, const Address &p_base, const StringName &p_function_name, const Vector<Address> &p_arguments) {
 	append_opcode_and_argcount(p_target.mode == Address::NIL ? GDScriptFunction::OPCODE_CALL : GDScriptFunction::OPCODE_CALL_RETURN, 2 + p_arguments.size());
 	for (int i = 0; i < p_arguments.size(); i++) {
 		append(p_arguments[i]);
 	}
 	append(p_base);
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_function_name);
 }
@@ -936,7 +946,7 @@ void GDScriptByteCodeGenerator::write_super_call(const Address &p_target, const
 	for (int i = 0; i < p_arguments.size(); i++) {
 		append(p_arguments[i]);
 	}
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_function_name);
 }
@@ -947,7 +957,7 @@ void GDScriptByteCodeGenerator::write_call_async(const Address &p_target, const
 		append(p_arguments[i]);
 	}
 	append(p_base);
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_function_name);
 }
@@ -957,7 +967,7 @@ void GDScriptByteCodeGenerator::write_call_gdscript_utility(const Address &p_tar
 	for (int i = 0; i < p_arguments.size(); i++) {
 		append(p_arguments[i]);
 	}
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_function);
 }
@@ -983,7 +993,7 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
 		for (int i = 0; i < p_arguments.size(); i++) {
 			append(p_arguments[i]);
 		}
-		append(p_target);
+		append(get_call_target(p_target));
 		append(p_arguments.size());
 		append(Variant::get_validated_utility_function(p_function));
 	} else {
@@ -991,7 +1001,7 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
 		for (int i = 0; i < p_arguments.size(); i++) {
 			append(p_arguments[i]);
 		}
-		append(p_target);
+		append(get_call_target(p_target));
 		append(p_arguments.size());
 		append(p_function);
 	}
@@ -1035,7 +1045,7 @@ void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target,
 		append(p_arguments[i]);
 	}
 	append(p_base);
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(Variant::get_validated_builtin_method(p_type, p_method));
 }
@@ -1064,7 +1074,7 @@ void GDScriptByteCodeGenerator::write_call_builtin_type_static(const Address &p_
 		for (int i = 0; i < p_arguments.size(); i++) {
 			append(p_arguments[i]);
 		}
-		append(p_target);
+		append(get_call_target(p_target));
 		append(p_type);
 		append(p_method);
 		append(p_arguments.size());
@@ -1085,7 +1095,7 @@ void GDScriptByteCodeGenerator::write_call_builtin_type_static(const Address &p_
 		append(p_arguments[i]);
 	}
 	append(Address()); // No base since it's static.
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(Variant::get_validated_builtin_method(p_type, p_method));
 }
@@ -1101,7 +1111,7 @@ void GDScriptByteCodeGenerator::write_call_native_static(const Address &p_target
 		for (int i = 0; i < p_arguments.size(); i++) {
 			append(p_arguments[i]);
 		}
-		append(p_target);
+		append(get_call_target(p_target));
 		append(method);
 		append(p_arguments.size());
 		return;
@@ -1114,7 +1124,7 @@ void GDScriptByteCodeGenerator::write_call_method_bind(const Address &p_target,
 		append(p_arguments[i]);
 	}
 	append(p_base);
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_method);
 }
@@ -1178,7 +1188,7 @@ void GDScriptByteCodeGenerator::write_call_ptrcall(const Address &p_target, cons
 		append(p_arguments[i]);
 	}
 	append(p_base);
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_method);
 	if (is_ptrcall) {
@@ -1194,7 +1204,7 @@ void GDScriptByteCodeGenerator::write_call_self(const Address &p_target, const S
 		append(p_arguments[i]);
 	}
 	append(GDScriptFunction::ADDR_TYPE_STACK << GDScriptFunction::ADDR_BITS);
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_function_name);
 }
@@ -1205,7 +1215,7 @@ void GDScriptByteCodeGenerator::write_call_self_async(const Address &p_target, c
 		append(p_arguments[i]);
 	}
 	append(GDScriptFunction::ADDR_SELF);
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_function_name);
 }
@@ -1216,7 +1226,7 @@ void GDScriptByteCodeGenerator::write_call_script_function(const Address &p_targ
 		append(p_arguments[i]);
 	}
 	append(p_base);
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_function_name);
 }
@@ -1227,7 +1237,7 @@ void GDScriptByteCodeGenerator::write_lambda(const Address &p_target, GDScriptFu
 		append(p_captures[i]);
 	}
 
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_captures.size());
 	append(p_function);
 }
@@ -1266,7 +1276,7 @@ void GDScriptByteCodeGenerator::write_construct(const Address &p_target, Variant
 			for (int i = 0; i < p_arguments.size(); i++) {
 				append(p_arguments[i]);
 			}
-			append(p_target);
+			append(get_call_target(p_target));
 			append(p_arguments.size());
 			append(Variant::get_validated_constructor(p_type, valid_constructor));
 			return;
@@ -1277,7 +1287,7 @@ void GDScriptByteCodeGenerator::write_construct(const Address &p_target, Variant
 	for (int i = 0; i < p_arguments.size(); i++) {
 		append(p_arguments[i]);
 	}
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 	append(p_type);
 }
@@ -1287,7 +1297,7 @@ void GDScriptByteCodeGenerator::write_construct_array(const Address &p_target, c
 	for (int i = 0; i < p_arguments.size(); i++) {
 		append(p_arguments[i]);
 	}
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size());
 }
 
@@ -1296,7 +1306,7 @@ void GDScriptByteCodeGenerator::write_construct_typed_array(const Address &p_tar
 	for (int i = 0; i < p_arguments.size(); i++) {
 		append(p_arguments[i]);
 	}
-	append(p_target);
+	append(get_call_target(p_target));
 	if (p_element_type.script_type) {
 		Variant script_type = Ref<Script>(p_element_type.script_type);
 		int addr = get_constant_pos(script_type);
@@ -1315,7 +1325,7 @@ void GDScriptByteCodeGenerator::write_construct_dictionary(const Address &p_targ
 	for (int i = 0; i < p_arguments.size(); i++) {
 		append(p_arguments[i]);
 	}
-	append(p_target);
+	append(get_call_target(p_target));
 	append(p_arguments.size() / 2); // This is number of key-value pairs, so only half of actual arguments.
 }
 

modules/gdscript/gdscript_byte_codegen.h

@@ -309,6 +309,8 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
 		}
 	}
 
+	Address get_call_target(const Address &p_target);
+
 	int address_of(const Address &p_address) {
 		switch (p_address.mode) {
 			case Address::SELF:

modules/gdscript/gdscript_compiler.cpp

@@ -520,10 +520,12 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 		case GDScriptParser::Node::CALL: {
 			const GDScriptParser::CallNode *call = static_cast<const GDScriptParser::CallNode *>(p_expression);
 			GDScriptDataType type = _gdtype_from_datatype(call->get_datatype(), codegen.script);
-			GDScriptCodeGenerator::Address result = codegen.add_temporary(type);
-			GDScriptCodeGenerator::Address nil = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::NIL);
-
-			GDScriptCodeGenerator::Address return_addr = p_root ? nil : result;
+			GDScriptCodeGenerator::Address result;
+			if (p_root) {
+				result = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::NIL);
+			} else {
+				result = codegen.add_temporary(type);
+			}
 
 			Vector<GDScriptCodeGenerator::Address> arguments;
 			for (int i = 0; i < call->arguments.size(); i++) {
@@ -538,20 +540,20 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 				// Construct a built-in type.
 				Variant::Type vtype = GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(call->callee)->name);
 
-				gen->write_construct(return_addr, vtype, arguments);
+				gen->write_construct(result, vtype, arguments);
 			} else if (!call->is_super && call->callee->type == GDScriptParser::Node::IDENTIFIER && Variant::has_utility_function(call->function_name)) {
 				// Variant utility function.
-				gen->write_call_utility(return_addr, call->function_name, arguments);
+				gen->write_call_utility(result, call->function_name, arguments);
 			} else if (!call->is_super && call->callee->type == GDScriptParser::Node::IDENTIFIER && GDScriptUtilityFunctions::function_exists(call->function_name)) {
 				// GDScript utility function.
-				gen->write_call_gdscript_utility(return_addr, GDScriptUtilityFunctions::get_function(call->function_name), arguments);
+				gen->write_call_gdscript_utility(result, GDScriptUtilityFunctions::get_function(call->function_name), arguments);
 			} else {
 				// Regular function.
 				const GDScriptParser::ExpressionNode *callee = call->callee;
 
 				if (call->is_super) {
 					// Super call.
-					gen->write_super_call(return_addr, call->function_name, arguments);
+					gen->write_super_call(result, call->function_name, arguments);
 				} else {
 					if (callee->type == GDScriptParser::Node::IDENTIFIER) {
 						// Self function call.
@@ -563,24 +565,24 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 
 							if (_have_exact_arguments(method, arguments)) {
 								// Exact arguments, use ptrcall.
-								gen->write_call_ptrcall(return_addr, self, method, arguments);
+								gen->write_call_ptrcall(result, self, method, arguments);
 							} else {
 								// Not exact arguments, but still can use method bind call.
-								gen->write_call_method_bind(return_addr, self, method, arguments);
+								gen->write_call_method_bind(result, self, method, arguments);
 							}
 						} else if ((codegen.function_node && codegen.function_node->is_static) || call->function_name == "new") {
 							GDScriptCodeGenerator::Address self;
 							self.mode = GDScriptCodeGenerator::Address::CLASS;
 							if (within_await) {
-								gen->write_call_async(return_addr, self, call->function_name, arguments);
+								gen->write_call_async(result, self, call->function_name, arguments);
 							} else {
-								gen->write_call(return_addr, self, call->function_name, arguments);
+								gen->write_call(result, self, call->function_name, arguments);
 							}
 						} else {
 							if (within_await) {
-								gen->write_call_self_async(return_addr, call->function_name, arguments);
+								gen->write_call_self_async(result, call->function_name, arguments);
 							} else {
-								gen->write_call_self(return_addr, call->function_name, arguments);
+								gen->write_call_self(result, call->function_name, arguments);
 							}
 						}
 					} else if (callee->type == GDScriptParser::Node::SUBSCRIPT) {
@@ -589,18 +591,18 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 						if (subscript->is_attribute) {
 							// May be static built-in method call.
 							if (!call->is_super && subscript->base->type == GDScriptParser::Node::IDENTIFIER && GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name) < Variant::VARIANT_MAX) {
-								gen->write_call_builtin_type_static(return_addr, GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name), subscript->attribute->name, arguments);
+								gen->write_call_builtin_type_static(result, GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name), subscript->attribute->name, arguments);
 							} else if (!call->is_super && subscript->base->type == GDScriptParser::Node::IDENTIFIER && call->function_name != SNAME("new") &&
 									ClassDB::class_exists(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name) && !Engine::get_singleton()->has_singleton(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name)) {
 								// It's a static native method call.
-								gen->write_call_native_static(return_addr, static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name, subscript->attribute->name, arguments);
+								gen->write_call_native_static(result, static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name, subscript->attribute->name, arguments);
 							} else {
 								GDScriptCodeGenerator::Address base = _parse_expression(codegen, r_error, subscript->base);
 								if (r_error) {
 									return GDScriptCodeGenerator::Address();
 								}
 								if (within_await) {
-									gen->write_call_async(return_addr, base, call->function_name, arguments);
+									gen->write_call_async(result, base, call->function_name, arguments);
 								} else if (base.type.has_type && base.type.kind != GDScriptDataType::BUILTIN) {
 									// Native method, use faster path.
 									StringName class_name;
@@ -613,18 +615,18 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 										MethodBind *method = ClassDB::get_method(class_name, call->function_name);
 										if (_have_exact_arguments(method, arguments)) {
 											// Exact arguments, use ptrcall.
-											gen->write_call_ptrcall(return_addr, base, method, arguments);
+											gen->write_call_ptrcall(result, base, method, arguments);
 										} else {
 											// Not exact arguments, but still can use method bind call.
-											gen->write_call_method_bind(return_addr, base, method, arguments);
+											gen->write_call_method_bind(result, base, method, arguments);
 										}
 									} else {
-										gen->write_call(return_addr, base, call->function_name, arguments);
+										gen->write_call(result, base, call->function_name, arguments);
 									}
 								} else if (base.type.has_type && base.type.kind == GDScriptDataType::BUILTIN) {
-									gen->write_call_builtin_type(return_addr, base, base.type.builtin_type, call->function_name, arguments);
+									gen->write_call_builtin_type(result, base, base.type.builtin_type, call->function_name, arguments);
 								} else {
-									gen->write_call(return_addr, base, call->function_name, arguments);
+									gen->write_call(result, base, call->function_name, arguments);
 								}
 								if (base.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
 									gen->pop_temporary();

modules/gdscript/tests/scripts/runtime/features/await_on_void.gd

@@ -0,0 +1,7 @@
+func wait() -> void:
+	pass
+
+func test():
+	@warning_ignore(redundant_await)
+	await wait()
+	print("end")

modules/gdscript/tests/scripts/runtime/features/await_on_void.out

@@ -0,0 +1,2 @@
+GDTEST_OK
+end

modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.gd

@@ -0,0 +1,45 @@
+# https://github.com/godotengine/godot/issues/70964
+
+func test():
+	test_construct(0, false)
+	test_utility(0, false)
+	test_builtin_call(Vector2.UP, false)
+	test_builtin_call_validated(Vector2.UP, false)
+	test_object_call(RefCounted.new(), false)
+	test_object_call_method_bind(Resource.new(), false)
+	test_object_call_ptrcall(RefCounted.new(), false)
+
+	print("end")
+
+func test_construct(v, f):
+	Vector2(v, v) # Built-in type construct.
+	assert(not f) # Test unary operator reading from `nil`.
+
+func test_utility(v, f):
+	abs(v) # Utility function.
+	assert(not f) # Test unary operator reading from `nil`.
+
+func test_builtin_call(v, f):
+	@warning_ignore(unsafe_method_access)
+	v.angle() # Built-in method call.
+	assert(not f) # Test unary operator reading from `nil`.
+
+func test_builtin_call_validated(v: Vector2, f):
+	@warning_ignore(return_value_discarded)
+	v.abs() # Built-in method call validated.
+	assert(not f) # Test unary operator reading from `nil`.
+
+func test_object_call(v, f):
+	@warning_ignore(unsafe_method_access)
+	v.get_reference_count() # Native type method call.
+	assert(not f) # Test unary operator reading from `nil`.
+
+func test_object_call_method_bind(v: Resource, f):
+	@warning_ignore(return_value_discarded)
+	v.duplicate() # Native type method call with MethodBind.
+	assert(not f) # Test unary operator reading from `nil`.
+
+func test_object_call_ptrcall(v: RefCounted, f):
+	@warning_ignore(return_value_discarded)
+	v.get_reference_count() # Native type method call with ptrcall.
+	assert(not f) # Test unary operator reading from `nil`.

modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.out

@@ -0,0 +1,2 @@
+GDTEST_OK
+end