Merge pull request #27128 from bojidar-bg/27111-gdscript-confounding-class

Fix GDScriptCompiler bugging out with identically-named inner class

2 files changed, 70 insertions, 129 deletions

diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp
index 5f521c682a..ae67521749 100644
--- a/modules/gdscript/gdscript_compiler.cpp
+++ b/modules/gdscript/gdscript_compiler.cpp
@@ -139,17 +139,32 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
 			result.native_type = result.script_type->get_instance_base_type();
 		} break;
 		case GDScriptParser::DataType::CLASS: {
-			result.kind = GDScriptDataType::GDSCRIPT;
-			if (!p_datatype.class_type->owner) {
-				result.script_type = Ref<GDScript>(main_script);
-			} else {
-				result.script_type = class_map[p_datatype.class_type->name];
+			// Locate class by constructing the path to it and following that path
+			GDScriptParser::ClassNode *class_type = p_datatype.class_type;
+			List<StringName> names;
+			while (class_type->owner) {
+				names.push_back(class_type->name);
+				class_type = class_type->owner;
 			}
-			result.native_type = result.script_type->get_instance_base_type();
+
+			Ref<GDScript> script = Ref<GDScript>(main_script);
+			while (names.back()) {
+				if (!script->subclasses.has(names.back()->get())) {
+					ERR_PRINT("Parser bug: Cannot locate datatype class.");
+					result.has_type = false;
+					return GDScriptDataType();
+				}
+				script = script->subclasses[names.back()->get()];
+				names.pop_back();
+			}
+
+			result.kind = GDScriptDataType::GDSCRIPT;
+			result.script_type = script;
+			result.native_type = script->get_instance_base_type();
 		} break;
 		default: {
 			ERR_PRINT("Parser bug: converting unresolved type.");
-			result.has_type = false;
+			return GDScriptDataType();
 		}
 	}
 
@@ -460,12 +475,14 @@ int GDScriptCompiler::_parse_expression(CodeGen &codegen, const GDScriptParser::
 				codegen.alloc_stack(slevel);
 			}
 
