Merge pull request #49112 from vnen/gdscript-assign-type-check

GDScript: Use analyzer data to decide assignment conversion

6 files changed, 74 insertions, 51 deletions

diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 7b04959227..da0ce251a6 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -543,6 +543,7 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas
 							} else {
 								// TODO: Add warning.
 								mark_node_unsafe(member.variable->initializer);
+								member.variable->use_conversion_assign = true;
 							}
 						} else if (datatype.builtin_type == Variant::INT && member.variable->initializer->get_datatype().builtin_type == Variant::FLOAT) {
 #ifdef DEBUG_ENABLED
@@ -552,6 +553,7 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas
 						if (member.variable->initializer->get_datatype().is_variant()) {
 							// TODO: Warn unsafe assign.
 							mark_node_unsafe(member.variable->initializer);
+							member.variable->use_conversion_assign = true;
 						}
 					}
 				} else if (member.variable->infer_datatype) {
@@ -1145,6 +1147,7 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable
 				} else {
 					// TODO: Add warning.
 					mark_node_unsafe(p_variable->initializer);
+					p_variable->use_conversion_assign = true;
 				}
 #ifdef DEBUG_ENABLED
 			} else if (type.builtin_type == Variant::INT && p_variable->initializer->get_datatype().builtin_type == Variant::FLOAT) {
@@ -1154,6 +1157,7 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable
 			if (p_variable->initializer->get_datatype().is_variant()) {
 				// TODO: Warn unsafe assign.
 				mark_node_unsafe(p_variable->initializer);
+				p_variable->use_conversion_assign = true;
 			}
 		}
 	} else if (p_variable->infer_datatype) {
@@ -1608,10 +1612,12 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 					} else {
 						// TODO: Add warning.
 						mark_node_unsafe(p_assignment);
+						p_assignment->use_conversion_assign = true;
 					}
 				} else {
 					// TODO: Warning in this case.
 					mark_node_unsafe(p_assignment);
+					p_assignment->use_conversion_assign = true;
 				}
 			}
 		} else {
@@ -1621,6 +1627,9 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 
 	if (assignee_type.has_no_type() || assigned_value_type.is_variant()) {
 		mark_node_unsafe(p_assignment);
+		if (assignee_type.is_hard_type()) {
+			p_assignment->use_conversion_assign = true;
+		}
 	}
 
 	if (p_assignment->assignee->type == GDScriptParser::Node::IDENTIFIER) {
diff --git a/modules/gdscript/gdscript_byte_codegen.cpp b/modules/gdscript/gdscript_byte_codegen.cpp
index ea34a2ca2d..77a972ef12 100644
--- a/modules/gdscript/gdscript_byte_codegen.cpp
+++ b/modules/gdscript/gdscript_byte_codegen.cpp
@@ -779,63 +779,43 @@ void GDScriptByteCodeGenerator::write_get_member(const Address &p_target, const
 	append(p_name);
 }
 
-void GDScriptByteCodeGenerator::write_assign(const Address &p_target, const Address &p_source) {
-	if (p_target.type.has_type && !p_source.type.has_type) {
-		// Typed assignment.
-		switch (p_target.type.kind) {
-			case GDScriptDataType::BUILTIN: {
-				if (p_target.type.builtin_type == Variant::ARRAY && p_target.type.has_container_element_type()) {
-					append(GDScriptFunction::OPCODE_ASSIGN_TYPED_ARRAY, 2);
-					append(p_target);
-					append(p_source);
-				} else {
-					append(GDScriptFunction::OPCODE_ASSIGN_TYPED_BUILTIN, 2);
-					append(p_target);
-					append(p_source);
-					append(p_target.type.builtin_type);
-				}
-			} break;
-			case GDScriptDataType::NATIVE: {
-				int class_idx = GDScriptLanguage::get_singleton()->get_global_map()[p_target.type.native_type];
-				Variant nc = GDScriptLanguage::get_singleton()->get_global_array()[class_idx];
-				class_idx = get_constant_pos(nc) | (GDScriptFunction::ADDR_TYPE_CONSTANT << GDScriptFunction::ADDR_BITS);
-				append(GDScriptFunction::OPCODE_ASSIGN_TYPED_NATIVE, 3);
-				append(p_target);
-				append(p_source);
-				append(class_idx);
-			} break;
-			case GDScriptDataType::SCRIPT:
-			case GDScriptDataType::GDSCRIPT: {
-				Variant script = p_target.type.script_type;
-				int idx = get_constant_pos(script) | (GDScriptFunction::ADDR_TYPE_CONSTANT << GDScriptFunction::ADDR_BITS);
-
-				append(GDScriptFunction::OPCODE_ASSIGN_TYPED_SCRIPT, 3);
+void GDScriptByteCodeGenerator::write_assign_with_conversion(const Address &p_target, const Address &p_source) {
+	switch (p_target.type.kind) {
+		case GDScriptDataType::BUILTIN: {
+			if (p_target.type.builtin_type == Variant::ARRAY && p_target.type.has_container_element_type()) {
+				append(GDScriptFunction::OPCODE_ASSIGN_TYPED_ARRAY, 2);
 				append(p_target);
 				append(p_source);
-				append(idx);
-			} break;
-			default: {
-				ERR_PRINT("Compiler bug: unresolved assign.");
-
-				// Shouldn't get here, but fail-safe to a regular assignment
-				append(GDScriptFunction::OPCODE_ASSIGN, 2);
+			} else {
+				append(GDScriptFunction::OPCODE_ASSIGN_TYPED_BUILTIN, 2);
 				append(p_target);
 				append(p_source);
+				append(p_target.type.builtin_type);
 			}
-		}
-	} else {
-		if (p_target.type.kind == GDScriptDataType::BUILTIN && p_target.type.builtin_type == Variant::ARRAY && p_target.type.has_container_element_type()) {
-			append(GDScriptFunction::OPCODE_ASSIGN_TYPED_ARRAY, 2);
+		} break;
+		case GDScriptDataType::NATIVE: {
+			int class_idx = GDScriptLanguage::get_singleton()->get_global_map()[p_target.type.native_type];
+			Variant nc = GDScriptLanguage::get_singleton()->get_global_array()[class_idx];
+			class_idx = get_constant_pos(nc) | (GDScriptFunction::ADDR_TYPE_CONSTANT << GDScriptFunction::ADDR_BITS);
+			append(GDScriptFunction::OPCODE_ASSIGN_TYPED_NATIVE, 3);
 			append(p_target);
 			append(p_source);
-		} else if (p_target.type.kind == GDScriptDataType::BUILTIN && p_source.type.kind == GDScriptDataType::BUILTIN && p_target.type.builtin_type != p_source.type.builtin_type) {
-			// Need conversion..
-			append(GDScriptFunction::OPCODE_ASSIGN_TYPED_BUILTIN, 2);
+			append(class_idx);
+		} break;
+		case GDScriptDataType::SCRIPT:
+		case GDScriptDataType::GDSCRIPT: {
+			Variant script = p_target.type.script_type;
+			int idx = get_constant_pos(script) | (GDScriptFunction::ADDR_TYPE_CONSTANT << GDScriptFunction::ADDR_BITS);
+
+			append(GDScriptFunction::OPCODE_ASSIGN_TYPED_SCRIPT, 3);
 			append(p_target);
 			append(p_source);
-			append(p_target.type.builtin_type);
-		} else {
-			// Either untyped assignment or already type-checked by the parser
+			append(idx);
+		} break;
+		default: {
+			ERR_PRINT("Compiler bug: unresolved assign.");
+
+			// Shouldn't get here, but fail-safe to a regular assignment
 			append(GDScriptFunction::OPCODE_ASSIGN, 2);
 			append(p_target);
 			append(p_source);
@@ -843,6 +823,24 @@ void GDScriptByteCodeGenerator::write_assign(const Address &p_target, const Addr
 	}
 }
 
+void GDScriptByteCodeGenerator::write_assign(const Address &p_target, const Address &p_source) {
+	if (p_target.type.kind == GDScriptDataType::BUILTIN && p_target.type.builtin_type == Variant::ARRAY && p_target.type.has_container_element_type()) {
+		append(GDScriptFunction::OPCODE_ASSIGN_TYPED_ARRAY, 2);
+		append(p_target);
+		append(p_source);
+	} else if (p_target.type.kind == GDScriptDataType::BUILTIN && p_source.type.kind == GDScriptDataType::BUILTIN && p_target.type.builtin_type != p_source.type.builtin_type) {
+		// Need conversion.
+		append(GDScriptFunction::OPCODE_ASSIGN_TYPED_BUILTIN, 2);
+		append(p_target);
+		append(p_source);
+		append(p_target.type.builtin_type);
+	} else {
+		append(GDScriptFunction::OPCODE_ASSIGN, 2);
+		append(p_target);
+		append(p_source);
+	}
+}
+
 void GDScriptByteCodeGenerator::write_assign_true(const Address &p_target) {
 	append(GDScriptFunction::OPCODE_ASSIGN_TRUE, 1);
 	append(p_target);
diff --git a/modules/gdscript/gdscript_byte_codegen.h b/modules/gdscript/gdscript_byte_codegen.h
index f8c05fea83..b1f3cd5fb3 100644
--- a/modules/gdscript/gdscript_byte_codegen.h
+++ b/modules/gdscript/gdscript_byte_codegen.h
@@ -450,6 +450,7 @@ public:
 	virtual void write_set_member(const Address &p_value, const StringName &p_name) override;
 	virtual void write_get_member(const Address &p_target, const StringName &p_name) override;
 	virtual void write_assign(const Address &p_target, const Address &p_source) override;
+	virtual void write_assign_with_conversion(const Address &p_target, const Address &p_source) override;
 	virtual void write_assign_true(const Address &p_target) override;
 	virtual void write_assign_false(const Address &p_target) override;
 	virtual void write_assign_default_parameter(const Address &p_dst, const Address &p_src) override;
diff --git a/modules/gdscript/gdscript_codegen.h b/modules/gdscript/gdscript_codegen.h
index 399c9d6de7..cac6544f03 100644
--- a/modules/gdscript/gdscript_codegen.h
+++ b/modules/gdscript/gdscript_codegen.h
@@ -111,6 +111,7 @@ public:
 	virtual void write_set_member(const Address &p_value, const StringName &p_name) = 0;
 	virtual void write_get_member(const Address &p_target, const StringName &p_name) = 0;
 	virtual void write_assign(const Address &p_target, const Address &p_source) = 0;
+	virtual void write_assign_with_conversion(const Address &p_target, const Address &p_source) = 0;
 	virtual void write_assign_true(const Address &p_target) = 0;
 	virtual void write_assign_false(const Address &p_target) = 0;
 	virtual void write_assign_default_parameter(const Address &dst, const Address &src) = 0;
diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp
index 4ac9864d4f..179316b97e 100644
--- a/modules/gdscript/gdscript_compiler.cpp
+++ b/modules/gdscript/gdscript_compiler.cpp
@@ -1084,7 +1084,11 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 					gen->write_call(GDScriptCodeGenerator::Address(), GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::SELF), setter_function, args);
 				} else {
 					// Just assign.
-					gen->write_assign(target, op_result);
+					if (assignment->use_conversion_assign) {
+						gen->write_assign_with_conversion(target, op_result);
+					} else {
+						gen->write_assign(target, op_result);
+					}
 				}
 
 				if (op_result.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
@@ -1792,7 +1796,11 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
 					if (error) {
 						return error;
 					}
-					gen->write_assign(local, src_address);
+					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();
 					}
@@ -1930,7 +1938,11 @@ GDScriptFunction *GDScriptCompiler::_parse_function(Error &r_error, GDScript *p_
 					return nullptr;
 				}
 
-				codegen.generator->write_assign(dst_address, src_address);
+				if (field->use_conversion_assign) {
+					codegen.generator->write_assign_with_conversion(dst_address, src_address);
+				} else {
+					codegen.generator->write_assign(dst_address, src_address);
+				}
 				if (src_address.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
 					codegen.generator->pop_temporary();
 				}
diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h
index b1b29a7bd1..ee5e411cad 100644
--- a/modules/gdscript/gdscript_parser.h
+++ b/modules/gdscript/gdscript_parser.h
@@ -370,6 +370,7 @@ public:
 		Variant::Operator variant_op = Variant::OP_MAX;
 		ExpressionNode *assignee = nullptr;
 		ExpressionNode *assigned_value = nullptr;
+		bool use_conversion_assign = false;
 
 		AssignmentNode() {
 			type = ASSIGNMENT;
@@ -1119,6 +1120,7 @@ public:
 		MultiplayerAPI::RPCMode rpc_mode = MultiplayerAPI::RPC_MODE_DISABLED;
 		int assignments = 0;
 		int usages = 0;
+		bool use_conversion_assign = false;
 #ifdef TOOLS_ENABLED
 		String doc_description;
 #endif // TOOLS_ENABLED