Merge pull request #41547 from vnen/gdscript-2-fixes

Some more GDScript fixes

8 files changed, 89 insertions, 49 deletions

diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp
index 541d46a2af..0263e32c5b 100644
--- a/modules/gdscript/gdscript.cpp
+++ b/modules/gdscript/gdscript.cpp
@@ -603,10 +603,8 @@ Error GDScript::reload(bool p_keep_state) {
 		}
 		if (!source_path.empty()) {
 			MutexLock lock(GDScriptCache::singleton->lock);
-			Ref<GDScript> self(this);
 			if (!GDScriptCache::singleton->shallow_gdscript_cache.has(source_path)) {
-				GDScriptCache::singleton->shallow_gdscript_cache[source_path] = self;
-				self->unreference();
+				GDScriptCache::singleton->shallow_gdscript_cache[source_path] = this;
 			}
 		}
 	}
diff --git a/modules/gdscript/gdscript.h b/modules/gdscript/gdscript.h
index 9906b4014d..79317ff846 100644
--- a/modules/gdscript/gdscript.h
+++ b/modules/gdscript/gdscript.h
@@ -69,6 +69,7 @@ class GDScript : public Script {
 
 	friend class GDScriptInstance;
 	friend class GDScriptFunction;
+	friend class GDScriptAnalyzer;
 	friend class GDScriptCompiler;
 	friend class GDScriptFunctions;
 	friend class GDScriptLanguage;
diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 3253c8e945..cabe096579 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -182,7 +182,7 @@ Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class,
 				return ERR_PARSE_ERROR;
 			}
 
-			Error err = parser->raise_status(GDScriptParserRef::INHERITANCE_SOLVED);
+			Error err = parser->raise_status(GDScriptParserRef::INTERFACE_SOLVED);
 			if (err != OK) {
 				push_error(vformat(R"(Could not resolve super class inheritance from "%s".)", p_class->extends_path), p_class);
 				return err;
@@ -208,7 +208,7 @@ Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class,
 						return ERR_PARSE_ERROR;
 					}
 
-					Error err = parser->raise_status(GDScriptParserRef::INHERITANCE_SOLVED);
+					Error err = parser->raise_status(GDScriptParserRef::INTERFACE_SOLVED);
 					if (err != OK) {
 						push_error(vformat(R"(Could not resolve super class inheritance from "%s".)", name), p_class);
 						return err;
@@ -227,7 +227,7 @@ Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class,
 					return ERR_PARSE_ERROR;
 				}
 
-				Error err = parser->raise_status(GDScriptParserRef::INHERITANCE_SOLVED);
+				Error err = parser->raise_status(GDScriptParserRef::INTERFACE_SOLVED);
 				if (err != OK) {
 					push_error(vformat(R"(Could not resolve super class inheritance from "%s".)", name), p_class);
 					return err;
@@ -302,6 +302,16 @@ Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class,
 		return ERR_PARSE_ERROR;
 	}
 
+	// Check for cyclic inheritance.
+	const GDScriptParser::ClassNode *base_class = result.class_type;
+	while (base_class) {
+		if (base_class->fqcn == p_class->fqcn) {
+			push_error("Cyclic inheritance.", p_class);
+			return ERR_PARSE_ERROR;
+		}
+		base_class = base_class->base_type.class_type;
+	}
+
 	p_class->base_type = result;
 	class_type.native_type = result.native_type;
 	p_class->set_datatype(class_type);
@@ -309,7 +319,10 @@ Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class,
 	if (p_recursive) {
 		for (int i = 0; i < p_class->members.size(); i++) {
 			if (p_class->members[i].type == GDScriptParser::ClassNode::Member::CLASS) {
-				resolve_inheritance(p_class->members[i].m_class, true);
+				Error err = resolve_inheritance(p_class->members[i].m_class, true);
+				if (err) {
+					return err;
+				}
 			}
 		}
 	}
