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.

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 access non-static members in static context #91412

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 23 additions & 8 deletions modules/gdscript/gdscript_analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3384,7 +3384,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
}

if (parent_function) {
push_error(vformat(R"*(Cannot call non-static function "%s()" from static function "%s()".)*", p_call->function_name, parent_function->identifier->name), p_call);
push_error(vformat(R"*(Cannot call non-static function "%s()" from the static function "%s()".)*", p_call->function_name, parent_function->identifier->name), p_call);
} else {
push_error(vformat(R"*(Cannot call non-static function "%s()" from a static variable initializer.)*", p_call->function_name), p_call);
}
Expand Down Expand Up @@ -3801,6 +3801,8 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
if (is_base && (!base.is_meta_type || member.function->is_static || is_constructor)) {
p_identifier->set_datatype(make_callable_type(member.function->info));
p_identifier->source = GDScriptParser::IdentifierNode::MEMBER_FUNCTION;
p_identifier->function_source = member.function;
p_identifier->function_source_is_static = member.function->is_static;
return;
}
} break;
Expand Down Expand Up @@ -3849,6 +3851,7 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
if (method_info.name == p_identifier->name) {
p_identifier->set_datatype(make_callable_type(method_info));
p_identifier->source = GDScriptParser::IdentifierNode::MEMBER_FUNCTION;
p_identifier->function_source_is_static = method_info.flags & METHOD_FLAG_STATIC;
return;
}

Expand Down Expand Up @@ -4029,25 +4032,37 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
}

