Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDScript: Fix some lambda bugs #81605

Merged
merged 1 commit into from
Sep 16, 2023
+314 −84
Merged

GDScript: Fix some lambda bugs #81605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions .editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,3 @@ indent_size = 4
[*.{yml,yaml}]
indent_style = space
indent_size = 2

# GDScript unit test files
[*.gd]
indent_style = tab
indent_size = 4
insert_final_newline = true
trim_trailing_whitespace = true

[*.out]
insert_final_newline = true
8 changes: 8 additions & 0 deletions modules/gdscript/.editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[*.gd]
indent_style = tab
indent_size = 4
insert_final_newline = true
trim_trailing_whitespace = true

[*.out]
insert_final_newline = true
37 changes: 32 additions & 5 deletions modules/gdscript/gdscript_compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ void GDScriptCompiler::_set_error(const String &p_error, const GDScriptParser::N
}
}

GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner) {
GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner, bool p_handle_metatype) {
if (!p_datatype.is_set() || !p_datatype.is_hard_type() || p_datatype.is_coroutine) {
return GDScriptDataType();
}
Expand All @@ -101,18 +101,39 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
result.builtin_type = p_datatype.builtin_type;
} break;
case GDScriptParser::DataType::NATIVE: {
if (p_handle_metatype && p_datatype.is_meta_type) {
result.kind = GDScriptDataType::NATIVE;
result.builtin_type = Variant::OBJECT;
result.native_type = GDScriptNativeClass::get_class_static();
break;
}

result.kind = GDScriptDataType::NATIVE;
result.native_type = p_datatype.native_type;
result.builtin_type = p_datatype.builtin_type;
} break;
case GDScriptParser::DataType::SCRIPT: {
if (p_handle_metatype && p_datatype.is_meta_type) {
result.kind = GDScriptDataType::NATIVE;
result.builtin_type = Variant::OBJECT;
result.native_type = p_datatype.script_type.is_valid() ? p_datatype.script_type->get_class() : Script::get_class_static();
break;
}

result.kind = GDScriptDataType::SCRIPT;
result.builtin_type = p_datatype.builtin_type;
result.script_type_ref = p_datatype.script_type;
result.script_type = result.script_type_ref.ptr();
result.native_type = p_datatype.native_type;
} break;
case GDScriptParser::DataType::CLASS: {
if (p_handle_metatype && p_datatype.is_meta_type) {
result.kind = GDScriptDataType::NATIVE;
result.builtin_type = Variant::OBJECT;
result.native_type = GDScript::get_class_static();
break;
}

result.kind = GDScriptDataType::GDSCRIPT;
result.builtin_type = p_datatype.builtin_type;
result.native_type = p_datatype.native_type;
Expand Down Expand Up @@ -148,6 +169,12 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
}
} break;
case GDScriptParser::DataType::ENUM:
if (p_handle_metatype && p_datatype.is_meta_type) {
result.kind = GDScriptDataType::BUILTIN;
result.builtin_type = Variant::DICTIONARY;
break;
}

result.kind = GDScriptDataType::BUILTIN;
result.builtin_type = p_datatype.builtin_type;
break;
Expand All @@ -159,7 +186,7 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
}

if (p_datatype.has_container_element_type()) {
result.set_container_element_type(_gdtype_from_datatype(p_datatype.get_container_element_type(), p_owner));
result.set_container_element_type(_gdtype_from_datatype(p_datatype.get_container_element_type(), p_owner, false));
}

