Skip to content

Simplify and optimize call arguments from generic parse_class_body() #107849

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

Closed
Closed
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
56 changes: 30 additions & 26 deletions modules/gdscript/gdscript_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -876,13 +876,13 @@ bool GDScriptParser::has_class(const GDScriptParser::ClassNode *p_class) const {
return false;
}

GDScriptParser::ClassNode *GDScriptParser::parse_class(bool p_is_abstract, bool p_is_static) {
GDScriptParser::ClassNode *GDScriptParser::parse_class(int p_flags) {
Copy link
Member

Choose a reason for hiding this comment

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

This (and other places where you want to pass these flags) should use BitField. Passing flags in a plain int doesn't provide enough static safety.

ClassNode *n_class = alloc_node<ClassNode>();

ClassNode *previous_class = current_class;
current_class = n_class;
n_class->outer = previous_class;
n_class->is_abstract = p_is_abstract;
n_class->is_abstract = bool(p_flags & GDScriptParser::MemberFlag::FLAG_ABSTRACT);

if (consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected identifier for the class name after "class".)")) {
n_class->identifier = parse_identifier();
Expand Down Expand Up @@ -978,7 +978,7 @@ void GDScriptParser::parse_extends() {
}

template <typename T>
void GDScriptParser::parse_class_member(T *(GDScriptParser::*p_parse_function)(bool, bool), AnnotationInfo::TargetKind p_target, const String &p_member_kind, bool p_is_abstract, bool p_is_static) {
void GDScriptParser::parse_class_member(T *(GDScriptParser::*p_parse_function)(int), AnnotationInfo::TargetKind p_target, const String &p_member_kind, int p_flags) {
advance();

// Consume annotations.
Expand All @@ -994,7 +994,7 @@ void GDScriptParser::parse_class_member(T *(GDScriptParser::*p_parse_function)(b
}
}

T *member = (this->*p_parse_function)(p_is_abstract, p_is_static);
T *member = (this->*p_parse_function)(p_flags);
if (member == nullptr) {
return;
}
Expand Down Expand Up @@ -1050,22 +1050,26 @@ void GDScriptParser::parse_class_member(T *(GDScriptParser::*p_parse_function)(b

void GDScriptParser::parse_class_body(bool p_first_is_abstract, bool p_is_multiline) {
bool class_end = false;

int flags = GDScriptParser::MemberFlag::FLAG_NONE;
// The header parsing code could consume `abstract` for the first function or inner class.
bool next_is_abstract = p_first_is_abstract;
bool next_is_static = false;
if (p_first_is_abstract) {
flags |= GDScriptParser::MemberFlag::FLAG_ABSTRACT;
}

while (!class_end && !is_at_end()) {
GDScriptTokenizer::Token token = current;
switch (token.type) {
case GDScriptTokenizer::Token::ABSTRACT: {
advance();
next_is_abstract = true;
flags |= GDScriptParser::MemberFlag::FLAG_ABSTRACT;
if (!check(GDScriptTokenizer::Token::CLASS) && !check(GDScriptTokenizer::Token::FUNC)) {
push_error(R"(Expected "class" or "func" after "abstract".)");
}
} break;
case GDScriptTokenizer::Token::VAR:
parse_class_member(&GDScriptParser::parse_variable, AnnotationInfo::VARIABLE, "variable", false, next_is_static);
if (next_is_static) {
parse_class_member(&GDScriptParser::parse_variable, AnnotationInfo::VARIABLE, "variable", flags);
if (flags & GDScriptParser::MemberFlag::FLAG_STATIC) {
current_class->has_static_data = true;
}
break;
Expand All @@ -1076,17 +1080,17 @@ void GDScriptParser::parse_class_body(bool p_first_is_abstract, bool p_is_multil
parse_class_member(&GDScriptParser::parse_signal, AnnotationInfo::SIGNAL, "signal");
break;
case GDScriptTokenizer::Token::FUNC:
parse_class_member(&GDScriptParser::parse_function, AnnotationInfo::FUNCTION, "function", next_is_abstract, next_is_static);
parse_class_member(&GDScriptParser::parse_function, AnnotationInfo::FUNCTION, "function", flags);
break;
case GDScriptTokenizer::Token::CLASS:
parse_class_member(&GDScriptParser::parse_class, AnnotationInfo::CLASS, "class", next_is_abstract);
parse_class_member(&GDScriptParser::parse_class, AnnotationInfo::CLASS, "class", flags);
break;
case GDScriptTokenizer::Token::ENUM:
parse_class_member(&GDScriptParser::parse_enum, AnnotationInfo::NONE, "enum");
break;
case GDScriptTokenizer::Token::STATIC: {
advance();
next_is_static = true;
flags |= GDScriptParser::MemberFlag::FLAG_STATIC;
if (!check(GDScriptTokenizer::Token::FUNC) && !check(GDScriptTokenizer::Token::VAR)) {
push_error(R"(Expected "func" or "var" after "static".)");
}
Expand Down Expand Up @@ -1160,10 +1164,10 @@ void GDScriptParser::parse_class_body(bool p_first_is_abstract, bool p_is_multil
break;
}
if (token.type != GDScriptTokenizer::Token::ABSTRACT) {
next_is_abstract = false;
flags &= ~GDScriptParser::MemberFlag::FLAG_ABSTRACT;
}
if (token.type != GDScriptTokenizer::Token::STATIC) {
next_is_static = false;
flags &= ~GDScriptParser::MemberFlag::FLAG_STATIC;
}
if (panic_mode) {
synchronize();
Expand All @@ -1174,11 +1178,11 @@ void GDScriptParser::parse_class_body(bool p_first_is_abstract, bool p_is_multil
}
}

GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_abstract, bool p_is_static) {
return parse_variable(p_is_abstract, p_is_static, true);
GDScriptParser::VariableNode *GDScriptParser::parse_variable(int p_flags) {
return parse_variable(p_flags, true);
}

GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_abstract, bool p_is_static, bool p_allow_property) {
GDScriptParser::VariableNode *GDScriptParser::parse_variable(int p_flags, bool p_allow_property) {
VariableNode *variable = alloc_node<VariableNode>();

if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected variable name after "var".)")) {
Expand All @@ -1188,7 +1192,7 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_abstract,

variable->identifier = parse_identifier();
variable->export_info.name = variable->identifier->name;
variable->is_static = p_is_static;
variable->is_static = bool(p_flags & GDScriptParser::MemberFlag::FLAG_STATIC);

if (match(GDScriptTokenizer::Token::COLON)) {
if (check(GDScriptTokenizer::Token::NEWLINE)) {
Expand Down Expand Up @@ -1414,7 +1418,7 @@ void GDScriptParser::parse_property_getter(VariableNode *p_variable) {
}
}

GDScriptParser::ConstantNode *GDScriptParser::parse_constant(bool p_is_abstract, bool p_is_static) {
GDScriptParser::ConstantNode *GDScriptParser::parse_constant(int p_flags) {
ConstantNode *constant = alloc_node<ConstantNode>();

if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected constant name after "const".)")) {
Expand Down Expand Up @@ -1482,7 +1486,7 @@ GDScriptParser::ParameterNode *GDScriptParser::parse_parameter() {
return parameter;
}

GDScriptParser::SignalNode *GDScriptParser::parse_signal(bool p_is_abstract, bool p_is_static) {
GDScriptParser::SignalNode *GDScriptParser::parse_signal(int p_flags) {
SignalNode *signal = alloc_node<SignalNode>();

if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected signal name after "signal".)")) {
Expand Down Expand Up @@ -1527,7 +1531,7 @@ GDScriptParser::SignalNode *GDScriptParser::parse_signal(bool p_is_abstract, boo
return signal;
}

GDScriptParser::EnumNode *GDScriptParser::parse_enum(bool p_is_abstract, bool p_is_static) {
GDScriptParser::EnumNode *GDScriptParser::parse_enum(int p_flags) {
EnumNode *enum_node = alloc_node<EnumNode>();
bool named = false;

Expand Down Expand Up @@ -1712,10 +1716,10 @@ void GDScriptParser::parse_function_signature(FunctionNode *p_function, SuiteNod
}
}

GDScriptParser::FunctionNode *GDScriptParser::parse_function(bool p_is_abstract, bool p_is_static) {
GDScriptParser::FunctionNode *GDScriptParser::parse_function(int p_flags) {
FunctionNode *function = alloc_node<FunctionNode>();
function->is_abstract = p_is_abstract;
function->is_static = p_is_static;
function->is_abstract = bool(p_flags & GDScriptParser::MemberFlag::FLAG_ABSTRACT);
function->is_static = bool(p_flags & GDScriptParser::MemberFlag::FLAG_STATIC);

make_completion_context(COMPLETION_OVERRIDE_METHOD, function);

Expand Down Expand Up @@ -1995,11 +1999,11 @@ GDScriptParser::Node *GDScriptParser::parse_statement() {
break;
case GDScriptTokenizer::Token::VAR:
advance();
result = parse_variable(false, false, false);
result = parse_variable(GDScriptParser::MemberFlag::FLAG_NONE, false);
break;
case GDScriptTokenizer::Token::TK_CONST:
advance();
result = parse_constant(false, false);
result = parse_constant(GDScriptParser::MemberFlag::FLAG_NONE);
break;
case GDScriptTokenizer::Token::IF:
advance();
Expand Down
22 changes: 14 additions & 8 deletions modules/gdscript/gdscript_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -1429,6 +1429,12 @@ class GDScriptParser {
};
static ParseRule *get_rule(GDScriptTokenizer::Token::Type p_token_type);

enum MemberFlag {
FLAG_NONE = 0,
FLAG_ABSTRACT = 1,
FLAG_STATIC = 2,
};

List<Node *> nodes_in_progress;
void complete_extents(Node *p_node);
void update_extents(Node *p_node);
Expand Down Expand Up @@ -1503,16 +1509,16 @@ class GDScriptParser {

// Main blocks.
void parse_program();
ClassNode *parse_class(bool p_is_abstract, bool p_is_static);
ClassNode *parse_class(int p_flags);
void parse_class_name();
void parse_extends();
void parse_class_body(bool p_first_is_abstract, bool p_is_multiline);
template <typename T>
void parse_class_member(T *(GDScriptParser::*p_parse_function)(bool, bool), AnnotationInfo::TargetKind p_target, const String &p_member_kind, bool p_is_abstract = false, bool p_is_static = false);
SignalNode *parse_signal(bool p_is_abstract, bool p_is_static);
EnumNode *parse_enum(bool p_is_abstract, bool p_is_static);
void parse_class_member(T *(GDScriptParser::*p_parse_function)(int), AnnotationInfo::TargetKind p_target, const String &p_member_kind, int p_flags = GDScriptParser::MemberFlag::FLAG_NONE);
SignalNode *parse_signal(int p_flags);
EnumNode *parse_enum(int p_flags);
ParameterNode *parse_parameter();
FunctionNode *parse_function(bool p_is_abstract, bool p_is_static);
FunctionNode *parse_function(int p_flags);
void parse_function_signature(FunctionNode *p_function, SuiteNode *p_body, const String &p_type, int p_signature_start);
SuiteNode *parse_suite(const String &p_context, SuiteNode *p_suite = nullptr, bool p_for_lambda = false);
// Annotations
Expand All @@ -1536,12 +1542,12 @@ class GDScriptParser {
bool rpc_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
// Statements.
Node *parse_statement();
VariableNode *parse_variable(bool p_is_abstract, bool p_is_static);
VariableNode *parse_variable(bool p_is_abstract, bool p_is_static, bool p_allow_property);
VariableNode *parse_variable(int p_flags);
VariableNode *parse_variable(int p_flags, bool p_allow_property);
VariableNode *parse_property(VariableNode *p_variable, bool p_need_indent);
void parse_property_getter(VariableNode *p_variable);
void parse_property_setter(VariableNode *p_variable);
ConstantNode *parse_constant(bool p_is_abstract, bool p_is_static);
ConstantNode *parse_constant(int p_flags);
AssertNode *parse_assert();
BreakNode *parse_break();
ContinueNode *parse_continue();
Expand Down