if (found_source) {
bool source_is_variable = p_identifier->source == GDScriptParser::IdentifierNode::MEMBER_VARIABLE || p_identifier->source == GDScriptParser::IdentifierNode::INHERITED_VARIABLE;
bool source_is_signal = p_identifier->source == GDScriptParser::IdentifierNode::MEMBER_SIGNAL;
if ((source_is_variable || source_is_signal) && static_context) {
const bool source_is_instance_variable = p_identifier->source == GDScriptParser::IdentifierNode::MEMBER_VARIABLE || p_identifier->source == GDScriptParser::IdentifierNode::INHERITED_VARIABLE;
const bool source_is_instance_function = p_identifier->source == GDScriptParser::IdentifierNode::MEMBER_FUNCTION && !p_identifier->function_source_is_static;
const bool source_is_signal = p_identifier->source == GDScriptParser::IdentifierNode::MEMBER_SIGNAL;

if (static_context && (source_is_instance_variable || source_is_instance_function || source_is_signal)) {
// Get the parent function above any lambda.
GDScriptParser::FunctionNode *parent_function = parser->current_function;
while (parent_function && parent_function->source_lambda) {
parent_function = parent_function->source_lambda->parent_function;
}

String source_type;
if (source_is_instance_variable) {
source_type = "non-static variable";
} else if (source_is_instance_function) {
source_type = "non-static function";
} else { // source_is_signal
source_type = "signal";
}

if (parent_function) {
push_error(vformat(R"*(Cannot access %s "%s" from the static function "%s()".)*", source_is_signal ? "signal" : "instance variable", p_identifier->name, parent_function->identifier->name), p_identifier);
push_error(vformat(R"*(Cannot access %s "%s" from the static function "%s()".)*", source_type, p_identifier->name, parent_function->identifier->name), p_identifier);
} else {
push_error(vformat(R"*(Cannot access %s "%s" from a static variable initializer.)*", source_is_signal ? "signal" : "instance variable", p_identifier->name), p_identifier);
push_error(vformat(R"*(Cannot access %s "%s" from a static variable initializer.)*", source_type, p_identifier->name), p_identifier);
}
}

if (current_lambda != nullptr) {
// If the identifier is a member variable (including the native class properties) or a signal, we consider the lambda to be using `self`, so we keep a reference to the current instance.
if (source_is_variable || source_is_signal) {
// If the identifier is a member variable (including the native class properties), member function, or a signal,
// we consider the lambda to be using `self`, so we keep a reference to the current instance.
if (source_is_instance_variable || source_is_instance_function || source_is_signal) {
Comment on lines -4049 to +4065
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an "unrelated" change, but I guess it makes sense? At least the tests did not detect regressions, but this is not always a reliable criterion.

mark_lambda_use_self();
return; // No need to capture.
}
Expand Down
5 changes: 4 additions & 1 deletion modules/gdscript/gdscript_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -902,8 +902,11 @@ class GDScriptParser {
VariableNode *variable_source;
ConstantNode *constant_source;
SignalNode *signal_source;
FunctionNode *function_source;
};
FunctionNode *source_function = nullptr;
bool function_source_is_static = false; // For non-GDScript scripts.

FunctionNode *source_function = nullptr; // TODO: Rename to disambiguate `function_source`.

int usages = 0; // Useful for binds/iterator variable.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# GH-91403

static func static_func():
print(non_static_func)

func non_static_func():
pass

func test():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot access non-static function "non_static_func" from the static function "static_func()".
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# GH-91403

func non_static_func():
pass

static func static_func(
f := func ():
var g := func ():
print(non_static_func)
g.call()
):
f.call()

func test():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot access non-static function "non_static_func" from the static function "static_func()".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot call non-static function "non_static_func()" from static function "static_func()".
Cannot call non-static function "non_static_func()" from the static function "static_func()".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot call non-static function "non_static_func()" from static function "static_func()".
Cannot call non-static function "non_static_func()" from the static function "static_func()".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot call non-static function "non_static_func()" from static function "static_func()".
Cannot call non-static function "non_static_func()" from the static function "static_func()".
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# GH-91403

func non_static_func():
pass

static var static_var = func ():
var f := func ():
var g := func ():
print(non_static_func)
g.call()
f.call()

func test():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot access non-static function "non_static_func" from a static variable initializer.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# GH-91403

func non_static_func():
pass

static var static_var:
set(_value):
var f := func ():
var g := func ():
print(non_static_func)
g.call()
f.call()

func test():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot access non-static function "non_static_func" from the static function "@static_var_setter()".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot call non-static function "non_static_func()" from static function "@static_var_setter()".
Cannot call non-static function "non_static_func()" from the static function "@static_var_setter()".
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# GH-91403

@static_unload

func non_static():
return "non static"

static var static_var = Callable(non_static)

func test():
print("does not run")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot access non-static function "non_static" from a static variable initializer.
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
@static_unload

static var static_var
var non_static_var

signal my_signal()

static func static_func():
pass

func non_static_func():
pass

static var test_static_var_lambda = func ():
static_func()
print(static_func)
static_var = 1
print(static_var)

var test_non_static_var_lambda = func ():
static_func()
print(static_func)
static_var = 1
print(static_var)

non_static_func()
print(non_static_func)
non_static_var = 1
print(non_static_var)
my_signal.emit()
print(my_signal)

static var test_static_var_setter:
set(_value):
static_func()
print(static_func)
static_var = 1
print(static_var)

var test_non_static_var_setter:
set(_value):
static_func()
print(static_func)
static_var = 1
print(static_var)

non_static_func()
print(non_static_func)
non_static_var = 1
print(non_static_var)
my_signal.emit()
print(my_signal)

static func test_static_func():
static_func()
print(static_func)
static_var = 1
print(static_var)

func test_non_static_func():
static_func()
print(static_func)
static_var = 1
print(static_var)

non_static_func()
print(non_static_func)
non_static_var = 1
print(non_static_var)
my_signal.emit()
print(my_signal)

func test():
test_static_var_lambda = null
test_non_static_var_lambda = null
Comment on lines +74 to +75
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes memory leaks in CI. I'm not sure if this is a bug, but it's probably unrelated to the changes since I've noticed this problem before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a reference cycle. Lambda contains a reference to self which in turn contains a reference to the lambda. Not sure if there's a good way to solve it.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GDTEST_OK
Loading