From 5350e1beaa149725561fde8740e59eca7876394d Mon Sep 17 00:00:00 2001 From: Danil Alexeev Date: Thu, 25 Jul 2024 11:56:30 +0300 Subject: [PATCH] GDScript: Fix locals clearing after exiting `while` block --- modules/gdscript/gdscript_compiler.cpp | 18 ++++++++++++----- .../features/reset_local_var_on_exit_block.gd | 20 +++++++++++++++++-- .../reset_local_var_on_exit_block.out | 1 + 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 13707de12ab..d8b44a558f6 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -1901,7 +1901,7 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui case GDScriptParser::Node::MATCH: { const GDScriptParser::MatchNode *match = static_cast(s); - codegen.start_block(); // Add an extra block, since the binding pattern and @special variables belong to the branch scope. + codegen.start_block(); // Add an extra block, since @special locals belong to the match scope. // Evaluate the match expression. GDScriptCodeGenerator::Address value = codegen.add_local("@match_value", _gdtype_from_datatype(match->test->get_datatype(), codegen.script)); @@ -1939,7 +1939,7 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui const GDScriptParser::MatchBranchNode *branch = match->branches[j]; - codegen.start_block(); // Create an extra block around for binds. + codegen.start_block(); // Add an extra block, since binds belong to the match branch scope. // Add locals in block before patterns, so temporaries don't use the stack address for binds. List branch_locals = _add_block_locals(codegen, branch->block); @@ -1991,13 +1991,15 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui _clear_block_locals(codegen, branch_locals); - codegen.end_block(); // Get out of extra block. + codegen.end_block(); // Get out of extra block for binds. } // End all nested `if`s. for (int j = 0; j < match->branches.size(); j++) { gen->write_endif(); } + + codegen.end_block(); // Get out of extra block for match's @special locals. } break; case GDScriptParser::Node::IF: { const GDScriptParser::IfNode *if_n = static_cast(s); @@ -2031,7 +2033,9 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui case GDScriptParser::Node::FOR: { const GDScriptParser::ForNode *for_n = static_cast(s); - codegen.start_block(); // Add an extra block, since the iterator and @special variables belong to the loop scope. + // Add an extra block, since the iterator and @special locals belong to the loop scope. + // Also we use custom logic to clear block locals. + codegen.start_block(); GDScriptCodeGenerator::Address iterator = codegen.add_local(for_n->variable->name, _gdtype_from_datatype(for_n->variable->get_datatype(), codegen.script)); @@ -2064,11 +2068,13 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui _clear_block_locals(codegen, loop_locals); // Outside loop, after block - for `break` and normal exit. - codegen.end_block(); // Get out of extra block. + codegen.end_block(); // Get out of extra block for loop iterator, @special locals, and custom locals clearing. } break; case GDScriptParser::Node::WHILE: { const GDScriptParser::WhileNode *while_n = static_cast(s); + codegen.start_block(); // Add an extra block, since we use custom logic to clear block locals. + gen->start_while_condition(); GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, while_n->condition); @@ -2095,6 +2101,8 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui gen->write_endwhile(); _clear_block_locals(codegen, loop_locals); // Outside loop, after block - for `break` and normal exit. + + codegen.end_block(); // Get out of extra block for custom locals clearing. } break; case GDScriptParser::Node::BREAK: { gen->write_break(); diff --git a/modules/gdscript/tests/scripts/runtime/features/reset_local_var_on_exit_block.gd b/modules/gdscript/tests/scripts/runtime/features/reset_local_var_on_exit_block.gd index c774ebf83c4..df639a7b4da 100644 --- a/modules/gdscript/tests/scripts/runtime/features/reset_local_var_on_exit_block.gd +++ b/modules/gdscript/tests/scripts/runtime/features/reset_local_var_on_exit_block.gd @@ -1,6 +1,5 @@ # GH-77666 - -func test(): +func test_exit_if(): var ref := RefCounted.new() print(ref.get_reference_count()) @@ -8,3 +7,20 @@ func test(): var _temp := ref print(ref.get_reference_count()) + +# GH-94654 +func test_exit_while(): + var slots_data := [] + + while true: + @warning_ignore("confusable_local_declaration") + var slot = 42 + slots_data.append(slot) + break + + var slot: int = slots_data[0] + print(slot) + +func test(): + test_exit_if() + test_exit_while() diff --git a/modules/gdscript/tests/scripts/runtime/features/reset_local_var_on_exit_block.out b/modules/gdscript/tests/scripts/runtime/features/reset_local_var_on_exit_block.out index 04b4638adf6..164eb249637 100644 --- a/modules/gdscript/tests/scripts/runtime/features/reset_local_var_on_exit_block.out +++ b/modules/gdscript/tests/scripts/runtime/features/reset_local_var_on_exit_block.out @@ -1,3 +1,4 @@ GDTEST_OK 1 1 +42