return result;
Expand Down Expand Up @@ -533,7 +560,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
} break;
case GDScriptParser::Node::CAST: {
const GDScriptParser::CastNode *cn = static_cast<const GDScriptParser::CastNode *>(p_expression);
GDScriptDataType cast_type = _gdtype_from_datatype(cn->get_datatype(), codegen.script);
GDScriptDataType cast_type = _gdtype_from_datatype(cn->get_datatype(), codegen.script, false);

GDScriptCodeGenerator::Address result;
if (cast_type.has_type) {
Expand Down Expand Up @@ -911,7 +938,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
GDScriptCodeGenerator::Address result = codegen.add_temporary(_gdtype_from_datatype(type_test->get_datatype(), codegen.script));

GDScriptCodeGenerator::Address operand = _parse_expression(codegen, r_error, type_test->operand);
GDScriptDataType test_type = _gdtype_from_datatype(type_test->test_datatype, codegen.script);
GDScriptDataType test_type = _gdtype_from_datatype(type_test->test_datatype, codegen.script, false);
if (r_error) {
return GDScriptCodeGenerator::Address();
}
Expand Down Expand Up @@ -2587,7 +2614,7 @@ Error GDScriptCompiler::_prepare_compilation(GDScript *p_script, const GDScriptP
}
}

GDScriptDataType base_type = _gdtype_from_datatype(p_class->base_type, p_script);
GDScriptDataType base_type = _gdtype_from_datatype(p_class->base_type, p_script, false);

int native_idx = GDScriptLanguage::get_singleton()->get_global_map()[base_type.native_type];
p_script->native = GDScriptLanguage::get_singleton()->get_global_array()[native_idx];
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class GDScriptCompiler {
Error _create_binary_operator(CodeGen &codegen, const GDScriptParser::BinaryOpNode *on, Variant::Operator op, bool p_initializer = false, const GDScriptCodeGenerator::Address &p_index_addr = GDScriptCodeGenerator::Address());
Error _create_binary_operator(CodeGen &codegen, const GDScriptParser::ExpressionNode *p_left_operand, const GDScriptParser::ExpressionNode *p_right_operand, Variant::Operator op, bool p_initializer = false, const GDScriptCodeGenerator::Address &p_index_addr = GDScriptCodeGenerator::Address());

GDScriptDataType _gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner);
GDScriptDataType _gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner, bool p_handle_metatype = true);

