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: support variable definition as condition in if-statement #98538

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ void GDScriptEditorTranslationParserPlugin::_traverse_block(const GDScriptParser
} break;
case GDScriptParser::Node::IF: {
const GDScriptParser::IfNode *if_node = static_cast<const GDScriptParser::IfNode *>(statement);
if (if_node->variable) {
_assess_expression(if_node->variable->initializer);
}
_assess_expression(if_node->condition);
_traverse_block(if_node->true_block);
_traverse_block(if_node->false_block);
Expand Down
3 changes: 3 additions & 0 deletions modules/gdscript/gdscript_analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2096,6 +2096,9 @@ void GDScriptAnalyzer::resolve_parameter(GDScriptParser::ParameterNode *p_parame
}

void GDScriptAnalyzer::resolve_if(GDScriptParser::IfNode *p_if) {
if (p_if->variable) {
resolve_variable(p_if->variable, true);
}
reduce_expression(p_if->condition);

resolve_suite(p_if->true_block);
Expand Down
147 changes: 94 additions & 53 deletions modules/gdscript/gdscript_compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,45 @@ static bool _can_use_validate_call(const MethodBind *p_method, const Vector<GDSc
return true;
}

Error GDScriptCompiler::_parse_variable(CodeGen &codegen, const GDScriptParser::VariableNode *p_variable, const GDScriptParser::SuiteNode *p_block) {
GDScriptCodeGenerator *gen = codegen.generator;

// Should be already in stack when the block began.
GDScriptCodeGenerator::Address local = codegen.locals[p_variable->identifier->name];
GDScriptDataType local_type = _gdtype_from_datatype(p_variable->get_datatype(), codegen.script);

Error err = OK;

bool initialized = false;
if (p_variable->initializer != nullptr) {
GDScriptCodeGenerator::Address src_address = _parse_expression(codegen, err, p_variable->initializer);
if (err) {
return err;
}
if (p_variable->use_conversion_assign) {
gen->write_assign_with_conversion(local, src_address);
} else {
gen->write_assign(local, src_address);
}
if (src_address.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
codegen.generator->pop_temporary();
}
initialized = true;
} else if ((local_type.has_type && local_type.kind == GDScriptDataType::BUILTIN) || codegen.generator->is_local_dirty(local)) {
// Initialize with default for the type. Built-in types must always be cleared (they cannot be `null`).
// Objects and untyped variables are assigned to `null` only if the stack address has been re-used and not cleared.
codegen.generator->clear_address(local);
initialized = true;
}

// Don't check `is_local_dirty()` since the variable must be assigned to `null` **on each iteration**.
if (!initialized && p_block->is_in_loop) {
codegen.generator->clear_address(local);
}

return err;
}

GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::ExpressionNode *p_expression, bool p_root, bool p_initializer) {
if (p_expression->is_constant && !(p_expression->get_datatype().is_meta_type && p_expression->get_datatype().kind == GDScriptParser::DataType::CLASS)) {
return codegen.add_constant(p_expression->reduced_value);
Expand Down Expand Up @@ -1878,6 +1917,56 @@ void GDScriptCompiler::_clear_block_locals(CodeGen &codegen, const List<GDScript
}
}

Error GDScriptCompiler::_parse_if(CodeGen &codegen, const GDScriptParser::IfNode *if_n) {
Error err = OK;
GDScriptCodeGenerator *gen = codegen.generator;
List<GDScriptCodeGenerator::Address> block_locals;

gen->clear_temporaries();
codegen.start_block();
block_locals = _add_block_locals(codegen, if_n->condition_block);

if (if_n->variable) {
err = _parse_variable(codegen, if_n->variable, if_n->condition_block);
if (err) {
return err;
}

gen->clear_temporaries();
}

GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, if_n->condition);
if (err) {
return err;
}
gen->write_if(condition);

if (condition.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
codegen.generator->pop_temporary();
}

err = _parse_block(codegen, if_n->true_block);
if (err) {
return err;
}

_clear_block_locals(codegen, block_locals);
codegen.end_block();

if (if_n->false_block) {
gen->write_else();

err = _parse_block(codegen, if_n->false_block);
if (err) {
return err;
}
}

gen->write_endif();

return OK;
}

Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block, bool p_add_locals, bool p_clear_locals) {
Error err = OK;
GDScriptCodeGenerator *gen = codegen.generator;
Expand Down Expand Up @@ -2004,32 +2093,10 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
} break;
case GDScriptParser::Node::IF: {
const GDScriptParser::IfNode *if_n = static_cast<const GDScriptParser::IfNode *>(s);
GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, if_n->condition);
err = _parse_if(codegen, if_n);
if (err) {
return err;
}

gen->write_if(condition);

if (condition.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
codegen.generator->pop_temporary();
}

err = _parse_block(codegen, if_n->true_block);
if (err) {
return err;
}

