From 861743cd04dd1b46392b969ae5332a21b69f21b6 Mon Sep 17 00:00:00 2001 From: "ocean (they/them)" Date: Sun, 21 May 2023 19:22:00 -0400 Subject: GDScript: add errors when calling unimplemented virtual functions This PR does a small refactor of how method flags are handled in the GDScript analyzer. This way, it adds support for the analyzer to use any of MethodInfo's flags, where previously it could only use METHOD_FLAG_STATIC and METHOD_FLAG_VARARG. As a side-effect, this also normalizes behavior between editor and release templates, which fixes #76938. The tests added also brought a different issue to light, where using `super()` appears to generate a return variable discarded on calling super's _init(), which doesn't have a return value. This should be tackled in a different PR, which will have to change the output of this PR's tests. --- modules/gdscript/gdscript_analyzer.cpp | 66 ++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 30 deletions(-) (limited to 'modules/gdscript/gdscript_analyzer.cpp') diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 8251de2956..585f6a55a6 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -1610,11 +1610,10 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode * GDScriptParser::DataType parent_return_type; List parameters_types; int default_par_count = 0; - bool is_static = false; - bool is_vararg = false; + BitField method_flags; StringName native_base; - if (!p_is_lambda && get_function_signature(p_function, false, base_type, function_name, parent_return_type, parameters_types, default_par_count, is_static, is_vararg, &native_base)) { - bool valid = p_function->is_static == is_static; + if (!p_is_lambda && get_function_signature(p_function, false, base_type, function_name, parent_return_type, parameters_types, default_par_count, method_flags, &native_base)) { + bool valid = p_function->is_static == method_flags.has_flag(METHOD_FLAG_STATIC); valid = valid && parent_return_type == p_function->get_datatype(); int par_count_diff = p_function->parameters.size() - parameters_types.size(); @@ -2029,9 +2028,8 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) { GDScriptParser::DataType return_type; List par_types; int default_arg_count = 0; - bool is_static = false; - bool is_vararg = false; - if (get_function_signature(p_for->list, false, list_type, CoreStringNames::get_singleton()->_iter_get, return_type, par_types, default_arg_count, is_static, is_vararg)) { + BitField method_flags; + if (get_function_signature(p_for->list, false, list_type, CoreStringNames::get_singleton()->_iter_get, return_type, par_types, default_arg_count, method_flags)) { variable_type = return_type; variable_type.type_source = list_type.type_source; } else if (!list_type.is_hard_type()) { @@ -3084,15 +3082,24 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a return; } - bool is_static = false; - bool is_vararg = false; int default_arg_count = 0; + BitField method_flags; GDScriptParser::DataType return_type; List par_types; bool is_constructor = (base_type.is_meta_type || (p_call->callee && p_call->callee->type == GDScriptParser::Node::IDENTIFIER)) && p_call->function_name == SNAME("new"); - if (get_function_signature(p_call, is_constructor, base_type, p_call->function_name, return_type, par_types, default_arg_count, is_static, is_vararg)) { + if (get_function_signature(p_call, is_constructor, base_type, p_call->function_name, return_type, par_types, default_arg_count, method_flags)) { + // If the method is implemented in the class hierarchy, the virtual flag will not be set for that MethodInfo and the search stops there. + // MethodInfo's above the class that defines the method might still have the virtual flag set. + if (method_flags.has_flag(METHOD_FLAG_VIRTUAL)) { + if (p_call->is_super) { + push_error(vformat(R"*(Cannot call the parent class' virtual function "%s()" because it hasn't been defined.)*", p_call->function_name), p_call); + } else { + push_error(vformat(R"*(Cannot call virtual function "%s()" because it hasn't been defined.)*", p_call->function_name), p_call); + } + } + // If the function require typed arrays we must make literals be typed. for (const KeyValue &E : arrays) { int index = E.key; @@ -3100,14 +3107,14 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a update_array_literal_element_type(E.value, par_types[index].get_container_element_type()); } } - validate_call_arg(par_types, default_arg_count, is_vararg, p_call); + validate_call_arg(par_types, default_arg_count, method_flags.has_flag(METHOD_FLAG_VARARG), p_call); if (base_type.kind == GDScriptParser::DataType::ENUM && base_type.is_meta_type) { // Enum type is treated as a dictionary value for function calls. base_type.is_meta_type = false; } - if (is_self && static_context && !is_static) { + if (is_self && static_context && !method_flags.has_flag(METHOD_FLAG_STATIC)) { if (parser->current_function) { // Get the parent function above any lambda. GDScriptParser::FunctionNode *parent_function = parser->current_function; @@ -3118,10 +3125,10 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a } else { push_error(vformat(R"*(Cannot call non-static function "%s()" for static variable initializer.)*", p_call->function_name), p_call); } - } else if (!is_self && base_type.is_meta_type && !is_static) { + } else if (!is_self && base_type.is_meta_type && !method_flags.has_flag(METHOD_FLAG_STATIC)) { base_type.is_meta_type = false; // For `to_string()`. push_error(vformat(R"*(Cannot call non-static function "%s()" on the class "%s" directly. Make an instance instead.)*", p_call->function_name, base_type.to_string()), p_call); - } else if (is_self && !is_static) { + } else if (is_self && !method_flags.has_flag(METHOD_FLAG_STATIC)) { mark_lambda_use_self(); } @@ -3135,7 +3142,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a parser->push_warning(p_call, GDScriptWarning::RETURN_VALUE_DISCARDED, p_call->function_name); } - if (is_static && !is_constructor && !base_type.is_meta_type && !(is_self && static_context)) { + if (method_flags.has_flag(METHOD_FLAG_STATIC) && !is_constructor && !base_type.is_meta_type && !(is_self && static_context)) { String caller_type = String(base_type.native_type); if (caller_type.is_empty()) { @@ -4651,9 +4658,8 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo return result; } -bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType p_base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg, StringName *r_native_class) { - r_static = false; - r_vararg = false; +bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType p_base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List &r_par_types, int &r_default_arg_count, BitField &r_method_flags, StringName *r_native_class) { + r_method_flags = METHOD_FLAGS_DEFAULT; r_default_arg_count = 0; if (r_native_class) { *r_native_class = StringName(); @@ -4686,10 +4692,9 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo for (const MethodInfo &E : methods) { if (E.name == p_function) { - function_signature_from_info(E, r_return_type, r_par_types, r_default_arg_count, r_static, r_vararg); - r_static = Variant::is_builtin_method_static(p_base_type.builtin_type, function_name); + function_signature_from_info(E, r_return_type, r_par_types, r_default_arg_count, r_method_flags); // Cannot use non-const methods on enums. - if (!r_static && was_enum && !(E.flags & METHOD_FLAG_CONST)) { + if (!r_method_flags.has_flag(METHOD_FLAG_STATIC) && was_enum && !(E.flags & METHOD_FLAG_CONST)) { push_error(vformat(R"*(Cannot call non-const Dictionary function "%s()" on enum "%s".)*", p_function, p_base_type.enum_type), p_source); } return true; @@ -4720,7 +4725,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo if (p_is_constructor) { function_name = GDScriptLanguage::get_singleton()->strings._init; - r_static = true; + r_method_flags.set_flag(METHOD_FLAG_STATIC); } GDScriptParser::ClassNode *base_class = p_base_type.class_type; @@ -4743,7 +4748,9 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo } if (found_function != nullptr) { - r_static = p_is_constructor || found_function->is_static; + if (p_is_constructor || found_function->is_static) { + r_method_flags.set_flag(METHOD_FLAG_STATIC); + } for (int i = 0; i < found_function->parameters.size(); i++) { r_par_types.push_back(found_function->parameters[i]->get_datatype()); if (found_function->parameters[i]->initializer != nullptr) { @@ -4763,7 +4770,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo MethodInfo info = base_script->get_method_info(function_name); if (!(info == MethodInfo())) { - return function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_static, r_vararg); + return function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_method_flags); } base_script = base_script->get_base_script(); } @@ -4774,7 +4781,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo StringName script_class = p_base_type.kind == GDScriptParser::DataType::SCRIPT ? p_base_type.script_type->get_class_name() : StringName(GDScript::get_class_static()); if (ClassDB::get_method_info(script_class, function_name, &info)) { - return function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_static, r_vararg); + return function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_method_flags); } } @@ -4788,9 +4795,9 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo MethodInfo info; if (ClassDB::get_method_info(base_native, function_name, &info)) { - bool valid = function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_static, r_vararg); + bool valid = function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_method_flags); if (valid && Engine::get_singleton()->has_singleton(base_native)) { - r_static = true; + r_method_flags.set_flag(METHOD_FLAG_STATIC); } #ifdef DEBUG_ENABLED MethodBind *native_method = ClassDB::get_method(base_native, function_name); @@ -4804,11 +4811,10 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo return false; } -bool GDScriptAnalyzer::function_signature_from_info(const MethodInfo &p_info, GDScriptParser::DataType &r_return_type, List &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg) { +bool GDScriptAnalyzer::function_signature_from_info(const MethodInfo &p_info, GDScriptParser::DataType &r_return_type, List &r_par_types, int &r_default_arg_count, BitField &r_method_flags) { r_return_type = type_from_property(p_info.return_val); r_default_arg_count = p_info.default_arguments.size(); - r_vararg = (p_info.flags & METHOD_FLAG_VARARG) != 0; - r_static = (p_info.flags & METHOD_FLAG_STATIC) != 0; + r_method_flags = p_info.flags; for (const PropertyInfo &E : p_info.arguments) { r_par_types.push_back(type_from_property(E, true)); -- cgit v1.2.3