GDScriptCodeGenerator::Address _parse_assign_right_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::AssignmentNode *p_assignmentint, const GDScriptCodeGenerator::Address &p_index_addr = GDScriptCodeGenerator::Address());
GDScriptCodeGenerator::Address _parse_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::ExpressionNode *p_expression, bool p_root = false, bool p_initializer = false, const GDScriptCodeGenerator::Address &p_index_addr = GDScriptCodeGenerator::Address());
Expand Down
74 changes: 72 additions & 2 deletions modules/gdscript/gdscript_lambda_callable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,48 @@ void GDScriptLambdaCallable::call(const Variant **p_arguments, int p_argcount, V
args.resize(p_argcount + captures_amount);
for (int i = 0; i < captures_amount; i++) {
args.write[i] = &captures[i];
if (captures[i].get_type() == Variant::OBJECT) {
bool was_freed = false;
captures[i].get_validated_object_with_check(was_freed);
if (was_freed) {
ERR_PRINT(vformat(R"(Lambda capture at index %d was freed. Passed "null" instead.)", i));
static Variant nil;
args.write[i] = &nil;
}
}
}
for (int i = 0; i < p_argcount; i++) {
args.write[i + captures_amount] = p_arguments[i];
}

r_return_value = function->call(nullptr, args.ptrw(), args.size(), r_call_error);
r_call_error.argument -= captures_amount;
switch (r_call_error.error) {
case Callable::CallError::CALL_ERROR_INVALID_ARGUMENT:
r_call_error.argument -= captures_amount;
#ifdef DEBUG_ENABLED
if (r_call_error.argument < 0) {
ERR_PRINT(vformat("GDScript bug (please report): Invalid value of lambda capture at index %d.", captures_amount + r_call_error.argument));
r_call_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD; // TODO: Add a more suitable error code.
r_call_error.argument = 0;
r_call_error.expected = 0;
}
#endif
break;
case Callable::CallError::CALL_ERROR_TOO_MANY_ARGUMENTS:
case Callable::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS:
r_call_error.expected -= captures_amount;
#ifdef DEBUG_ENABLED
if (r_call_error.expected < 0) {
ERR_PRINT("GDScript bug (please report): Invalid lambda captures count.");
r_call_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD; // TODO: Add a more suitable error code.
r_call_error.argument = 0;
r_call_error.expected = 0;
}
#endif
break;
default:
break;
}
} else {
r_return_value = function->call(nullptr, p_arguments, p_argcount, r_call_error);
}
Expand Down Expand Up @@ -148,13 +183,48 @@ void GDScriptLambdaSelfCallable::call(const Variant **p_arguments, int p_argcoun
args.resize(p_argcount + captures_amount);
for (int i = 0; i < captures_amount; i++) {
args.write[i] = &captures[i];
if (captures[i].get_type() == Variant::OBJECT) {
bool was_freed = false;
captures[i].get_validated_object_with_check(was_freed);
if (was_freed) {
ERR_PRINT(vformat(R"(Lambda capture at index %d was freed. Passed "null" instead.)", i));
static Variant nil;
args.write[i] = &nil;
}
}
}
for (int i = 0; i < p_argcount; i++) {
args.write[i + captures_amount] = p_arguments[i];
}

r_return_value = function->call(static_cast<GDScriptInstance *>(object->get_script_instance()), args.ptrw(), args.size(), r_call_error);
r_call_error.argument -= captures_amount;
switch (r_call_error.error) {
case Callable::CallError::CALL_ERROR_INVALID_ARGUMENT:
r_call_error.argument -= captures_amount;
#ifdef DEBUG_ENABLED
if (r_call_error.argument < 0) {
ERR_PRINT(vformat("GDScript bug (please report): Invalid value of lambda capture at index %d.", captures_amount + r_call_error.argument));
r_call_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD; // TODO: Add a more suitable error code.
r_call_error.argument = 0;
r_call_error.expected = 0;
}
#endif
break;
case Callable::CallError::CALL_ERROR_TOO_MANY_ARGUMENTS:
case Callable::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS:
r_call_error.expected -= captures_amount;
#ifdef DEBUG_ENABLED
if (r_call_error.expected < 0) {
ERR_PRINT("GDScript bug (please report): Invalid lambda captures count.");
r_call_error.error = Callable::CallError::CALL_ERROR_INVALID_METHOD; // TODO: Add a more suitable error code.
r_call_error.argument = 0;
r_call_error.expected = 0;
}
#endif
break;
default:
break;
}
} else {
r_return_value = function->call(static_cast<GDScriptInstance *>(object->get_script_instance()), p_arguments, p_argcount, r_call_error);
}
Expand Down
9 changes: 5 additions & 4 deletions modules/gdscript/gdscript_vm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ String GDScriptFunction::_get_call_error(const Callable::CallError &p_err, const

if (p_err.error == Callable::CallError::CALL_ERROR_INVALID_ARGUMENT) {
int errorarg = p_err.argument;
ERR_FAIL_COND_V_MSG(errorarg < 0 || argptrs[errorarg] == nullptr, "GDScript bug (please report): Invalid CallError argument index or null pointer.", "Invalid CallError argument index or null pointer.");
// Handle the Object to Object case separately as we don't have further class details.
#ifdef DEBUG_ENABLED
if (p_err.expected == Variant::OBJECT && argptrs[errorarg]->get_type() == p_err.expected) {
Expand All @@ -128,9 +129,9 @@ String GDScriptFunction::_get_call_error(const Callable::CallError &p_err, const
err_text = "Invalid type in " + p_where + ". Cannot convert argument " + itos(errorarg + 1) + " from " + Variant::get_type_name(argptrs[errorarg]->get_type()) + " to " + Variant::get_type_name(Variant::Type(p_err.expected)) + ".";
}
} else if (p_err.error == Callable::CallError::CALL_ERROR_TOO_MANY_ARGUMENTS) {
err_text = "Invalid call to " + p_where + ". Expected " + itos(p_err.argument) + " arguments.";
err_text = "Invalid call to " + p_where + ". Expected " + itos(p_err.expected) + " arguments.";
} else if (p_err.error == Callable::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS) {
err_text = "Invalid call to " + p_where + ". Expected " + itos(p_err.argument) + " arguments.";
err_text = "Invalid call to " + p_where + ". Expected " + itos(p_err.expected) + " arguments.";
} else if (p_err.error == Callable::CallError::CALL_ERROR_INVALID_METHOD) {
err_text = "Invalid call. Nonexistent " + p_where + ".";
} else if (p_err.error == Callable::CallError::CALL_ERROR_INSTANCE_IS_NULL) {
Expand Down Expand Up @@ -511,13 +512,13 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
if (p_argcount != _argument_count) {
if (p_argcount > _argument_count) {
r_err.error = Callable::CallError::CALL_ERROR_TOO_MANY_ARGUMENTS;
r_err.argument = _argument_count;
r_err.expected = _argument_count;

call_depth--;
return _get_default_variant_for_data_type(return_type);
} else if (p_argcount < _argument_count - _default_arg_count) {
r_err.error = Callable::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS;
r_err.argument = _argument_count - _default_arg_count;
r_err.expected = _argument_count - _default_arg_count;
call_depth--;
return _get_default_variant_for_data_type(return_type);
} else {
Expand Down
65 changes: 6 additions & 59 deletions modules/gdscript/tests/scripts/runtime/features/member_info.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ class MyClass:

enum MyEnum {}

const Utils = preload("../../utils.notest.gd")

static var test_static_var_untyped
static var test_static_var_weak_null = null
static var test_static_var_weak_int = 1
Expand Down Expand Up @@ -58,68 +60,13 @@ func test():
var script: Script = get_script()
for property in script.get_property_list():
if str(property.name).begins_with("test_"):
if not (property.usage & PROPERTY_USAGE_SCRIPT_VARIABLE):
print("Error: Missing `PROPERTY_USAGE_SCRIPT_VARIABLE` flag.")
print("static var ", property.name, ": ", get_type(property))
print(Utils.get_property_signature(property, true))
for property in get_property_list():
if str(property.name).begins_with("test_"):
if not (property.usage & PROPERTY_USAGE_SCRIPT_VARIABLE):
print("Error: Missing `PROPERTY_USAGE_SCRIPT_VARIABLE` flag.")
print("var ", property.name, ": ", get_type(property))
print(Utils.get_property_signature(property))
for method in get_method_list():
if str(method.name).begins_with("test_"):
print(get_signature(method))
print(Utils.get_method_signature(method))
for method in get_signal_list():
if str(method.name).begins_with("test_"):
print(get_signature(method, true))

func get_type(property: Dictionary, is_return: bool = false) -> String:
match property.type:
TYPE_NIL:
if property.usage & PROPERTY_USAGE_NIL_IS_VARIANT:
return "Variant"
return "void" if is_return else "null"
TYPE_BOOL:
return "bool"
TYPE_INT:
if property.usage & PROPERTY_USAGE_CLASS_IS_ENUM:
return property.class_name
return "int"
TYPE_STRING:
return "String"
TYPE_DICTIONARY:
return "Dictionary"
TYPE_ARRAY:
if property.hint == PROPERTY_HINT_ARRAY_TYPE:
return "Array[%s]" % property.hint_string
return "Array"
TYPE_OBJECT:
if not str(property.class_name).is_empty():
return property.class_name
return "Object"
return "<error>"

func get_signature(method: Dictionary, is_signal: bool = false) -> String:
var result: String = ""
if method.flags & METHOD_FLAG_STATIC:
result += "static "
result += ("signal " if is_signal else "func ") + method.name + "("

var args: Array[Dictionary] = method.args
var default_args: Array = method.default_args
var mandatory_argc: int = args.size() - default_args.size()
for i in args.size():
if i > 0:
result += ", "
var arg: Dictionary = args[i]
result += arg.name + ": " + get_type(arg)
if i >= mandatory_argc:
result += " = " + var_to_str(default_args[i - mandatory_argc])

result += ")"
if is_signal:
if get_type(method.return, true) != "void":
print("Error: Signal return type must be `void`.")
else:
result += " -> " + get_type(method.return, true)
return result
print(Utils.get_method_signature(method, true))
6 changes: 3 additions & 3 deletions modules/gdscript/tests/scripts/runtime/features/member_info.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ static var test_static_var_hard_int: int
var test_var_untyped: Variant
var test_var_weak_null: Variant
var test_var_weak_int: Variant
var test_var_weak_int_exported: int
@export var test_var_weak_int_exported: int
var test_var_weak_variant_type: Variant
var test_var_weak_variant_type_exported: Variant.Type
@export var test_var_weak_variant_type_exported: Variant.Type
var test_var_hard_variant: Variant
var test_var_hard_int: int
var test_var_hard_variant_type: Variant.Type
var test_var_hard_variant_type_exported: Variant.Type
@export var test_var_hard_variant_type_exported: Variant.Type
var test_var_hard_node_process_mode: Node.ProcessMode
var test_var_hard_my_enum: TestMemberInfo.MyEnum
var test_var_hard_array: Array
Expand Down
Loading