if (if_n->false_block) {
gen->write_else();

err = _parse_block(codegen, if_n->false_block);
if (err) {
return err;
}
}

gen->write_endif();
} break;
case GDScriptParser::Node::FOR: {
const GDScriptParser::ForNode *for_n = static_cast<const GDScriptParser::ForNode *>(s);
Expand Down Expand Up @@ -2166,36 +2233,10 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
#endif
} break;
case GDScriptParser::Node::VARIABLE: {
const GDScriptParser::VariableNode *lv = static_cast<const GDScriptParser::VariableNode *>(s);
// Should be already in stack when the block began.
GDScriptCodeGenerator::Address local = codegen.locals[lv->identifier->name];
GDScriptDataType local_type = _gdtype_from_datatype(lv->get_datatype(), codegen.script);

bool initialized = false;
if (lv->initializer != nullptr) {
GDScriptCodeGenerator::Address src_address = _parse_expression(codegen, err, lv->initializer);
if (err) {
return err;
}
if (lv->use_conversion_assign) {
gen->write_assign_with_conversion(local, src_address);
} else {
gen->write_assign(local, src_address);
}
if (src_address.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
codegen.generator->pop_temporary();
}
initialized = true;
} else if ((local_type.has_type && local_type.kind == GDScriptDataType::BUILTIN) || codegen.generator->is_local_dirty(local)) {
// Initialize with default for the type. Built-in types must always be cleared (they cannot be `null`).
// Objects and untyped variables are assigned to `null` only if the stack address has been re-used and not cleared.
codegen.generator->clear_address(local);
initialized = true;
}

// Don't check `is_local_dirty()` since the variable must be assigned to `null` **on each iteration**.
if (!initialized && p_block->is_in_loop) {
codegen.generator->clear_address(local);
const GDScriptParser::VariableNode *variable_n = static_cast<const GDScriptParser::VariableNode *>(s);
err = _parse_variable(codegen, variable_n, p_block);
if (err) {
return err;
}
} break;
case GDScriptParser::Node::CONSTANT: {
Expand Down
2 changes: 2 additions & 0 deletions modules/gdscript/gdscript_compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ class GDScriptCompiler {
GDScriptCodeGenerator::Address _parse_match_pattern(CodeGen &codegen, Error &r_error, const GDScriptParser::PatternNode *p_pattern, const GDScriptCodeGenerator::Address &p_value_addr, const GDScriptCodeGenerator::Address &p_type_addr, const GDScriptCodeGenerator::Address &p_previous_test, bool p_is_first, bool p_is_nested);
List<GDScriptCodeGenerator::Address> _add_block_locals(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block);
void _clear_block_locals(CodeGen &codegen, const List<GDScriptCodeGenerator::Address> &p_locals);
Error _parse_variable(CodeGen &codegen, const GDScriptParser::VariableNode *p_variable, const GDScriptParser::SuiteNode *p_block);
Error _parse_if(CodeGen &codegen, const GDScriptParser::IfNode *if_n);
Error _parse_block(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block, bool p_add_locals = true, bool p_clear_locals = true);
GDScriptFunction *_parse_function(Error &r_error, GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready = false, bool p_for_lambda = false);
GDScriptFunction *_make_static_initializer(Error &r_error, GDScript *p_script, const GDScriptParser::ClassNode *p_class);
Expand Down
98 changes: 83 additions & 15 deletions modules/gdscript/gdscript_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static) {
return parse_variable(p_is_static, true);
}

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

if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected variable name after "var".)")) {
Expand Down Expand Up @@ -1135,7 +1135,10 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static, b
}

complete_extents(variable);
end_statement("variable declaration");

if (p_is_if_condition == false) {
end_statement("variable declaration");
}

return variable;
}
Expand Down Expand Up @@ -2087,21 +2090,68 @@ GDScriptParser::ForNode *GDScriptParser::parse_for() {
}

GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) {
IfNode *n_if = alloc_node<IfNode>();
SuiteNode *condition_block = alloc_node<SuiteNode>();
condition_block->parent_block = current_suite;
condition_block->parent_function = current_function;
SuiteNode *saved_suite = current_suite;
current_suite = condition_block;

VariableNode *variable = nullptr;
ExpressionNode *condition = nullptr;
if (match(GDScriptTokenizer::Token::VAR)) {
// Variable declaration
variable = parse_variable(false, false, true);
if (variable == nullptr) {
push_error(vformat(R"(Expected variable definition after "%s".)", p_token));
} else if (variable->initializer == nullptr) {
push_error(R"(Expected expression for variable initial value.)");
} else {
const SuiteNode::Local &local = current_suite->get_local(variable->identifier->name);
if (local.type != SuiteNode::Local::UNDEFINED) {
push_error(vformat(R"(There is already a %s named "%s" declared in this scope.)", local.get_name(), variable->identifier->name), variable->identifier);
}
condition_block->add_local(variable, current_function);

IdentifierNode *identifier = alloc_node<IdentifierNode>();
identifier->name = variable->identifier->name;
identifier->suite = condition_block;
identifier->source = IdentifierNode::Source::LOCAL_VARIABLE;
const SuiteNode::Local &declaration = condition_block->get_local(identifier->name);
identifier->variable_source = declaration.variable;
declaration.variable->usages++;
complete_extents(identifier);

condition_block->statements.push_back(variable);

n_if->condition = parse_expression(false);
if (n_if->condition == nullptr) {
push_error(vformat(R"(Expected conditional expression after "%s".)", p_token));
condition = identifier;
}
} else {
// Expression
ExpressionNode *expression = parse_expression(false);
if (expression == nullptr) {
push_error(vformat(R"(Expected conditional expression after "%s".)", p_token));
} else {
condition = expression;
}
}

consume(GDScriptTokenizer::Token::COLON, vformat(R"(Expected ":" after "%s" condition.)", p_token));

n_if->true_block = parse_suite(vformat(R"("%s" block)", p_token));
n_if->true_block->parent_if = n_if;
SuiteNode *true_block = parse_suite(vformat(R"("%s" block)", p_token));

if (n_if->true_block->has_continue) {
current_suite->has_continue = true;
}
complete_extents(condition_block);
current_suite = saved_suite;

IfNode *n_if = alloc_node<IfNode>();

true_block->parent_function = current_function;
true_block->parent_block = condition_block;
true_block->parent_if = n_if;

n_if->variable = variable;
n_if->condition = condition;
n_if->condition_block = condition_block;
n_if->true_block = true_block;

if (match(GDScriptTokenizer::Token::ELIF)) {
SuiteNode *else_block = alloc_node<SuiteNode>();
Expand All @@ -2123,10 +2173,10 @@ GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) {
}
complete_extents(n_if);

if (n_if->false_block != nullptr && n_if->false_block->has_return && n_if->true_block->has_return) {
if ((n_if->false_block != nullptr && n_if->false_block->has_return) && n_if->true_block->has_return) {
current_suite->has_return = true;
}
if (n_if->false_block != nullptr && n_if->false_block->has_continue) {
if ((n_if->false_block != nullptr && n_if->false_block->has_continue) || n_if->true_block->has_continue) {
current_suite->has_continue = true;
}

Expand Down Expand Up @@ -4787,11 +4837,25 @@ bool GDScriptParser::warning_annotations(AnnotationNode *p_annotation, Node *p_t

// Contain bodies.
SIMPLE_CASE(Node::FOR, ForNode, list)
SIMPLE_CASE(Node::IF, IfNode, condition)
SIMPLE_CASE(Node::MATCH, MatchNode, test)
SIMPLE_CASE(Node::WHILE, WhileNode, condition)
#undef SIMPLE_CASE

case Node::IF: {
IfNode *if_n = static_cast<IfNode *>(p_target);
if (if_n->variable) {
if (if_n->variable->initializer) {
end_line = if_n->variable->initializer->end_line;
} else {
end_line = if_n->start_line;
}
} else if (if_n->condition) {
end_line = if_n->condition->end_line;
} else {
end_line = if_n->start_line;
}
} break;

case Node::CLASS: {
end_line = p_target->start_line;
for (const AnnotationNode *annotation : p_target->annotations) {
Expand Down Expand Up @@ -5744,7 +5808,11 @@ void GDScriptParser::TreePrinter::print_if(IfNode *p_if, bool p_is_elif) {
} else {
push_text("If ");
}
print_expression(p_if->condition);
if (p_if->variable) {
print_variable(p_if->variable);
} else {
print_expression(p_if->condition);
}
push_line(" :");

increase_indent();
Expand Down
4 changes: 3 additions & 1 deletion modules/gdscript/gdscript_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,9 @@ class GDScriptParser {
};

struct IfNode : public Node {
VariableNode *variable = nullptr;
ExpressionNode *condition = nullptr;
SuiteNode *condition_block = nullptr;
SuiteNode *true_block = nullptr;
SuiteNode *false_block = nullptr;

Expand Down Expand Up @@ -1516,7 +1518,7 @@ class GDScriptParser {
// Statements.
Node *parse_statement();
VariableNode *parse_variable(bool p_is_static);
VariableNode *parse_variable(bool p_is_static, bool p_allow_property);
VariableNode *parse_variable(bool p_is_static, bool p_allow_property, bool p_is_if_condition = false);
VariableNode *parse_property(VariableNode *p_variable, bool p_need_indent);
void parse_property_getter(VariableNode *p_variable);
void parse_property_setter(VariableNode *p_variable);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
func test():
if var x = 100:
print("t")
elif x > 0:
print("f")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Identifier "x" not declared in the current scope.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
func test():
if var x = 100:
print("t")
else:
print(x)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Identifier "x" not declared in the current scope.
Loading
Loading