From 8fc015144467ef299c2fba83996cdc2bcef0455b Mon Sep 17 00:00:00 2001 From: melex750 Date: Mon, 16 Jan 2017 11:38:34 -0500 Subject: [PATCH] Fix increment and decrement syntax --- src/CBot/CBotInstr/CBotExprVar.cpp | 8 +++-- src/CBot/CBotInstr/CBotExprVar.h | 11 +++--- src/CBot/CBotInstr/CBotFieldExpr.cpp | 5 ++- src/CBot/CBotInstr/CBotFieldExpr.h | 7 ++-- src/CBot/CBotInstr/CBotLeftExpr.cpp | 5 ++- src/CBot/CBotInstr/CBotParExpr.cpp | 29 +++++++-------- test/unit/CBot/CBot_test.cpp | 53 ++++++++++++++++++++++++++++ 7 files changed, 83 insertions(+), 35 deletions(-) diff --git a/src/CBot/CBotInstr/CBotExprVar.cpp b/src/CBot/CBotInstr/CBotExprVar.cpp index a15c9c9a..ed77d8ac 100644 --- a/src/CBot/CBotInstr/CBotExprVar.cpp +++ b/src/CBot/CBotInstr/CBotExprVar.cpp @@ -44,7 +44,7 @@ CBotExprVar::~CBotExprVar() } //////////////////////////////////////////////////////////////////////////////// -CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::ProtectionLevel privat) +CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, bool bCheckReadOnly) { // CBotToken* pDebut = p; CBotCStack* pStk = pStack->TokenStack(); @@ -67,7 +67,7 @@ CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::Prot if (ident > 0 && ident < 9000) { - if (CBotFieldExpr::CheckProtectionError(pStk, nullptr, var, privat)) + if (CBotFieldExpr::CheckProtectionError(pStk, nullptr, var, bCheckReadOnly)) { pStk->SetError(CBotErrPrivate, p); goto err; @@ -122,6 +122,8 @@ CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::Prot { if (p->GetNext()->GetType() == ID_OPENPAR) // a method call? { + if (bCheckReadOnly) goto err; // don't allow increment a method call "++" + CBotInstr* i = CBotInstrMethode::Compile(p, pStk, var); if (!pStk->IsOk()) goto err; inst->AddNext3(i); // added after @@ -137,7 +139,7 @@ CBotInstr* CBotExprVar::Compile(CBotToken*& p, CBotCStack* pStack, CBotVar::Prot if (var != nullptr) { i->SetUniqNum(var->GetUniqNum()); - if (CBotFieldExpr::CheckProtectionError(pStk, preVar, var, privat)) + if (CBotFieldExpr::CheckProtectionError(pStk, preVar, var, bCheckReadOnly)) { pStk->SetError(CBotErrPrivate, pp); goto err; diff --git a/src/CBot/CBotInstr/CBotExprVar.h b/src/CBot/CBotInstr/CBotExprVar.h index 9cf68991..2f2e69ab 100644 --- a/src/CBot/CBotInstr/CBotExprVar.h +++ b/src/CBot/CBotInstr/CBotExprVar.h @@ -40,14 +40,13 @@ public: ~CBotExprVar(); /*! - * \brief Compile - * \param p - * \param pStack - * \param privat + * \brief Compile an expression of a variable, possibly chained with index operators and/or dot operators + * \param p[in, out] Pointer to first token of the expression, will be updated to point to first token after the expression + * \param pStack Current compilation stack frame + * \param bCheckReadOnly True for operations that would modify the value of the variable * \return */ - static CBotInstr* Compile(CBotToken*& p, CBotCStack* pStack, - CBotVar::ProtectionLevel privat = CBotVar::ProtectionLevel::Protected); + static CBotInstr* Compile(CBotToken*& p, CBotCStack* pStack, bool bCheckReadOnly = false); /*! * \brief CompileMethode diff --git a/src/CBot/CBotInstr/CBotFieldExpr.cpp b/src/CBot/CBotInstr/CBotFieldExpr.cpp index 09ac5f8e..04635a1a 100644 --- a/src/CBot/CBotInstr/CBotFieldExpr.cpp +++ b/src/CBot/CBotInstr/CBotFieldExpr.cpp @@ -135,12 +135,11 @@ std::string CBotFieldExpr::GetDebugData() } //////////////////////////////////////////////////////////////////////////////// -bool CBotFieldExpr::CheckProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar, - CBotVar::ProtectionLevel privat) +bool CBotFieldExpr::CheckProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar, bool bCheckReadOnly) { CBotVar::ProtectionLevel varPriv = pVar->GetPrivate(); - if (privat == CBotVar::ProtectionLevel::ReadOnly && varPriv == privat) + if (bCheckReadOnly && varPriv == CBotVar::ProtectionLevel::ReadOnly) return true; if (varPriv == CBotVar::ProtectionLevel::Public) return false; diff --git a/src/CBot/CBotInstr/CBotFieldExpr.h b/src/CBot/CBotInstr/CBotFieldExpr.h index 67ab2f05..5f093bd7 100644 --- a/src/CBot/CBotInstr/CBotFieldExpr.h +++ b/src/CBot/CBotInstr/CBotFieldExpr.h @@ -72,13 +72,12 @@ public: * This function doesn't set the error flag itself. * * \param pStack Current compilation stack frame - * \param pPrev Class instance which variable to check is part of, or nullptr if not part of a class + * \param pPrev Class instance which variable to check is part of, or nullptr when compiler inserts 'this.' before * \param pVar Variable to check - * \param privat CBotVar::ProtectionLevel::ReadOnly if requesting read-only access, anything else otherwise + * \param bCheckReadOnly True for operations that would modify the value of the variable * \return true if pVar is inaccessible in the current context, false if access should be allowed */ - static bool CheckProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar, - CBotVar::ProtectionLevel privat = CBotVar::ProtectionLevel::Protected); + static bool CheckProtectionError(CBotCStack* pStack, CBotVar* pPrev, CBotVar* pVar, bool bCheckReadOnly = false); protected: virtual const std::string GetDebugName() override { return "CBotFieldExpr"; } diff --git a/src/CBot/CBotInstr/CBotLeftExpr.cpp b/src/CBot/CBotInstr/CBotLeftExpr.cpp index 4678ff8e..4c4400a9 100644 --- a/src/CBot/CBotInstr/CBotLeftExpr.cpp +++ b/src/CBot/CBotInstr/CBotLeftExpr.cpp @@ -64,7 +64,7 @@ CBotLeftExpr* CBotLeftExpr::Compile(CBotToken* &p, CBotCStack* pStack) inst->m_nIdent = var->GetUniqNum(); if (inst->m_nIdent > 0 && inst->m_nIdent < 9000) { - if (CBotFieldExpr::CheckProtectionError(pStk, nullptr, var, CBotVar::ProtectionLevel::ReadOnly)) + if (CBotFieldExpr::CheckProtectionError(pStk, nullptr, var, true)) { pStk->SetError(CBotErrPrivate, p); goto err; @@ -128,8 +128,7 @@ CBotLeftExpr* CBotLeftExpr::Compile(CBotToken* &p, CBotCStack* pStack) var = var->GetItem(p->GetString()); // get item correspondent if (var != nullptr) { - if (CBotFieldExpr::CheckProtectionError(pStk, preVar, var, - CBotVar::ProtectionLevel::ReadOnly)) + if (CBotFieldExpr::CheckProtectionError(pStk, preVar, var, true)) { pStk->SetError(CBotErrPrivate, pp); goto err; diff --git a/src/CBot/CBotInstr/CBotParExpr.cpp b/src/CBot/CBotInstr/CBotParExpr.cpp index 714e68e4..a32363fb 100644 --- a/src/CBot/CBotInstr/CBotParExpr.cpp +++ b/src/CBot/CBotInstr/CBotParExpr.cpp @@ -90,17 +90,16 @@ CBotInstr* CBotParExpr::Compile(CBotToken* &p, CBotCStack* pStack) // post incremented or decremented? if (IsOfType(p, ID_INC, ID_DEC)) { + // recompile the variable for read-only + delete inst; + p = pvar; + inst = CBotExprVar::Compile(p, pStk, true); if (pStk->GetType() >= CBotTypBoolean) { pStk->SetError(CBotErrBadType1, pp); delete inst; return pStack->Return(nullptr, pStk); } - - // recompile the variable for read-only - delete inst; - p = pvar; - inst = CBotExprVar::Compile(p, pStk, CBotVar::ProtectionLevel::ReadOnly); p = p->GetNext(); CBotPostIncExpr* i = new CBotPostIncExpr(); @@ -115,24 +114,22 @@ CBotInstr* CBotParExpr::Compile(CBotToken* &p, CBotCStack* pStack) CBotToken* pp = p; if (IsOfType(p, ID_INC, ID_DEC)) { - CBotPreIncExpr* i = new CBotPreIncExpr(); - i->SetToken(pp); - if (p->GetType() == TokenTypVar) { - if (nullptr != (i->m_instr = CBotExprVar::Compile(p, pStk, CBotVar::ProtectionLevel::ReadOnly))) + if (nullptr != (inst = CBotExprVar::Compile(p, pStk, true))) { - if (pStk->GetType() >= CBotTypBoolean) + if (pStk->GetType() < CBotTypBoolean) // a number ? { - pStk->SetError(CBotErrBadType1, pp); - delete inst; - return pStack->Return(nullptr, pStk); + CBotPreIncExpr* i = new CBotPreIncExpr(); + i->SetToken(pp); + i->m_instr = inst; + return pStack->Return(i, pStk); } - return pStack->Return(i, pStk); + delete inst; } - delete i; - return pStack->Return(nullptr, pStk); } + pStk->SetError(CBotErrBadType1, pp); + return pStack->Return(nullptr, pStk); } // is it a number or DefineNum? diff --git a/test/unit/CBot/CBot_test.cpp b/test/unit/CBot/CBot_test.cpp index 6cf28413..e456261b 100644 --- a/test/unit/CBot/CBot_test.cpp +++ b/test/unit/CBot/CBot_test.cpp @@ -2073,3 +2073,56 @@ TEST_F(CBotUT, ClassTestPrivateMember) CBotErrPrivate ); } + +TEST_F(CBotUT, IncrementDecrementSyntax) +{ + auto publicProgram = ExecuteTest( + "public class TestClass {\n" + " int GetInt() { return 1; }\n" + "}\n" + "extern void TestIncrementDecrement()\n" + "{\n" + " int i = 1;\n" + " ASSERT(2 == ++i);\n" + " ASSERT(2 == i++);\n" + " ASSERT(3 == i );\n" + " ASSERT(2 == --i);\n" + " ASSERT(2 == i--);\n" + " ASSERT(1 == i );\n" + "}\n" + ); + + ExecuteTest( + "extern void PreIncrementMethodCall()\n" + "{\n" + " TestClass tc();\n" + " ++tc.GetInt();\n" + "}\n", + CBotErrBadType1 + ); + + ExecuteTest( + "extern void PostIncrementMethodCall()\n" + "{\n" + " TestClass tc();\n" + " tc.GetInt()++;\n" + "}\n", + CBotErrBadType1 + ); + + ExecuteTest( + "extern void BadPreIncrementEmpty()\n" + "{\n" + " ++;\n" + "}\n", + CBotErrBadType1 + ); + + ExecuteTest( + "extern void BadPreIncrementNotAVar()\n" + "{\n" + " ++123;\n" + "}\n", + CBotErrBadType1 + ); +}