Skip to content

Commit

Permalink
Merge pull request #81605 from dalexeev/gds-fix-some-lambda-bugs
Browse files Browse the repository at this point in the history 
GDScript: Fix some lambda bugs
  • Loading branch information
@akien-mga
akien-mga committed Sep 16, 2023
2 parents e5ac7cf + b1eb737 commit 6c1be30
Show file tree
Hide file tree
Showing 12 changed files with 314 additions and 84 deletions.
10 changes: 0 additions & 10 deletions .editorconfig
View file
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
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
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 @@ -534,7 +561,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 @@ -912,7 +939,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 @@ -2588,7 +2615,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
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
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
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
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
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

0 comments on commit 6c1be30

Please sign in to comment.