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 mixing multiple variable definitions and expressions in if-statement #98335

Open
wants to merge 2 commits 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
11 changes: 10 additions & 1 deletion modules/gdscript/editor/gdscript_translation_parser_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,16 @@ void GDScriptEditorTranslationParserPlugin::_traverse_block(const GDScriptParser
} break;
case GDScriptParser::Node::IF: {
const GDScriptParser::IfNode *if_node = static_cast<const GDScriptParser::IfNode *>(statement);
_assess_expression(if_node->condition);
const List<GDScriptParser::Node *>::Element *E = if_node->conditions.front();
while (E) {
const GDScriptParser::Node *node = E->get();
if (node->is_expression()) {
_assess_expression(static_cast<const GDScriptParser::ExpressionNode *>(node));
} else if (node->type == GDScriptParser::Node::VARIABLE) {
_assess_expression(static_cast<const GDScriptParser::VariableNode *>(node)->initializer);
}
E = E->next();
}
_traverse_block(if_node->true_block);
_traverse_block(if_node->false_block);
} break;
Expand Down
11 changes: 10 additions & 1 deletion modules/gdscript/gdscript_analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2096,7 +2096,16 @@ void GDScriptAnalyzer::resolve_parameter(GDScriptParser::ParameterNode *p_parame
}

void GDScriptAnalyzer::resolve_if(GDScriptParser::IfNode *p_if) {
reduce_expression(p_if->condition);
List<GDScriptParser::Node *>::Element *E = p_if->conditions.front();
while (E) {
GDScriptParser::Node *condition = E->get();
if (condition->is_expression()) {
reduce_expression(static_cast<GDScriptParser::ExpressionNode *>(condition));
} else if (condition->type == GDScriptParser::Node::VARIABLE) {
resolve_variable(static_cast<GDScriptParser::VariableNode *>(condition), true);
}
E = E->next();
}

resolve_suite(p_if->true_block);
p_if->set_datatype(p_if->true_block->get_datatype());
Expand Down
9 changes: 6 additions & 3 deletions modules/gdscript/gdscript_byte_codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1501,13 +1501,16 @@ void GDScriptByteCodeGenerator::write_if(const Address &p_condition) {
append(0); // Jump destination, will be patched.
}

void GDScriptByteCodeGenerator::write_else() {
void GDScriptByteCodeGenerator::write_else(int count) {
append_opcode(GDScriptFunction::OPCODE_JUMP); // Jump from true if block;
int else_jmp_addr = opcodes.size();
append(0); // Jump destination, will be patched.

patch_jump(if_jmp_addrs.back()->get());
if_jmp_addrs.pop_back();
for (int i = 0; i < count; i++) {
patch_jump(if_jmp_addrs.back()->get());
if_jmp_addrs.pop_back();
}

if_jmp_addrs.push_back(else_jmp_addr);
}

Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_byte_codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
virtual void write_construct_typed_dictionary(const Address &p_target, const GDScriptDataType &p_key_type, const GDScriptDataType &p_value_type, const Vector<Address> &p_arguments) override;
virtual void write_await(const Address &p_target, const Address &p_operand) override;
virtual void write_if(const Address &p_condition) override;
virtual void write_else() override;
virtual void write_else(int count) override;
virtual void write_endif() override;
virtual void write_jump_if_shared(const Address &p_value) override;
virtual void write_end_jump_if_shared() override;
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class GDScriptCodeGenerator {
virtual void write_construct_typed_dictionary(const Address &p_target, const GDScriptDataType &p_key_type, const GDScriptDataType &p_value_type, const Vector<Address> &p_arguments) = 0;
virtual void write_await(const Address &p_target, const Address &p_operand) = 0;
virtual void write_if(const Address &p_condition) = 0;
virtual void write_else() = 0;
virtual void write_else(int count) = 0;
virtual void write_endif() = 0;
virtual void write_jump_if_shared(const Address &p_value) = 0;
virtual void write_end_jump_if_shared() = 0;
Expand Down
165 changes: 111 additions & 54 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,72 @@ 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);

int count = 0;
const List<GDScriptParser::Node *>::Element *E = if_n->conditions.front();
while (E) {
const GDScriptParser::Node *condition_n = E->get();
if (condition_n->is_expression()) {
const GDScriptParser::ExpressionNode *expression_n = static_cast<const GDScriptParser::ExpressionNode *>(condition_n);

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

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

count++;
} else if (condition_n->type == GDScriptParser::Node::VARIABLE) {
const GDScriptParser::VariableNode *variable_n = static_cast<const GDScriptParser::VariableNode *>(condition_n);

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

gen->clear_temporaries();
}
E = E->next();
}

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(count);

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

gen->write_endif();
} else {
for (int i = 0; i < count; i++) {
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 @@ -1935,7 +2040,7 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
for (int j = 0; j < match->branches.size(); j++) {
if (j > 0) {
// Use `else` to not check the next branch after matching.
gen->write_else();
gen->write_else(1);
}

const GDScriptParser::MatchBranchNode *branch = match->branches[j];
Expand Down Expand Up @@ -2004,32 +2109,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);
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);
err = _parse_if(codegen, if_n);
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 +2249,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
42 changes: 25 additions & 17 deletions modules/gdscript/gdscript_editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2205,25 +2205,33 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
}
}

