From 458c68775f4adee9df0f57081296c6d1c0d4e67e Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Wed, 19 Oct 2016 14:36:54 +0200 Subject: [PATCH] - fixed handling of loop statements. All break and continue statements were collected in one list, and each loop statement taking hold of that entire list, including the breaks and continues from previous outer loops. Changed it so that loop statements contain the jump addresses themselves and set a pointer in FCompileContext, so that the jump point can be set directly in the loop statement - and an error printed if there is none in the resolving stage, not the emitting one. Consolidated the identical backpatching code of the three loop statement nodes into a subfunction. --- src/scripting/codegeneration/codegen.cpp | 112 +++++++++-------------- src/scripting/codegeneration/codegen.h | 17 ++-- 2 files changed, 52 insertions(+), 77 deletions(-) diff --git a/src/scripting/codegeneration/codegen.cpp b/src/scripting/codegeneration/codegen.cpp index 7687be371..d5c70f493 100644 --- a/src/scripting/codegeneration/codegen.cpp +++ b/src/scripting/codegeneration/codegen.cpp @@ -5185,23 +5185,36 @@ ExpEmit FxIfStatement::Emit(VMFunctionBuilder *build) return ExpEmit(); } - //========================================================================== // -// FxLoopStatement +// FxLoopStatement :: Resolve +// +// saves the loop pointer in the context and sets this object as the current loop +// so that continues and breaks always resolve to the innermost loop. // //========================================================================== -void FxLoopStatement::HandleJumps(int token, FCompileContext &ctx) +FxExpression *FxLoopStatement::Resolve(FCompileContext &ctx) { - for (unsigned int i = 0; i < ctx.Jumps.Size(); i++) + auto outer = ctx.Loop; + ctx.Loop = this; + auto x = DoResolve(ctx); + ctx.Loop = outer; + return x; +} + +void FxLoopStatement::Backpatch(VMFunctionBuilder *build, size_t loopstart, size_t loopend) +{ + // Give a proper address to any break/continue statement within this loop. + for (unsigned int i = 0; i < Jumps.Size(); i++) { - if (ctx.Jumps[i]->Token == token) + if (Jumps[i]->Token == TK_Break) { - ctx.Jumps[i]->AddressResolver = this; - JumpAddresses.Push(ctx.Jumps[i]); - ctx.Jumps.Delete(i); - i--; + build->Backpatch(Jumps[i]->Address, loopend); + } + else + { // Continue statement. + build->Backpatch(Jumps[i]->Address, loopstart); } } } @@ -5224,15 +5237,12 @@ FxWhileLoop::~FxWhileLoop() SAFE_DELETE(Code); } -FxExpression *FxWhileLoop::Resolve(FCompileContext &ctx) +FxExpression *FxWhileLoop::DoResolve(FCompileContext &ctx) { CHECKRESOLVED(); SAFE_RESOLVE(Condition, ctx); SAFE_RESOLVE_OPT(Code, ctx); - HandleJumps(TK_Break, ctx); - HandleJumps(TK_Continue, ctx); - if (Condition->ValueType != TypeBool) { Condition = new FxBoolCast(Condition); @@ -5291,19 +5301,7 @@ ExpEmit FxWhileLoop::Emit(VMFunctionBuilder *build) build->Backpatch(jumpspot, loopend); } - // Give a proper address to any break/continue statement within this loop. - for (unsigned int i = 0; i < JumpAddresses.Size(); i++) - { - if (JumpAddresses[i]->Token == TK_Break) - { - build->Backpatch(JumpAddresses[i]->Address, loopend); - } - else - { // Continue statement. - build->Backpatch(JumpAddresses[i]->Address, loopstart); - } - } - + Backpatch(build, loopstart, loopend); return ExpEmit(); } @@ -5325,15 +5323,12 @@ FxDoWhileLoop::~FxDoWhileLoop() SAFE_DELETE(Code); } -FxExpression *FxDoWhileLoop::Resolve(FCompileContext &ctx) +FxExpression *FxDoWhileLoop::DoResolve(FCompileContext &ctx) { CHECKRESOLVED(); SAFE_RESOLVE(Condition, ctx); SAFE_RESOLVE_OPT(Code, ctx); - HandleJumps(TK_Break, ctx); - HandleJumps(TK_Continue, ctx); - if (Condition->ValueType != TypeBool) { Condition = new FxBoolCast(Condition); @@ -5344,7 +5339,7 @@ FxExpression *FxDoWhileLoop::Resolve(FCompileContext &ctx) { if (static_cast(Condition)->GetValue().GetBool() == false) { // The code executes once, if any. - if (JumpAddresses.Size() == 0) + if (Jumps.Size() == 0) { // We would still have to handle the jumps however. FxExpression *e = Code; if (e == nullptr) e = new FxNop(ScriptPosition); @@ -5393,18 +5388,7 @@ ExpEmit FxDoWhileLoop::Emit(VMFunctionBuilder *build) } loopend = build->GetAddress(); - // Give a proper address to any break/continue statement within this loop. - for (unsigned int i = 0; i < JumpAddresses.Size(); i++) - { - if (JumpAddresses[i]->Token == TK_Break) - { - build->Backpatch(JumpAddresses[i]->Address, loopend); - } - else - { // Continue statement. - build->Backpatch(JumpAddresses[i]->Address, loopstart); - } - } + Backpatch(build, loopstart, loopend); return ExpEmit(); } @@ -5429,7 +5413,7 @@ FxForLoop::~FxForLoop() SAFE_DELETE(Code); } -FxExpression *FxForLoop::Resolve(FCompileContext &ctx) +FxExpression *FxForLoop::DoResolve(FCompileContext &ctx) { CHECKRESOLVED(); SAFE_RESOLVE_OPT(Init, ctx); @@ -5437,9 +5421,6 @@ FxExpression *FxForLoop::Resolve(FCompileContext &ctx) SAFE_RESOLVE_OPT(Iteration, ctx); SAFE_RESOLVE_OPT(Code, ctx); - HandleJumps(TK_Break, ctx); - HandleJumps(TK_Continue, ctx); - if (Condition != nullptr) { if (Condition->ValueType != TypeBool) @@ -5480,7 +5461,7 @@ ExpEmit FxForLoop::Emit(VMFunctionBuilder *build) size_t codestart; size_t jumpspot; - // Init statement. + // Init statement (only used by DECORATE. ZScript is pulling it before the loop statement and enclosing the entire loop in a compound statement so that Init can have local variables.) if (Init != nullptr) { ExpEmit init = Init->Emit(build); @@ -5520,19 +5501,7 @@ ExpEmit FxForLoop::Emit(VMFunctionBuilder *build) build->Backpatch(jumpspot, loopend); } - // Give a proper address to any break/continue statement within this loop. - for (unsigned int i = 0; i < JumpAddresses.Size(); i++) - { - if (JumpAddresses[i]->Token == TK_Break) - { - build->Backpatch(JumpAddresses[i]->Address, loopend); - } - else - { // Continue statement. - build->Backpatch(JumpAddresses[i]->Address, loopstart); - } - } - + Backpatch(build, loopstart, loopend); return ExpEmit(); } @@ -5543,7 +5512,7 @@ ExpEmit FxForLoop::Emit(VMFunctionBuilder *build) //========================================================================== FxJumpStatement::FxJumpStatement(int token, const FScriptPosition &pos) -: FxExpression(pos), Token(token), AddressResolver(nullptr) +: FxExpression(pos), Token(token) { ValueType = TypeVoid; } @@ -5552,18 +5521,21 @@ FxExpression *FxJumpStatement::Resolve(FCompileContext &ctx) { CHECKRESOLVED(); - ctx.Jumps.Push(this); - - return this; + if (ctx.Loop != nullptr) + { + ctx.Loop->Jumps.Push(this); + return this; + } + else + { + ScriptPosition.Message(MSG_ERROR, "'%s' outside of a loop", Token == TK_Break ? "break" : "continue"); + delete this; + return nullptr; + } } ExpEmit FxJumpStatement::Emit(VMFunctionBuilder *build) { - if (AddressResolver == nullptr) - { - ScriptPosition.Message(MSG_ERROR, "Jump statement %s has nowhere to go!", FScanner::TokenName(Token).GetChars()); - } - Address = build->Emit(OP_JMP, 0); return ExpEmit(); diff --git a/src/scripting/codegeneration/codegen.h b/src/scripting/codegeneration/codegen.h index 1a2d2045d..c4e1753bd 100644 --- a/src/scripting/codegeneration/codegen.h +++ b/src/scripting/codegeneration/codegen.h @@ -62,10 +62,11 @@ class FxJumpStatement; // //========================================================================== struct FScriptPosition; +class FxLoopStatement; struct FCompileContext { - TArray Jumps; + FxLoopStatement *Loop; PPrototype *ReturnProto; PFunction *Function; // The function that is currently being compiled (or nullptr for constant evaluation.) PClass *Class; // The type of the owning class. @@ -1127,9 +1128,12 @@ protected: { } - void HandleJumps(int token, FCompileContext &ctx); + void Backpatch(VMFunctionBuilder *build, size_t loopstart, size_t loopend); + FxExpression *Resolve(FCompileContext&) final; + virtual FxExpression *DoResolve(FCompileContext&) = 0; - TArray JumpAddresses; +public: + TArray Jumps; }; //========================================================================== @@ -1146,7 +1150,7 @@ class FxWhileLoop : public FxLoopStatement public: FxWhileLoop(FxExpression *condition, FxExpression *code, const FScriptPosition &pos); ~FxWhileLoop(); - FxExpression *Resolve(FCompileContext&); + FxExpression *DoResolve(FCompileContext&); ExpEmit Emit(VMFunctionBuilder *build); }; @@ -1164,7 +1168,7 @@ class FxDoWhileLoop : public FxLoopStatement public: FxDoWhileLoop(FxExpression *condition, FxExpression *code, const FScriptPosition &pos); ~FxDoWhileLoop(); - FxExpression *Resolve(FCompileContext&); + FxExpression *DoResolve(FCompileContext&); ExpEmit Emit(VMFunctionBuilder *build); }; @@ -1184,7 +1188,7 @@ class FxForLoop : public FxLoopStatement public: FxForLoop(FxExpression *init, FxExpression *condition, FxExpression *iteration, FxExpression *code, const FScriptPosition &pos); ~FxForLoop(); - FxExpression *Resolve(FCompileContext&); + FxExpression *DoResolve(FCompileContext&); ExpEmit Emit(VMFunctionBuilder *build); }; @@ -1203,7 +1207,6 @@ public: int Token; size_t Address; - FxExpression *AddressResolver; }; //==========================================================================