@@ -2702,9 +2715,27 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_variant(const Variant &p_va
 			Ref<GDScript> gds = scr;
 			if (gds.is_valid()) {
 				result.kind = GDScriptParser::DataType::CLASS;
-				Ref<GDScriptParserRef> ref = get_parser_for(gds->get_path());
+				// This might be an inner class, so we want to get the parser for the root.
+				// But still get the inner class from that tree.
+				GDScript *current = gds.ptr();
+				List<StringName> class_chain;
+				while (current->_owner) {
+					// Push to front so it's in reverse.
+					class_chain.push_front(current->name);
+					current = current->_owner;
+				}
+
+				Ref<GDScriptParserRef> ref = get_parser_for(current->path);
 				ref->raise_status(GDScriptParserRef::INTERFACE_SOLVED);
-				result.class_type = ref->get_parser()->head;
+
+				GDScriptParser::ClassNode *found = ref->get_parser()->head;
+
+				// It should be okay to assume this exists, since we have a complete script already.
+				for (const List<StringName>::Element *E = class_chain.front(); E; E = E->next()) {
+					found = found->get_member(E->get()).m_class;
+				}
+
+				result.class_type = found;
 				result.script_path = ref->get_parser()->script_path;
 			} else {
 				result.kind = GDScriptParser::DataType::SCRIPT;
@@ -3236,6 +3267,9 @@ Error GDScriptAnalyzer::resolve_program() {
 	List<String> parser_keys;
 	depended_parsers.get_key_list(&parser_keys);
 	for (const List<String>::Element *E = parser_keys.front(); E != nullptr; E = E->next()) {
+		if (depended_parsers[E->get()].is_null()) {
+			return ERR_PARSE_ERROR;
+		}
 		depended_parsers[E->get()]->raise_status(GDScriptParserRef::FULLY_SOLVED);
 	}
 	depended_parsers.clear();
diff --git a/modules/gdscript/gdscript_cache.cpp b/modules/gdscript/gdscript_cache.cpp
index cdb14d6281..992f8f4b58 100644
--- a/modules/gdscript/gdscript_cache.cpp
+++ b/modules/gdscript/gdscript_cache.cpp
@@ -127,7 +127,7 @@ Ref<GDScriptParserRef> GDScriptCache::get_parser(const String &p_path, GDScriptP
 		singleton->dependencies[p_owner].insert(p_path);
 	}
 	if (singleton->parser_map.has(p_path)) {
-		ref = singleton->parser_map[p_path];
+		ref = Ref<GDScriptParserRef>(singleton->parser_map[p_path]);
 	} else {
 		if (!FileAccess::exists(p_path)) {
 			r_error = ERR_FILE_NOT_FOUND;
@@ -137,8 +137,7 @@ Ref<GDScriptParserRef> GDScriptCache::get_parser(const String &p_path, GDScriptP
 		ref.instance();
 		ref->parser = parser;
 		ref->path = p_path;
-		singleton->parser_map[p_path] = ref;
-		ref->unreference();
+		singleton->parser_map[p_path] = ref.ptr();
 	}
 
 	r_error = ref->raise_status(p_status);
@@ -186,10 +185,7 @@ Ref<GDScript> GDScriptCache::get_shallow_script(const String &p_path, const Stri
 	script->set_script_path(p_path);
 	script->load_source_code(p_path);
 
-	singleton->shallow_gdscript_cache[p_path] = script;
-	// The one in cache is not a hard reference: if the script dies somewhere else it's fine.
-	// Scripts remove themselves from cache when they die.
-	script->unreference();
+	singleton->shallow_gdscript_cache[p_path] = script.ptr();
 	return script;
 }
 
@@ -217,7 +213,7 @@ Ref<GDScript> GDScriptCache::get_full_script(const String &p_path, Error &r_erro
 		return script;
 	}
 
-	singleton->full_gdscript_cache[p_path] = script;
+	singleton->full_gdscript_cache[p_path] = script.ptr();
 	singleton->shallow_gdscript_cache.erase(p_path);
 
 	return script;
@@ -226,7 +222,7 @@ Ref<GDScript> GDScriptCache::get_full_script(const String &p_path, Error &r_erro
 Error GDScriptCache::finish_compiling(const String &p_owner) {
 	// Mark this as compiled.
 	Ref<GDScript> script = get_shallow_script(p_owner);
-	singleton->full_gdscript_cache[p_owner] = script;
+	singleton->full_gdscript_cache[p_owner] = script.ptr();
 	singleton->shallow_gdscript_cache.erase(p_owner);
 
 	Set<String> depends = singleton->dependencies[p_owner];
diff --git a/modules/gdscript/gdscript_cache.h b/modules/gdscript/gdscript_cache.h
index 770704d6eb..865df34051 100644
--- a/modules/gdscript/gdscript_cache.h
+++ b/modules/gdscript/gdscript_cache.h
@@ -70,9 +70,9 @@ public:
 
 class GDScriptCache {
 	// String key is full path.
-	HashMap<String, Ref<GDScriptParserRef>> parser_map;
-	HashMap<String, Ref<GDScript>> shallow_gdscript_cache;
-	HashMap<String, Ref<GDScript>> full_gdscript_cache;
+	HashMap<String, GDScriptParserRef *> parser_map;
+	HashMap<String, GDScript *> shallow_gdscript_cache;
+	HashMap<String, GDScript *> full_gdscript_cache;
 	HashMap<String, Set<String>> dependencies;
 
 	friend class GDScript;
diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp
index e34d87f5cc..67a894912f 100644
--- a/modules/gdscript/gdscript_compiler.cpp
+++ b/modules/gdscript/gdscript_compiler.cpp
@@ -2609,16 +2609,27 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, const GDScriptPar
 			p_script->_base = base.ptr();
 
 			if (p_class->base_type.kind == GDScriptParser::DataType::CLASS && p_class->base_type.class_type != nullptr) {
-				if (!parsed_classes.has(p_script->_base)) {
-					if (parsing_classes.has(p_script->_base)) {
-						String class_name = p_class->identifier ? p_class->identifier->name : "<main>";
-						_set_error("Cyclic class reference for '" + class_name + "'.", p_class);
-						return ERR_PARSE_ERROR;
+				if (p_class->base_type.script_path == main_script->path) {
+					if (!parsed_classes.has(p_script->_base)) {
+						if (parsing_classes.has(p_script->_base)) {
+							String class_name = p_class->identifier ? p_class->identifier->name : "<main>";
+							_set_error("Cyclic class reference for '" + class_name + "'.", p_class);
+							return ERR_PARSE_ERROR;
+						}
+						Error err = _parse_class_level(p_script->_base, p_class->base_type.class_type, p_keep_state);
+						if (err) {
+							return err;
+						}
 					}
-					Error err = _parse_class_level(p_script->_base, p_class->base_type.class_type, p_keep_state);
+				} else {
+					Error err = OK;
+					base = GDScriptCache::get_full_script(p_class->base_type.script_path, err, main_script->path);
 					if (err) {
 						return err;
 					}
+					if (base.is_null() && !base->is_valid()) {
+						return ERR_COMPILATION_FAILED;
+					}
 				}
 			}
 
@@ -2696,11 +2707,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, const GDScriptPar
 				const GDScriptParser::ConstantNode *constant = member.constant;
 				StringName name = constant->identifier->name;
 
-				ERR_CONTINUE(constant->initializer->type != GDScriptParser::Node::LITERAL);
-
-				const GDScriptParser::LiteralNode *literal = static_cast<const GDScriptParser::LiteralNode *>(constant->initializer);
-
-				p_script->constants.insert(name, literal->value);
+				p_script->constants.insert(name, constant->initializer->reduced_value);
 #ifdef TOOLS_ENABLED
 
 				p_script->member_lines[name] = constant->start_line;
diff --git a/modules/gdscript/gdscript_editor.cpp b/modules/gdscript/gdscript_editor.cpp
index 88bf8cde0f..2e372575da 100644
--- a/modules/gdscript/gdscript_editor.cpp
+++ b/modules/gdscript/gdscript_editor.cpp
@@ -483,6 +483,8 @@ String GDScriptLanguage::make_function(const String &p_class, const String &p_na
 
 #ifdef TOOLS_ENABLED
 
+#define COMPLETION_RECURSION_LIMIT 200
+
 struct GDScriptCompletionIdentifier {
 	GDScriptParser::DataType type;
 	String enumeration;
@@ -766,9 +768,11 @@ static void _find_identifiers_in_suite(const GDScriptParser::SuiteNode *p_suite,
 	}
 }
 
-static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base, bool p_only_functions, Map<String, ScriptCodeCompletionOption> &r_result);
+static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base, bool p_only_functions, Map<String, ScriptCodeCompletionOption> &r_result, int p_recursion_depth);
+
+static void _find_identifiers_in_class(const GDScriptParser::ClassNode *p_class, bool p_only_functions, bool p_static, bool p_parent_only, Map<String, ScriptCodeCompletionOption> &r_result, int p_recursion_depth) {
+	ERR_FAIL_COND(p_recursion_depth > COMPLETION_RECURSION_LIMIT);
 
-static void _find_identifiers_in_class(const GDScriptParser::ClassNode *p_class, bool p_only_functions, bool p_static, bool p_parent_only, Map<String, ScriptCodeCompletionOption> &r_result) {
 	if (!p_parent_only) {
 		bool outer = false;
 		const GDScriptParser::ClassNode *clss = p_class;
@@ -820,7 +824,6 @@ static void _find_identifiers_in_class(const GDScriptParser::ClassNode *p_class,
 						break;
 					case GDScriptParser::ClassNode::Member::SIGNAL:
 						if (p_only_functions || outer) {
-							clss = clss->outer;
 							continue;
 						}
 						option = ScriptCodeCompletionOption(member.signal->identifier->name, ScriptCodeCompletionOption::KIND_SIGNAL);
@@ -840,10 +843,12 @@ static void _find_identifiers_in_class(const GDScriptParser::ClassNode *p_class,
 	base_type.type = p_class->base_type;
 	base_type.type.is_meta_type = p_static;
 
-	_find_identifiers_in_base(base_type, p_only_functions, r_result);
+	_find_identifiers_in_base(base_type, p_only_functions, r_result, p_recursion_depth + 1);
 }
 
-static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base, bool p_only_functions, Map<String, ScriptCodeCompletionOption> &r_result) {
+static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base, bool p_only_functions, Map<String, ScriptCodeCompletionOption> &r_result, int p_recursion_depth) {
+	ERR_FAIL_COND(p_recursion_depth > COMPLETION_RECURSION_LIMIT);
+
 	GDScriptParser::DataType base_type = p_base.type;
 	bool _static = base_type.is_meta_type;
 
@@ -856,7 +861,7 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
 	while (!base_type.has_no_type()) {
 		switch (base_type.kind) {
 			case GDScriptParser::DataType::CLASS: {
-				_find_identifiers_in_class(base_type.class_type, p_only_functions, _static, false, r_result);
+				_find_identifiers_in_class(base_type.class_type, p_only_functions, _static, false, r_result, p_recursion_depth + 1);
 				// This already finds all parent identifiers, so we are done.
 				base_type = GDScriptParser::DataType();
 			} break;
@@ -1007,14 +1012,14 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
 	}
 }
 
-static void _find_identifiers(GDScriptParser::CompletionContext &p_context, bool p_only_functions, Map<String, ScriptCodeCompletionOption> &r_result) {
+static void _find_identifiers(GDScriptParser::CompletionContext &p_context, bool p_only_functions, Map<String, ScriptCodeCompletionOption> &r_result, int p_recursion_depth) {
 	if (!p_only_functions && p_context.current_suite) {
 		// This includes function parameters, since they are also locals.
 		_find_identifiers_in_suite(p_context.current_suite, r_result);
 	}
 
 	if (p_context.current_class) {
-		_find_identifiers_in_class(p_context.current_class, p_only_functions, (!p_context.current_function || p_context.current_function->is_static), false, r_result);
+		_find_identifiers_in_class(p_context.current_class, p_only_functions, (!p_context.current_function || p_context.current_function->is_static), false, r_result, p_recursion_depth + 1);
 	}
 
 	for (int i = 0; i < GDScriptFunctions::FUNC_MAX; i++) {
@@ -2453,7 +2458,7 @@ Error GDScriptLanguage::complete_code(const String &p_code, const String &p_path
 				break;
 			}
 			if (!_guess_expression_type(completion_context, static_cast<const GDScriptParser::AssignmentNode *>(completion_context.node)->assignee, type)) {
-				_find_identifiers(completion_context, false, options);
+				_find_identifiers(completion_context, false, options, 0);
 				r_forced = true;
 				break;
 			}
@@ -2462,7 +2467,7 @@ Error GDScriptLanguage::complete_code(const String &p_code, const String &p_path
 				_find_enumeration_candidates(completion_context, type.enumeration, options);
 				r_forced = options.size() > 0;
 			} else {
-				_find_identifiers(completion_context, false, options);
+				_find_identifiers(completion_context, false, options, 0);
 				r_forced = true;
 			}
 		} break;
@@ -2470,7 +2475,7 @@ Error GDScriptLanguage::complete_code(const String &p_code, const String &p_path
 			is_function = true;
 			[[fallthrough]];
 		case GDScriptParser::COMPLETION_IDENTIFIER: {
-			_find_identifiers(completion_context, is_function, options);
+			_find_identifiers(completion_context, is_function, options, 0);
 		} break;
 		case GDScriptParser::COMPLETION_ATTRIBUTE_METHOD:
 			is_function = true;
@@ -2484,7 +2489,7 @@ Error GDScriptLanguage::complete_code(const String &p_code, const String &p_path
 					break;
 				}
 
-				_find_identifiers_in_base(base, is_function, options);
+				_find_identifiers_in_base(base, is_function, options, 0);
 			}
 		} break;
 		case GDScriptParser::COMPLETION_SUBSCRIPT: {
@@ -2504,7 +2509,7 @@ Error GDScriptLanguage::complete_code(const String &p_code, const String &p_path
 				c.current_class = nullptr;
 			}
 
-			_find_identifiers_in_base(base, false, options);
+			_find_identifiers_in_base(base, false, options, 0);
 		} break;
 		case GDScriptParser::COMPLETION_TYPE_ATTRIBUTE: {
 			if (!completion_context.current_class) {
@@ -2530,7 +2535,7 @@ Error GDScriptLanguage::complete_code(const String &p_code, const String &p_path
 
 			// TODO: Improve this to only list types.
 			if (found) {
-				_find_identifiers_in_base(base, false, options);
+				_find_identifiers_in_base(base, false, options, 0);
 			}
 			r_forced = true;
 		} break;
@@ -2651,7 +2656,7 @@ Error GDScriptLanguage::complete_code(const String &p_code, const String &p_path
 			if (!completion_context.current_class) {
 				break;
 			}
-			_find_identifiers_in_class(completion_context.current_class, true, false, true, options);
+			_find_identifiers_in_class(completion_context.current_class, true, false, true, options, 0);
 		} break;
 	}
 
diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp
index aeec1c0379..03da5e926b 100644
--- a/modules/gdscript/gdscript_parser.cpp
+++ b/modules/gdscript/gdscript_parser.cpp
@@ -631,7 +631,6 @@ void GDScriptParser::parse_extends() {
 		current_class->extends_path = previous.literal;
 
 		if (!match(GDScriptTokenizer::Token::PERIOD)) {
-			end_statement("superclass path");
 			return;
 		}
 	}
@@ -1356,7 +1355,7 @@ GDScriptParser::Node *GDScriptParser::parse_statement() {
 			advance();
 			ReturnNode *n_return = alloc_node<ReturnNode>();
 			if (!is_statement_end()) {
-				if (current_function->identifier->name == GDScriptLanguage::get_singleton()->strings._init) {
+				if (current_function && current_function->identifier->name == GDScriptLanguage::get_singleton()->strings._init) {
 					push_error(R"(Constructor cannot return a value.)");
 				}
 				n_return->return_value = parse_expression(false);