if (suite->parent_if && suite->parent_if->condition && suite->parent_if->condition->type == GDScriptParser::Node::TYPE_TEST) {
// Operator `is` used, check if identifier is in there! this helps resolve in blocks that are (if (identifier is value)): which are very common..
// Super dirty hack, but very useful.
// Credit: Zylann.
// TODO: this could be hacked to detect ANDed conditions too...
const GDScriptParser::TypeTestNode *type_test = static_cast<const GDScriptParser::TypeTestNode *>(suite->parent_if->condition);
if (type_test->operand && type_test->test_type && type_test->operand->type == GDScriptParser::Node::IDENTIFIER && static_cast<const GDScriptParser::IdentifierNode *>(type_test->operand)->name == p_identifier->name && static_cast<const GDScriptParser::IdentifierNode *>(type_test->operand)->source == p_identifier->source) {
// Bingo.
GDScriptParser::CompletionContext c = p_context;
c.current_line = type_test->operand->start_line;
c.current_suite = suite;
if (type_test->test_datatype.is_hard_type()) {
id_type.type = type_test->test_datatype;
if (last_assign_line < c.current_line) {
// Override last assignment.
last_assign_line = c.current_line;
last_assigned_expression = nullptr;
if (suite->parent_if) {
List<GDScriptParser::Node *>::Element *E = suite->parent_if->conditions.front();
while (E) {
GDScriptParser::Node *condition = E->get();
if (condition->is_expression() && condition->type == GDScriptParser::Node::TYPE_TEST) {
// Operator `is` used, check if identifier is in there! this helps resolve in blocks that are (if (identifier is value)): which are very common..
// Super dirty hack, but very useful.
// Credit: Zylann.
// TODO: this could be hacked to detect ANDed conditions too...
const GDScriptParser::TypeTestNode *type_test = static_cast<const GDScriptParser::TypeTestNode *>(condition);
if (type_test->operand && type_test->test_type && type_test->operand->type == GDScriptParser::Node::IDENTIFIER && static_cast<const GDScriptParser::IdentifierNode *>(type_test->operand)->name == p_identifier->name && static_cast<const GDScriptParser::IdentifierNode *>(type_test->operand)->source == p_identifier->source) {
// Bingo.
GDScriptParser::CompletionContext c = p_context;
c.current_line = type_test->operand->start_line;
c.current_suite = suite;
if (type_test->test_datatype.is_hard_type()) {
id_type.type = type_test->test_datatype;
if (last_assign_line < c.current_line) {
// Override last assignment.
last_assign_line = c.current_line;
last_assigned_expression = nullptr;
}
}
break;
}
}
E = E->next();
}
}

Expand Down
Loading
Loading