-			switch (cn->cast_type.kind) {
-				case GDScriptParser::DataType::BUILTIN: {
+			GDScriptDataType cast_type = _gdtype_from_datatype(cn->cast_type);
+
+			switch (cast_type.kind) {
+				case GDScriptDataType::BUILTIN: {
 					codegen.opcodes.push_back(GDScriptFunction::OPCODE_CAST_TO_BUILTIN);
 					codegen.opcodes.push_back(cn->cast_type.builtin_type);
 				} break;
-				case GDScriptParser::DataType::NATIVE: {
+				case GDScriptDataType::NATIVE: {
 					int class_idx;
 					if (GDScriptLanguage::get_singleton()->get_global_map().has(cn->cast_type.native_type)) {
 
@@ -478,32 +495,8 @@ int GDScriptCompiler::_parse_expression(CodeGen &codegen, const GDScriptParser::
 					codegen.opcodes.push_back(GDScriptFunction::OPCODE_CAST_TO_NATIVE); // perform operator
 					codegen.opcodes.push_back(class_idx); // variable type
 				} break;
-				case GDScriptParser::DataType::CLASS: {
-
-					Variant script;
-					int idx = -1;
-					if (!cn->cast_type.class_type->owner) {
-						script = codegen.script;
-					} else {
-						StringName name = cn->cast_type.class_type->name;
-						if (codegen.script->subclasses.has(name) && class_map[name] == codegen.script->subclasses[name]) {
-							idx = codegen.get_name_map_pos(name);
-							idx |= GDScriptFunction::ADDR_TYPE_CLASS_CONSTANT << GDScriptFunction::ADDR_BITS;
-						} else {
-							script = class_map[name];
-						}
-					}
-
-					if (idx < 0) {
-						idx = codegen.get_constant_pos(script);
-						idx |= GDScriptFunction::ADDR_TYPE_LOCAL_CONSTANT << GDScriptFunction::ADDR_BITS; //make it a local constant (faster access)
-					}
-
-					codegen.opcodes.push_back(GDScriptFunction::OPCODE_CAST_TO_SCRIPT); // perform operator
-					codegen.opcodes.push_back(idx); // variable type
-				} break;
-				case GDScriptParser::DataType::SCRIPT:
-				case GDScriptParser::DataType::GDSCRIPT: {
+				case GDScriptDataType::SCRIPT:
+				case GDScriptDataType::GDSCRIPT: {
 
 					Variant script = cn->cast_type.script_type;
 					int idx = codegen.get_constant_pos(script);
@@ -1149,18 +1142,18 @@ int GDScriptCompiler::_parse_expression(CodeGen &codegen, const GDScriptParser::
 						if (src_address_b < 0)
 							return -1;
 
-						GDScriptParser::DataType assign_type = on->arguments[0]->get_datatype();
+						GDScriptDataType assign_type = _gdtype_from_datatype(on->arguments[0]->get_datatype());
 
 						if (assign_type.has_type && !on->arguments[1]->get_datatype().has_type) {
 							// Typed assignment
 							switch (assign_type.kind) {
-								case GDScriptParser::DataType::BUILTIN: {
+								case GDScriptDataType::BUILTIN: {
 									codegen.opcodes.push_back(GDScriptFunction::OPCODE_ASSIGN_TYPED_BUILTIN); // perform operator
 									codegen.opcodes.push_back(assign_type.builtin_type); // variable type
 									codegen.opcodes.push_back(dst_address_a); // argument 1
 									codegen.opcodes.push_back(src_address_b); // argument 2
 								} break;
-								case GDScriptParser::DataType::NATIVE: {
+								case GDScriptDataType::NATIVE: {
 									int class_idx;
 									if (GDScriptLanguage::get_singleton()->get_global_map().has(assign_type.native_type)) {
 
@@ -1175,34 +1168,8 @@ int GDScriptCompiler::_parse_expression(CodeGen &codegen, const GDScriptParser::
 									codegen.opcodes.push_back(dst_address_a); // argument 1
 									codegen.opcodes.push_back(src_address_b); // argument 2
 								} break;
-								case GDScriptParser::DataType::CLASS: {
-
-									Variant script;
-									int idx = -1;
-									if (!assign_type.class_type->owner) {
-										script = codegen.script;
-									} else {
-										StringName name = assign_type.class_type->name;
-										if (codegen.script->subclasses.has(name) && class_map[name] == codegen.script->subclasses[name]) {
-											idx = codegen.get_name_map_pos(name);
-											idx |= GDScriptFunction::ADDR_TYPE_CLASS_CONSTANT << GDScriptFunction::ADDR_BITS;
-										} else {
-											script = class_map[name];
-										}
-									}
-
-									if (idx < 0) {
-										idx = codegen.get_constant_pos(script);
-										idx |= GDScriptFunction::ADDR_TYPE_LOCAL_CONSTANT << GDScriptFunction::ADDR_BITS; //make it a local constant (faster access)
-									}
-
-									codegen.opcodes.push_back(GDScriptFunction::OPCODE_ASSIGN_TYPED_SCRIPT); // perform operator
-									codegen.opcodes.push_back(idx); // variable type
-									codegen.opcodes.push_back(dst_address_a); // argument 1
-									codegen.opcodes.push_back(src_address_b); // argument 2
-								} break;
-								case GDScriptParser::DataType::SCRIPT:
-								case GDScriptParser::DataType::GDSCRIPT: {
+								case GDScriptDataType::SCRIPT:
+								case GDScriptDataType::GDSCRIPT: {
 
 									Variant script = assign_type.script_type;
 									int idx = codegen.get_constant_pos(script);
@@ -1845,22 +1812,21 @@ Error GDScriptCompiler::_parse_function(GDScript *p_script, const GDScriptParser
 	return OK;
 }
 
-Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
+Error GDScriptCompiler::_parse_class_level(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
+
+	parsing_classes.insert(p_script);
 
 	if (p_class->owner && p_class->owner->owner) {
 		// Owner is not root
-		StringName owner_name = p_class->owner->name;
-		if (!parsed_classes.has(owner_name)) {
-			if (parsing_classes.has(owner_name)) {
-				_set_error("Cyclic class reference for '" + String(owner_name) + "'.", p_class);
+		if (!parsed_classes.has(p_script->_owner)) {
+			if (parsing_classes.has(p_script->_owner)) {
+				_set_error("Cyclic class reference for '" + String(p_class->name) + "'.", p_class);
 				return ERR_PARSE_ERROR;
 			}
-			parsing_classes.insert(owner_name);
-			Error err = _parse_class_level(class_map[owner_name].ptr(), class_map[owner_name]->_owner, p_class->owner, p_keep_state);
+			Error err = _parse_class_level(p_script->_owner, p_class->owner, p_keep_state);
 			if (err) {
 				return err;
 			}
-			parsing_classes.erase(owner_name);
 		}
 	}
 
@@ -1878,47 +1844,26 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner
 	p_script->_signals.clear();
 	p_script->initializer = NULL;
 
-	p_script->subclasses.clear();
-	p_script->_owner = p_owner;
 	p_script->tool = p_class->tool;
 	p_script->name = p_class->name;
 
 	Ref<GDScriptNativeClass> native;
 
+	GDScriptDataType base_type = _gdtype_from_datatype(p_class->base_type);
 	// Inheritance
-	switch (p_class->base_type.kind) {
-		case GDScriptParser::DataType::CLASS: {
-			StringName base_name = p_class->base_type.class_type->name;
-			// Make sure dependency is parsed first
-			if (!parsed_classes.has(base_name)) {
-				if (parsing_classes.has(base_name)) {
-					_set_error("Cyclic class reference for '" + String(base_name) + "'.", p_class);
-					return ERR_PARSE_ERROR;
-				}
-				parsing_classes.insert(base_name);
-				Error err = _parse_class_level(class_map[base_name].ptr(), class_map[base_name]->_owner, p_class->base_type.class_type, p_keep_state);
-				if (err) {
-					return err;
-				}
-				parsing_classes.erase(base_name);
-			}
-			Ref<GDScript> base = class_map[base_name];
-			p_script->base = base;
-			p_script->_base = p_script->base.ptr();
-			p_script->member_indices = base->member_indices;
-		} break;
-		case GDScriptParser::DataType::GDSCRIPT: {
-			Ref<GDScript> base = p_class->base_type.script_type;
-			p_script->base = base;
-			p_script->_base = p_script->base.ptr();
-			p_script->member_indices = base->member_indices;
-		} break;
-		case GDScriptParser::DataType::NATIVE: {
-			int native_idx = GDScriptLanguage::get_singleton()->get_global_map()[p_class->base_type.native_type];
+	switch (base_type.kind) {
+		case GDScriptDataType::NATIVE: {
+			int native_idx = GDScriptLanguage::get_singleton()->get_global_map()[base_type.native_type];
 			native = GDScriptLanguage::get_singleton()->get_global_array()[native_idx];
 			ERR_FAIL_COND_V(native.is_null(), ERR_BUG);
 			p_script->native = native;
 		} break;
+		case GDScriptDataType::GDSCRIPT: {
+			Ref<GDScript> base = base_type.script_type;
+			p_script->base = base;
+			p_script->_base = base.ptr();
+			p_script->member_indices = base->member_indices;
+		} break;
 		default: {
 			_set_error("Parser bug: invalid inheritance.", p_class);
 			return ERR_BUG;
@@ -2012,24 +1957,19 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner
 		p_script->_signals[name] = p_class->_signals[i].arguments;
 	}
 
-	if (p_class->owner) {
-		parsed_classes.insert(p_class->name);
-		if (parsing_classes.has(p_class->name)) {
-			parsing_classes.erase(p_class->name);
-		}
-	}
+	parsed_classes.insert(p_script);
+	parsing_classes.erase(p_script);
 
 	//parse sub-classes
 
 	for (int i = 0; i < p_class->subclasses.size(); i++) {
 		StringName name = p_class->subclasses[i]->name;
 
-		Ref<GDScript> subclass = class_map[name];
+		GDScript *subclass = p_script->subclasses[name].ptr();
 
 		// Subclass might still be parsing, just skip it
-		if (!parsed_classes.has(name) && !parsing_classes.has(name)) {
-			parsing_classes.insert(name);
-			Error err = _parse_class_level(subclass.ptr(), p_script, p_class->subclasses[i], p_keep_state);
+		if (!parsed_classes.has(subclass) && !parsing_classes.has(subclass)) {
+			Error err = _parse_class_level(subclass, p_class->subclasses[i], p_keep_state);
 			if (err)
 				return err;
 		}
@@ -2040,7 +1980,6 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner
 #endif
 
 		p_script->constants.insert(name, subclass); //once parsed, goes to the list of constants
-		p_script->subclasses.insert(name, subclass);
 	}
 
 	return OK;
@@ -2139,9 +2078,9 @@ Error GDScriptCompiler::_parse_class_blocks(GDScript *p_script, const GDScriptPa
 
 	for (int i = 0; i < p_class->subclasses.size(); i++) {
 		StringName name = p_class->subclasses[i]->name;
-		Ref<GDScript> subclass = class_map[name];
+		GDScript *subclass = p_script->subclasses[name].ptr();
 
-		Error err = _parse_class_blocks(subclass.ptr(), p_class->subclasses[i], p_keep_state);
+		Error err = _parse_class_blocks(subclass, p_class->subclasses[i], p_keep_state);
 		if (err) {
 			return err;
 		}
@@ -2151,7 +2090,7 @@ Error GDScriptCompiler::_parse_class_blocks(GDScript *p_script, const GDScriptPa
 	return OK;
 }
 
-void GDScriptCompiler::_make_scripts(const GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
+void GDScriptCompiler::_make_scripts(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
 
 	Map<StringName, Ref<GDScript> > old_subclasses;
 
@@ -2159,6 +2098,8 @@ void GDScriptCompiler::_make_scripts(const GDScript *p_script, const GDScriptPar
 		old_subclasses = p_script->subclasses;
 	}
 
+	p_script->subclasses.clear();
+
 	for (int i = 0; i < p_class->subclasses.size(); i++) {
 		StringName name = p_class->subclasses[i]->name;
 
@@ -2170,10 +2111,10 @@ void GDScriptCompiler::_make_scripts(const GDScript *p_script, const GDScriptPar
 			subclass.instance();
 		}
 
-		subclass->_owner = const_cast<GDScript *>(p_script);
-		class_map.insert(name, subclass);
+		subclass->_owner = p_script;
+		p_script->subclasses.insert(name, subclass);
 
-		_make_scripts(subclass.ptr(), p_class->subclasses[i], p_keep_state);
+		_make_scripts(subclass.ptr(), p_class->subclasses[i], false);
 	}
 }
 
@@ -2192,7 +2133,8 @@ Error GDScriptCompiler::compile(const GDScriptParser *p_parser, GDScript *p_scri
 	// Create scripts for subclasses beforehand so they can be referenced
 	_make_scripts(p_script, static_cast<const GDScriptParser::ClassNode *>(root), p_keep_state);
 
-	Error err = _parse_class_level(p_script, NULL, static_cast<const GDScriptParser::ClassNode *>(root), p_keep_state);
+	p_script->_owner = NULL;
+	Error err = _parse_class_level(p_script, static_cast<const GDScriptParser::ClassNode *>(root), p_keep_state);
 
 	if (err)
 		return err;
diff --git a/modules/gdscript/gdscript_compiler.h b/modules/gdscript/gdscript_compiler.h
index 8440807a56..2cf630ba72 100644
--- a/modules/gdscript/gdscript_compiler.h
+++ b/modules/gdscript/gdscript_compiler.h
@@ -38,9 +38,8 @@
 class GDScriptCompiler {
 
 	const GDScriptParser *parser;
-	Map<StringName, Ref<GDScript> > class_map;
-	Set<StringName> parsed_classes;
-	Set<StringName> parsing_classes;
+	Set<GDScript *> parsed_classes;
+	Set<GDScript *> parsing_classes;
 	GDScript *main_script;
 	struct CodeGen {
 
@@ -149,9 +148,9 @@ class GDScriptCompiler {
 	int _parse_expression(CodeGen &codegen, const GDScriptParser::Node *p_expression, int p_stack_level, bool p_root = false, bool p_initializer = false);
 	Error _parse_block(CodeGen &codegen, const GDScriptParser::BlockNode *p_block, int p_stack_level = 0, int p_break_addr = -1, int p_continue_addr = -1);
 	Error _parse_function(GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready = false);
-	Error _parse_class_level(GDScript *p_script, GDScript *p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
+	Error _parse_class_level(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
 	Error _parse_class_blocks(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
-	void _make_scripts(const GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
+	void _make_scripts(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
 	int err_line;
 	int err_column;
 	StringName source;