Skip to content

Commit

Permalink
fixed calling external method in static analyzer, but not totally fixed
Browse files Browse the repository at this point in the history
  • Loading branch information
@Lazy-Rabbit-2001
Lazy-Rabbit-2001 committed Oct 21, 2024
1 parent 0bd4d91 commit 5abf1f5
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 35 deletions.
18 changes: 14 additions & 4 deletions modules/gdscript/gdscript.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1669,6 +1669,9 @@ bool GDScriptInstance::set(const StringName &p_name, const Variant &p_value) {
const GDScript::MemberInfo *member = &E->value;
Variant value = p_value;
if (member->data_type.has_type && !member->data_type.is_type(value)) {
if (!execute_access_restriction(p_name, script, script->member_access_restrictions[p_name])) {
return false;
}
const Variant *args = &p_value;
Callable::CallError err;
Variant::construct(member->data_type.builtin_type, value, &args, 1, err);
Expand All @@ -1677,10 +1680,15 @@ bool GDScriptInstance::set(const StringName &p_name, const Variant &p_value) {
}
}
if (likely(script->valid) && member->setter) {
if (!execute_access_restriction(member->setter, script, script->member_access_restrictions[p_name])) {
return false;
}
const Variant *args = &value;
Callable::CallError err;
callp(member->setter, &args, 1, err);
return err.error == Callable::CallError::CALL_OK;
} else if (!execute_access_restriction(member->setter, script, script->member_access_restrictions[p_name])) {
return false;
} else {
members.write[member->index] = value;
return true;
Expand All @@ -1696,15 +1704,15 @@ bool GDScriptInstance::set(const StringName &p_name, const Variant &p_value) {
const GDScript::MemberInfo *member = &E->value;
Variant value = p_value;
if (member->data_type.has_type && !member->data_type.is_type(value)) {
if (!execute_access_restriction(p_name, script, sptr->member_access_restrictions[p_name])) {
return false;
}
const Variant *args = &p_value;
Callable::CallError err;
Variant::construct(member->data_type.builtin_type, value, &args, 1, err);
if (err.error != Callable::CallError::CALL_OK || !member->data_type.is_type(value)) {
return false;
}
if (!execute_access_restriction(p_name, script, sptr->member_access_restrictions[p_name])) {
return false;
}
}
if (likely(sptr->valid) && member->setter) {
if (!execute_access_restriction(member->setter, script, sptr->member_access_restrictions[p_name])) {
Expand All @@ -1714,9 +1722,11 @@ bool GDScriptInstance::set(const StringName &p_name, const Variant &p_value) {
Callable::CallError err;
callp(member->setter, &args, 1, err);
return err.error == Callable::CallError::CALL_OK;
} else if (!execute_access_restriction(member->setter, script, script->member_access_restrictions[p_name])) {
return false;
} else {
sptr->static_variables.write[member->index] = value;
return true;
return execute_access_restriction(member->setter, script, script->member_access_restrictions[p_name]);
}
}
}
Expand Down
53 changes: 25 additions & 28 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,17 +296,16 @@ Error GDScriptAnalyzer::check_class_member_name_conflict(const GDScriptParser::C
return OK;
}

bool GDScriptAnalyzer::execute_access_protection(const GDScriptParser::ClassNode *p_derived_class, const GDScriptParser::Node *p_protected_member, const Vector<StringName> &p_super_classes, const GDScriptParser::Node *p_node) {
bool GDScriptAnalyzer::execute_access_protection(const GDScriptParser::ClassNode *p_derived_class, const GDScriptParser::Node *p_protected_member, const Vector<StringName> &p_super_classes, const GDScriptParser::Node *p_node, const bool p_is_call) {
if (p_protected_member->access_restriction == GDScriptParser::Node::ACCESS_RESTRICTION_PUBLIC) {
return true;
}

ERR_FAIL_COND_V_MSG(!p_derived_class, false, R"(Could not resolve the null derived class node...)");
ERR_FAIL_COND_V_MSG(!p_protected_member, false, R"(Could not resolve the null member node...)");

const bool is_call = p_protected_member->type == GDScriptParser::Node::CALL;
const String member_type = (is_call || p_protected_member->type == GDScriptParser::Node::FUNCTION) ? "method" : (p_protected_member->type == GDScriptParser::Node::Type::SIGNAL ? "signal" : "property");
const String action = is_call ? "call" : "access";
const String member_type = p_protected_member->type == GDScriptParser::Node::FUNCTION ? "method" : (p_protected_member->type == GDScriptParser::Node::Type::SIGNAL ? "signal" : "property");
const String action = p_is_call ? "call" : "access";

const bool is_from_non_derived = !p_super_classes.has(p_derived_class->identifier->name);

Expand All @@ -315,10 +314,10 @@ bool GDScriptAnalyzer::execute_access_protection(const GDScriptParser::ClassNode

switch (execute_access_protection_global(p_derived_class->identifier->name, p_protected_member, p_super_classes, p_node)) {
case GDScriptAnalyzer::AccessRestrictionError::ACCESS_PRIVATE:
push_error(vformat(R"*(Could not %s %s "%s%s" in %s class, because it is private.)*", action, member_type, member_name, is_call ? "()" : "", is_from_non_derived ? "external" : "super"), p_node);
push_error(vformat(R"*(Could not %s %s "%s%s" in %s class, because it is private.)*", action, member_type, member_name, p_is_call ? "()" : "", is_from_non_derived ? "external" : "super"), p_node);
return false;
case GDScriptAnalyzer::AccessRestrictionError::ACCESS_PROTECTED:
push_error(vformat(R"*(Could not %s %s "%s%s" in external class, because it is protected by class "%s".)*", action, member_type, member_name, is_call ? "()" : "", p_protected_member->access_member_owner), p_node);
push_error(vformat(R"*(Could not %s %s "%s%s" in external class, because it is protected by class "%s".)*", action, member_type, member_name, p_is_call ? "()" : "", p_protected_member->access_member_owner), p_node);
return false;
default:
break; // GDScriptAnalyzer::AccessRestrictionError::ACCESS_OTHER_ERR was thrown, so skip this here.
Expand Down Expand Up @@ -3577,30 +3576,28 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
}

// Access protection for calling a method
// It's very tricky to do so in this method
GDScriptParser::ClassNode *base_class = base_type.class_type;
if (base_class) {
GDScriptParser::FunctionNode *method = nullptr;
// For calling an external method, including ClassName.method(), or super.method().
if (base_class->has_member(p_call->function_name)) {
method = static_cast<GDScriptParser::FunctionNode *>(base_class->get_member(p_call->function_name).get_source_node());
} else {
// For calling a method that is declared in the super class of the base class.
for (GDScriptParser::IdentifierNode *E : base_class->extends) {
if (!ScriptServer::is_global_class(E->name)) {
continue; // Skip because it is native or undefined.
}
GDScriptParser::ClassNode *super_class = make_global_class_meta_type(E->name, E).class_type;
if (super_class->has_member(p_call->function_name)) {
resolve_class_member(super_class, p_call->function_name, p_call);
method = static_cast<GDScriptParser::FunctionNode *>(super_class->get_member(p_call->function_name).get_source_node());
break;
}
// It's very hacky to do so in this method
// TODO: Performance boost
GDScriptParser::FunctionNode *method = nullptr;
// For calling an external method, including ClassName.method(), or super.method().
if (parser->current_class->has_member(p_call->function_name)) {
method = static_cast<GDScriptParser::FunctionNode *>(parser->current_class->get_member(p_call->function_name).get_source_node());
} else {
// For calling a method that is declared in the super class of the base class.
const GDScriptParser::ClassNode *current_super_class = base_type.class_type;
while (current_super_class) {
if (!ScriptServer::is_global_class(current_super_class->identifier->name)) {
continue;
} else if (current_super_class->has_member(p_call->function_name)) {
method = static_cast<GDScriptParser::FunctionNode *>(current_super_class->get_member(p_call->function_name).get_source_node());
break;
}
current_super_class = current_super_class->base_type.class_type;
}
if (method) {
execute_access_protection(parser->current_class, method, parser->current_class->get_access_member_owner_extends_names(), p_call);
}
print_line(vformat(R"(Method: %s, owner: %s)", method->identifier->name, method->access_member_owner));
}
if (method) {
execute_access_protection(parser->current_class, method, parser->current_class->get_access_member_owner_extends_names(), p_call, true);
}

int default_arg_count = 0;
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_analyzer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class GDScriptAnalyzer {
Error check_native_member_name_conflict(const StringName &p_member_name, const GDScriptParser::Node *p_member_node, const StringName &p_native_type_string);
Error check_class_member_name_conflict(const GDScriptParser::ClassNode *p_class_node, const StringName &p_member_name, const GDScriptParser::Node *p_member_node);

bool execute_access_protection(const GDScriptParser::ClassNode *p_derived_class, const GDScriptParser::Node *p_protected_member, const Vector<StringName> &p_super_classes, const GDScriptParser::Node *p_node);
bool execute_access_protection(const GDScriptParser::ClassNode *p_derived_class, const GDScriptParser::Node *p_protected_member, const Vector<StringName> &p_super_classes, const GDScriptParser::Node *p_node, const bool p_is_call = false);

void get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list, GDScriptParser::Node *p_source);

Expand Down
2 changes: 2 additions & 0 deletions modules/gdscript/gdscript_compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2822,12 +2822,14 @@ Error GDScriptCompiler::_prepare_compilation(GDScript *p_script, const GDScriptP
if (variable->is_static) {
minfo.index = p_script->static_variables_indices.size();
p_script->static_variables_indices[name] = minfo;

GDScript::MemberAccessRestriction::AccessRestriction access_restriction = GDScript::MemberAccessRestriction::AccessRestriction(variable->access_restriction);
p_script->script_static_access_restrictions[name] = GDScript::MemberAccessRestriction(access_restriction, variable->access_member_owner);
} else {
minfo.index = p_script->member_indices.size();
p_script->member_indices[name] = minfo;
p_script->members.insert(name);

GDScript::MemberAccessRestriction::AccessRestriction access_restriction = GDScript::MemberAccessRestriction::AccessRestriction(variable->access_restriction);
p_script->member_access_restrictions[name] = GDScript::MemberAccessRestriction(access_restriction, variable->access_member_owner);
}
Expand Down
10 changes: 8 additions & 2 deletions modules/gdscript/gdscript_parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,7 @@ void GDScriptParser::parse_class_body(bool p_is_multiline) {

while (!class_end && !is_at_end()) {
GDScriptTokenizer::Token token = current;
// Token functions
switch (token.type) {
case GDScriptTokenizer::Token::VAR:
parse_class_member(&GDScriptParser::parse_variable, AnnotationInfo::VARIABLE, "variable", next_is_static, next_is_access_restricted);
Expand All @@ -997,10 +998,10 @@ void GDScriptParser::parse_class_body(bool p_is_multiline) {
}
break;
case GDScriptTokenizer::Token::CONST:
parse_class_member(&GDScriptParser::parse_constant, AnnotationInfo::CONSTANT, "constant");
parse_class_member(&GDScriptParser::parse_constant, AnnotationInfo::CONSTANT, "constant", false, next_is_access_restricted); // Static-not-modifiable
break;
case GDScriptTokenizer::Token::SIGNAL:
parse_class_member(&GDScriptParser::parse_signal, AnnotationInfo::SIGNAL, "signal"); // Static-not-modifiable
parse_class_member(&GDScriptParser::parse_signal, AnnotationInfo::SIGNAL, "signal", false, next_is_access_restricted); // Static-not-modifiable
break;
case GDScriptTokenizer::Token::FUNC:
parse_class_member(&GDScriptParser::parse_function, AnnotationInfo::FUNCTION, "function", next_is_static, next_is_access_restricted);
Expand Down Expand Up @@ -1090,9 +1091,14 @@ void GDScriptParser::parse_class_body(bool p_is_multiline) {
advance();
break;
}
// Clear modifier status
if (token.type != GDScriptTokenizer::Token::STATIC) {
next_is_static = false;
}
if (token.type != GDScriptTokenizer::Token::PRIVATE && token.type != GDScriptTokenizer::Token::PROTECTED) {
next_is_access_restricted = GDScriptParser::Node::ACCESS_RESTRICTION_PUBLIC;
}
// Others
if (panic_mode) {
synchronize();
}
Expand Down

0 comments on commit 5abf1f5

Please sign in to comment.