From 2ff8251a811b6da93062bb885b8978afade0ef12 Mon Sep 17 00:00:00 2001 From: melex750 Date: Tue, 24 Jan 2017 13:47:00 -0500 Subject: [PATCH 1/3] Fix game crashing with syntax errors --- po/colobot.pot | 3 + po/de.po | 3 + po/fr.po | 3 + po/pl.po | 3 + po/ru.po | 3 + src/CBot/CBotClass.cpp | 25 ++++---- src/CBot/CBotDefParam.cpp | 10 ++-- src/CBot/CBotEnums.h | 1 + src/CBot/CBotInstr/CBotFunction.cpp | 15 +++-- src/CBot/CBotInstr/CBotListInstr.cpp | 2 +- src/CBot/CBotToken.cpp | 7 +++ src/common/restext.cpp | 1 + test/unit/CBot/CBotToken_test.cpp | 2 +- test/unit/CBot/CBot_test.cpp | 90 ++++++++++++++++++++++++++++ 14 files changed, 144 insertions(+), 24 deletions(-) diff --git a/po/colobot.pot b/po/colobot.pot index fd5c3de4..04eda131 100644 --- a/po/colobot.pot +++ b/po/colobot.pot @@ -1739,6 +1739,9 @@ msgstr "" msgid "Function needs return type \"void\"" msgstr "" +msgid "Class name expected" +msgstr "" + msgid "Dividing by zero" msgstr "" diff --git a/po/de.po b/po/de.po index e4c105af..440febf5 100644 --- a/po/de.po +++ b/po/de.po @@ -356,6 +356,9 @@ msgstr "" msgid "Checkpoint" msgstr "Checkpoint" +msgid "Class name expected" +msgstr "" + msgid "Climb\\Increases the power of the jet" msgstr "Steigen\\Leistung des Triebwerks steigern" diff --git a/po/fr.po b/po/fr.po index 905a0163..98c75496 100644 --- a/po/fr.po +++ b/po/fr.po @@ -346,6 +346,9 @@ msgstr "Console de triche\\Montre la console de triche" msgid "Checkpoint" msgstr "Indicateur" +msgid "Class name expected" +msgstr "" + msgid "Climb\\Increases the power of the jet" msgstr "Monter\\Augmenter la puissance du réacteur" diff --git a/po/pl.po b/po/pl.po index cd9f37c8..f298098a 100644 --- a/po/pl.po +++ b/po/pl.po @@ -348,6 +348,9 @@ msgstr "Konsola komend\\Pokaż konsolę komend" msgid "Checkpoint" msgstr "Punkt kontrolny" +msgid "Class name expected" +msgstr "" + msgid "Climb\\Increases the power of the jet" msgstr "W górę\\Zwiększa moc silnika" diff --git a/po/ru.po b/po/ru.po index 233cb300..4042a185 100644 --- a/po/ru.po +++ b/po/ru.po @@ -353,6 +353,9 @@ msgstr "Консоль чит-кодов\\Показать консоль для msgid "Checkpoint" msgstr "Контрольная точка" +msgid "Class name expected" +msgstr "" + msgid "Climb\\Increases the power of the jet" msgstr "Взлет и подъем\\Увеличивает мощность реактивного двигателя" diff --git a/src/CBot/CBotClass.cpp b/src/CBot/CBotClass.cpp index c19cbf4b..d787eb73 100644 --- a/src/CBot/CBotClass.cpp +++ b/src/CBot/CBotClass.cpp @@ -471,26 +471,27 @@ CBotClass* CBotClass::Compile1(CBotToken* &p, CBotCStack* pStack) std::string name = p->GetString(); - CBotClass* pOld = CBotClass::Find(name); - if ( (pOld != nullptr && pOld->m_IsDef) || /* public class exists in different program */ - pStack->GetProgram()->ClassExists(name)) /* class exists in this program */ - { - pStack->SetError( CBotErrRedefClass, p ); - return nullptr; - } - // a name of the class is there? if (IsOfType(p, TokenTypVar)) { + CBotClass* pOld = CBotClass::Find(name); + if ((pOld != nullptr && pOld->m_IsDef) || /* public class exists in different program */ + pStack->GetProgram()->ClassExists(name)) /* class exists in this program */ + { + pStack->SetError(CBotErrRedefClass, p->GetPrev()); + return nullptr; + } + CBotClass* pPapa = nullptr; if ( IsOfType( p, ID_EXTENDS ) ) { std::string name = p->GetString(); pPapa = CBotClass::Find(name); + CBotToken* pp = p; if (!IsOfType(p, TokenTypVar) || pPapa == nullptr ) { - pStack->SetError( CBotErrNotClass, p ); + pStack->SetError(CBotErrNoClassName, pp); return nullptr; } } @@ -519,6 +520,9 @@ CBotClass* CBotClass::Compile1(CBotToken* &p, CBotCStack* pStack) if (pStack->IsOk()) return classe; } + else + pStack->SetError(CBotErrNoClassName, p); + pStack->SetError(CBotErrNoTerminator, p); return nullptr; } @@ -810,10 +814,11 @@ CBotClass* CBotClass::Compile(CBotToken* &p, CBotCStack* pStack) // TODO: Not sure how correct is that - I have no idea how the precompilation (Compile1 method) works ~krzys_h std::string name = p->GetString(); CBotClass* pPapa = CBotClass::Find(name); + CBotToken* pp = p; if (!IsOfType(p, TokenTypVar) || pPapa == nullptr) { - pStack->SetError( CBotErrNotClass, p ); + pStack->SetError(CBotErrNoClassName, pp); return nullptr; } pOld->m_parent = pPapa; diff --git a/src/CBot/CBotDefParam.cpp b/src/CBot/CBotDefParam.cpp index ff4acf1f..e66c8622 100644 --- a/src/CBot/CBotDefParam.cpp +++ b/src/CBot/CBotDefParam.cpp @@ -52,7 +52,7 @@ CBotDefParam* CBotDefParam::Compile(CBotToken* &p, CBotCStack* pStack) { CBotDefParam* list = nullptr; - while (!IsOfType(p, ID_CLOSEPAR)) + if (!IsOfType(p, ID_CLOSEPAR)) while (true) { CBotDefParam* param = new CBotDefParam(); if (list == nullptr) list = param; @@ -85,10 +85,12 @@ CBotDefParam* CBotDefParam::Compile(CBotToken* &p, CBotCStack* pStack) var->SetUniqNum(param->m_nIdent); pStack->AddVar(var); // place on the stack - if (IsOfType(p, ID_COMMA) || p->GetType() == ID_CLOSEPAR) - continue; + if (IsOfType(p, ID_COMMA)) continue; + if (IsOfType(p, ID_CLOSEPAR)) break; + + pStack->SetError(CBotErrClosePar, p->GetStart()); } - pStack->SetError(CBotErrClosePar, p->GetStart()); + pStack->SetError(CBotErrNoVar, p->GetStart()); } pStack->SetError(CBotErrNoType, p); delete list; diff --git a/src/CBot/CBotEnums.h b/src/CBot/CBotEnums.h index b7453e59..a34c9e7c 100644 --- a/src/CBot/CBotEnums.h +++ b/src/CBot/CBotEnums.h @@ -238,6 +238,7 @@ enum CBotError : int CBotErrNoExpression = 5043, //!< expression expected after = CBotErrAmbiguousCall = 5044, //!< ambiguous call to overloaded function CBotErrFuncNotVoid = 5045, //!< function needs return type "void" + CBotErrNoClassName = 5046, //!< class name expected // Runtime errors CBotErrZeroDiv = 6000, //!< division by zero diff --git a/src/CBot/CBotInstr/CBotFunction.cpp b/src/CBot/CBotInstr/CBotFunction.cpp index ddbd3667..f08ddd9e 100644 --- a/src/CBot/CBotInstr/CBotFunction.cpp +++ b/src/CBot/CBotInstr/CBotFunction.cpp @@ -177,7 +177,11 @@ CBotFunction* CBotFunction::Compile(CBotToken* &p, CBotCStack* pStack, CBotFunct func->m_MasterClass = pp->GetString(); func->m_classToken = *pp; CBotClass* pClass = CBotClass::Find(pp); - if ( pClass == nullptr ) goto bad; + if ( pClass == nullptr ) + { + pStk->SetError(CBotErrNoClassName, pp); + goto bad; + } // pp = p; func->m_token = *p; @@ -280,13 +284,8 @@ CBotFunction* CBotFunction::Compile1(CBotToken* &p, CBotCStack* pStack, CBotClas if ( IsOfType( p, ID_DBLDOTS ) ) // method for a class { func->m_MasterClass = pp->GetString(); - CBotClass* pClass = CBotClass::Find(pp); - if ( pClass == nullptr ) - { - pStk->SetError(CBotErrNotClass, pp); - goto bad; - } - + // existence of the class is checked + // later in CBotFunction::Compile() pp = p; func->m_token = *p; if (!IsOfType(p, TokenTypVar)) goto bad; diff --git a/src/CBot/CBotInstr/CBotListInstr.cpp b/src/CBot/CBotInstr/CBotListInstr.cpp index 58413a42..0ae396f5 100644 --- a/src/CBot/CBotInstr/CBotListInstr.cpp +++ b/src/CBot/CBotInstr/CBotListInstr.cpp @@ -52,7 +52,7 @@ CBotInstr* CBotListInstr::Compile(CBotToken* &p, CBotCStack* pStack, bool bLocal if (IsOfType(p, ID_SEP)) continue; // empty statement ignored if (p->GetType() == ID_CLBLK) break; - if (IsOfType(p, 0)) + if (p->GetType() == TokenTypNone) { pStack->SetError(CBotErrCloseBlock, p->GetStart()); delete inst; diff --git a/src/CBot/CBotToken.cpp b/src/CBot/CBotToken.cpp index 28e6690c..e10901eb 100644 --- a/src/CBot/CBotToken.cpp +++ b/src/CBot/CBotToken.cpp @@ -439,6 +439,13 @@ std::unique_ptr CBotToken::CompileTokens(const std::string& program) pp = p; } + // terminator token + nxt = new CBotToken(); + nxt->m_type = TokenTypNone; + nxt->m_end = nxt->m_start = pos; + prv->m_next = nxt; + nxt->m_prev = prv; + return std::unique_ptr(tokenbase); } diff --git a/src/common/restext.cpp b/src/common/restext.cpp index 789c68a4..93bab6c7 100644 --- a/src/common/restext.cpp +++ b/src/common/restext.cpp @@ -720,6 +720,7 @@ void InitializeRestext() stringsCbot[CBot::CBotErrNoExpression] = TR("Expression expected after ="); stringsCbot[CBot::CBotErrAmbiguousCall] = TR("Ambiguous call to overloaded function"); stringsCbot[CBot::CBotErrFuncNotVoid] = TR("Function needs return type \"void\""); + stringsCbot[CBot::CBotErrNoClassName] = TR("Class name expected"); stringsCbot[CBot::CBotErrZeroDiv] = TR("Dividing by zero"); stringsCbot[CBot::CBotErrNotInit] = TR("Variable not initialized"); diff --git a/test/unit/CBot/CBotToken_test.cpp b/test/unit/CBot/CBotToken_test.cpp index b51ca93e..a36841a5 100644 --- a/test/unit/CBot/CBotToken_test.cpp +++ b/test/unit/CBot/CBotToken_test.cpp @@ -60,7 +60,7 @@ protected: ASSERT_EQ(token->GetType(), correct.type) << "type mismatch at token #" << (i+1); i++; } - while((token = token->GetNext()) != nullptr); + while((token = token->GetNext()) != nullptr && !IsOfType(token, TokenTypNone)); ASSERT_EQ(i, data.size()) << "not enough tokens processed"; } }; diff --git a/test/unit/CBot/CBot_test.cpp b/test/unit/CBot/CBot_test.cpp index ca9ff919..71455e28 100644 --- a/test/unit/CBot/CBot_test.cpp +++ b/test/unit/CBot/CBot_test.cpp @@ -298,6 +298,96 @@ TEST_F(CBotUT, EmptyTest) ); } +TEST_F(CBotUT, FunctionCompileErrors) +{ + ExecuteTest( + "public", + CBotErrNoType + ); + + ExecuteTest( + "extern", + CBotErrNoType + ); + + ExecuteTest( + "public void", + CBotErrNoFunc + ); + + ExecuteTest( + "extern void", + CBotErrNoFunc + ); + + ExecuteTest( + "extern void MissingParameterType(", + CBotErrNoType + ); + + ExecuteTest( + "extern void MissingParamName(int", + CBotErrNoVar + ); + + ExecuteTest( + "extern void MissingCloseParen(int i", + CBotErrClosePar + ); + + ExecuteTest( + "extern void ParamTrailingComma(int i, ) {\n" + "}\n", + CBotErrNoType + ); + + ExecuteTest( + "extern void MissingOpenBlock(int i)", + CBotErrOpenBlock + ); + + ExecuteTest( + "extern void MissingCloseBlock()\n" + "{\n", + CBotErrCloseBlock + ); + +} + +TEST_F(CBotUT, ClassCompileErrors) +{ + ExecuteTest( + "public class", + CBotErrNoClassName + ); + + ExecuteTest( + "public class 1234", + CBotErrNoClassName + ); + + ExecuteTest( + "public class TestClass", + CBotErrOpenBlock + ); + + ExecuteTest( + "public class TestClass\n" + "{\n", + CBotErrCloseBlock + ); + + ExecuteTest( + "public class TestClass extends", + CBotErrNoClassName + ); + + ExecuteTest( + "public class TestClass extends 1234", + CBotErrNoClassName + ); +} + TEST_F(CBotUT, DivideByZero) { ExecuteTest( From baba6081b34f1c5a1fcf80ce75ec8e419e45973e Mon Sep 17 00:00:00 2001 From: melex750 Date: Tue, 24 Jan 2017 14:41:22 -0500 Subject: [PATCH 2/3] Add checking for return statements in functions issue #30 --- po/colobot.pot | 3 ++ po/de.po | 3 ++ po/fr.po | 3 ++ po/pl.po | 3 ++ po/ru.po | 3 ++ src/CBot/CBotEnums.h | 1 + src/CBot/CBotInstr/CBotFunction.cpp | 12 +++++++ src/CBot/CBotInstr/CBotFunction.h | 6 ++++ src/CBot/CBotInstr/CBotIf.cpp | 9 ++++++ src/CBot/CBotInstr/CBotIf.h | 8 +++++ src/CBot/CBotInstr/CBotInstr.cpp | 7 +++++ src/CBot/CBotInstr/CBotInstr.h | 6 ++++ src/CBot/CBotInstr/CBotListInstr.cpp | 6 ++++ src/CBot/CBotInstr/CBotListInstr.h | 7 +++++ src/CBot/CBotInstr/CBotReturn.cpp | 5 +++ src/CBot/CBotInstr/CBotReturn.h | 6 ++++ src/common/restext.cpp | 1 + test/unit/CBot/CBot_test.cpp | 47 ++++++++++++++++++++++++++-- 18 files changed, 133 insertions(+), 3 deletions(-) diff --git a/po/colobot.pot b/po/colobot.pot index 04eda131..72b447ac 100644 --- a/po/colobot.pot +++ b/po/colobot.pot @@ -1742,6 +1742,9 @@ msgstr "" msgid "Class name expected" msgstr "" +msgid "Non-void function needs \"return;\"" +msgstr "" + msgid "Dividing by zero" msgstr "" diff --git a/po/de.po b/po/de.po index 440febf5..cdcc3f39 100644 --- a/po/de.po +++ b/po/de.po @@ -942,6 +942,9 @@ msgstr "Kein konvertierbares Platin" msgid "No userlevels installed!" msgstr "" +msgid "Non-void function needs \"return;\"" +msgstr "" + msgid "Normal size" msgstr "Normale Größe" diff --git a/po/fr.po b/po/fr.po index 98c75496..71b45fb2 100644 --- a/po/fr.po +++ b/po/fr.po @@ -924,6 +924,9 @@ msgstr "Pas d'uranium à transformer" msgid "No userlevels installed!" msgstr "Pas de niveaux spéciaux installés !" +msgid "Non-void function needs \"return;\"" +msgstr "" + msgid "Normal size" msgstr "Taille normale" diff --git a/po/pl.po b/po/pl.po index f298098a..19b0f2b7 100644 --- a/po/pl.po +++ b/po/pl.po @@ -926,6 +926,9 @@ msgstr "Brak uranu do przetworzenia" msgid "No userlevels installed!" msgstr "Brak zainstalowanych poziomów użytkownika!" +msgid "Non-void function needs \"return;\"" +msgstr "" + msgid "Normal size" msgstr "Normalna wielkość" diff --git a/po/ru.po b/po/ru.po index 4042a185..c8ed6048 100644 --- a/po/ru.po +++ b/po/ru.po @@ -935,6 +935,9 @@ msgstr "Нет урана для преобразования" msgid "No userlevels installed!" msgstr "Не установленны пользовательские уровни!" +msgid "Non-void function needs \"return;\"" +msgstr "" + msgid "Normal size" msgstr "Нормальный размер" diff --git a/src/CBot/CBotEnums.h b/src/CBot/CBotEnums.h index a34c9e7c..22022c77 100644 --- a/src/CBot/CBotEnums.h +++ b/src/CBot/CBotEnums.h @@ -239,6 +239,7 @@ enum CBotError : int CBotErrAmbiguousCall = 5044, //!< ambiguous call to overloaded function CBotErrFuncNotVoid = 5045, //!< function needs return type "void" CBotErrNoClassName = 5046, //!< class name expected + CBotErrNoReturn = 5047, //!< non-void function needs "return;" // Runtime errors CBotErrZeroDiv = 6000, //!< division by zero diff --git a/src/CBot/CBotInstr/CBotFunction.cpp b/src/CBot/CBotInstr/CBotFunction.cpp index f08ddd9e..78d55fb5 100644 --- a/src/CBot/CBotInstr/CBotFunction.cpp +++ b/src/CBot/CBotInstr/CBotFunction.cpp @@ -228,6 +228,12 @@ CBotFunction* CBotFunction::Compile(CBotToken* &p, CBotCStack* pStack, CBotFunct func->m_closeblk = (p != nullptr && p->GetPrev() != nullptr) ? *(p->GetPrev()) : CBotToken(); if ( pStk->IsOk() ) { + if (!func->m_retTyp.Eq(CBotTypVoid) && !func->HasReturn()) + { + int errPos = func->m_closeblk.GetStart(); + pStk->ResetError(CBotErrNoReturn, errPos, errPos); + goto bad; + } return pStack->ReturnFunc(func, pStk); } } @@ -938,6 +944,12 @@ void CBotFunction::AddPublic(CBotFunction* func) m_publicFunctions.insert(func); } +bool CBotFunction::HasReturn() +{ + if (m_block != nullptr) return m_block->HasReturn(); + return false; +} + std::string CBotFunction::GetDebugData() { std::stringstream ss; diff --git a/src/CBot/CBotInstr/CBotFunction.h b/src/CBot/CBotInstr/CBotFunction.h index bab5e2bb..a23e2fd5 100644 --- a/src/CBot/CBotInstr/CBotFunction.h +++ b/src/CBot/CBotInstr/CBotFunction.h @@ -235,6 +235,12 @@ public: CBotGet modestart, CBotGet modestop); + /*! + * \brief Check if the function has a return statment that will execute. + * \return true if a return statment was found. + */ + bool HasReturn() override; + protected: virtual const std::string GetDebugName() override { return "CBotFunction"; } virtual std::string GetDebugData() override; diff --git a/src/CBot/CBotInstr/CBotIf.cpp b/src/CBot/CBotInstr/CBotIf.cpp index c0a1d1ee..92da9b48 100644 --- a/src/CBot/CBotInstr/CBotIf.cpp +++ b/src/CBot/CBotInstr/CBotIf.cpp @@ -163,6 +163,15 @@ void CBotIf :: RestoreState(CBotStack* &pj, bool bMain) } } +bool CBotIf::HasReturn() +{ + if (m_block != nullptr && m_blockElse != nullptr) + { + if (m_block->HasReturn() && m_blockElse->HasReturn()) return true; + } + return CBotInstr::HasReturn(); // check next block or instruction +} + std::map CBotIf::GetDebugLinks() { auto links = CBotInstr::GetDebugLinks(); diff --git a/src/CBot/CBotInstr/CBotIf.h b/src/CBot/CBotInstr/CBotIf.h index 94b2d257..3df400e1 100644 --- a/src/CBot/CBotInstr/CBotIf.h +++ b/src/CBot/CBotInstr/CBotIf.h @@ -56,6 +56,14 @@ public: */ void RestoreState(CBotStack* &pj, bool bMain) override; + /** + * \brief Check 'if' and 'else' for return statements. + * Returns true when 'if' and 'else' have return statements, + * if not, the next block or instruction is checked. + * \return true if a return statement is found. + */ + bool HasReturn() override; + protected: virtual const std::string GetDebugName() override { return "CBotIf"; } virtual std::map GetDebugLinks() override; diff --git a/src/CBot/CBotInstr/CBotInstr.cpp b/src/CBot/CBotInstr/CBotInstr.cpp index 2ae04233..1d30f673 100644 --- a/src/CBot/CBotInstr/CBotInstr.cpp +++ b/src/CBot/CBotInstr/CBotInstr.cpp @@ -359,6 +359,13 @@ CBotInstr* CBotInstr::CompileArray(CBotToken* &p, CBotCStack* pStack, CBotTypRes return nullptr; } +bool CBotInstr::HasReturn() +{ + assert(this != nullptr); + if (m_next != nullptr) return m_next->HasReturn(); + return false; // end of the list +} + std::map CBotInstr::GetDebugLinks() { return { diff --git a/src/CBot/CBotInstr/CBotInstr.h b/src/CBot/CBotInstr/CBotInstr.h index ab991366..a96550db 100644 --- a/src/CBot/CBotInstr/CBotInstr.h +++ b/src/CBot/CBotInstr/CBotInstr.h @@ -281,6 +281,12 @@ public: */ static bool ChkLvl(const std::string& label, int type); + /** + * \brief Check a list of instructions for a return statement. + * \return true if a return statement was found. + */ + virtual bool HasReturn(); + protected: friend class CBotDebug; /** diff --git a/src/CBot/CBotInstr/CBotListInstr.cpp b/src/CBot/CBotInstr/CBotListInstr.cpp index 0ae396f5..bcbd3bba 100644 --- a/src/CBot/CBotInstr/CBotListInstr.cpp +++ b/src/CBot/CBotInstr/CBotListInstr.cpp @@ -117,6 +117,12 @@ void CBotListInstr::RestoreState(CBotStack* &pj, bool bMain) if (p != nullptr) p->RestoreState(pile, true); } +bool CBotListInstr::HasReturn() +{ + if (m_instr != nullptr && m_instr->HasReturn()) return true; + return CBotInstr::HasReturn(); // check next block or instruction +} + std::map CBotListInstr::GetDebugLinks() { auto links = CBotInstr::GetDebugLinks(); diff --git a/src/CBot/CBotInstr/CBotListInstr.h b/src/CBot/CBotInstr/CBotListInstr.h index e0923e04..659784f6 100644 --- a/src/CBot/CBotInstr/CBotListInstr.h +++ b/src/CBot/CBotInstr/CBotListInstr.h @@ -56,6 +56,13 @@ public: */ void RestoreState(CBotStack* &pj, bool bMain) override; + /** + * \brief Check this block of instructions for a return statement. + * If not found, the next block or instruction is checked. + * \return true if a return statement was found. + */ + bool HasReturn() override; + protected: virtual const std::string GetDebugName() override { return "CBotListInstr"; } virtual std::map GetDebugLinks() override; diff --git a/src/CBot/CBotInstr/CBotReturn.cpp b/src/CBot/CBotInstr/CBotReturn.cpp index acbd3d8e..5979989e 100644 --- a/src/CBot/CBotInstr/CBotReturn.cpp +++ b/src/CBot/CBotInstr/CBotReturn.cpp @@ -111,6 +111,11 @@ void CBotReturn::RestoreState(CBotStack* &pj, bool bMain) } } +bool CBotReturn::HasReturn() +{ + return true; +} + std::map CBotReturn::GetDebugLinks() { auto links = CBotInstr::GetDebugLinks(); diff --git a/src/CBot/CBotInstr/CBotReturn.h b/src/CBot/CBotInstr/CBotReturn.h index 237571a3..9d14a3e9 100644 --- a/src/CBot/CBotInstr/CBotReturn.h +++ b/src/CBot/CBotInstr/CBotReturn.h @@ -55,6 +55,12 @@ public: */ void RestoreState(CBotStack* &pj, bool bMain) override; + /*! + * \brief Always returns true. + * \return true to signal a return statment has been found. + */ + bool HasReturn() override; + protected: virtual const std::string GetDebugName() override { return "CBotReturn"; } virtual std::map GetDebugLinks() override; diff --git a/src/common/restext.cpp b/src/common/restext.cpp index 93bab6c7..5d1f012f 100644 --- a/src/common/restext.cpp +++ b/src/common/restext.cpp @@ -721,6 +721,7 @@ void InitializeRestext() stringsCbot[CBot::CBotErrAmbiguousCall] = TR("Ambiguous call to overloaded function"); stringsCbot[CBot::CBotErrFuncNotVoid] = TR("Function needs return type \"void\""); stringsCbot[CBot::CBotErrNoClassName] = TR("Class name expected"); + stringsCbot[CBot::CBotErrNoReturn] = TR("Non-void function needs \"return;\""); stringsCbot[CBot::CBotErrZeroDiv] = TR("Dividing by zero"); stringsCbot[CBot::CBotErrNotInit] = TR("Variable not initialized"); diff --git a/test/unit/CBot/CBot_test.cpp b/test/unit/CBot/CBot_test.cpp index 71455e28..1d02b8a0 100644 --- a/test/unit/CBot/CBot_test.cpp +++ b/test/unit/CBot/CBot_test.cpp @@ -801,8 +801,7 @@ TEST_F(CBotUT, FunctionBadReturn) ); } -// TODO: Doesn't work -TEST_F(CBotUT, DISABLED_FunctionNoReturn) +TEST_F(CBotUT, FunctionNoReturn) { ExecuteTest( "int func()\n" @@ -812,7 +811,49 @@ TEST_F(CBotUT, DISABLED_FunctionNoReturn) "{\n" " func();\n" "}\n", - static_cast(-1) // TODO: no error for that + CBotErrNoReturn + ); + + ExecuteTest( + "int FuncDoesNotReturnAValue()\n" + "{\n" + " if (false) return 1;\n" + " while (false) return 1;\n" + " if (true) ; else return 1;\n" + " do { break; return 1; } while (false);\n" + " do { continue; return 1; } while (false);\n" + "}\n", + CBotErrNoReturn + ); + + ExecuteTest( + "int FuncHasReturn()\n" + "{\n" + " return 1;\n" + "}\n" + "int BlockHasReturn()\n" + "{\n" + " {\n" + " {\n" + " }\n" + " return 2;\n" + " }\n" + "}\n" + "int IfElseHasReturn()\n" + "{\n" + " if (false) {\n" + " return 3;\n" + " } else {\n" + " if (false) return 3;\n" + " else return 3;\n" + " }\n" + "}\n" + "extern void Test()\n" + "{\n" + " ASSERT(1 == FuncHasReturn());\n" + " ASSERT(2 == BlockHasReturn());\n" + " ASSERT(3 == IfElseHasReturn());\n" + "}\n" ); } From 92a8c48953460a9b7aaf05e3bb3153f6451753a7 Mon Sep 17 00:00:00 2001 From: melex750 Date: Tue, 24 Jan 2017 15:19:03 -0500 Subject: [PATCH 3/3] Add syntax for parameters with default values Also fixes #642 --- po/colobot.pot | 3 ++ po/de.po | 3 ++ po/fr.po | 3 ++ po/pl.po | 3 ++ po/ru.po | 3 ++ src/CBot/CBotDefParam.cpp | 74 ++++++++++++++++++++++---- src/CBot/CBotDefParam.h | 9 ++++ src/CBot/CBotEnums.h | 1 + src/CBot/CBotInstr/CBotFunction.cpp | 32 +++++++++-- src/CBot/CBotInstr/CBotNew.cpp | 2 +- src/CBot/CBotInstr/CBotParExpr.cpp | 10 ++++ src/CBot/CBotInstr/CBotParExpr.h | 8 +++ src/common/restext.cpp | 1 + test/unit/CBot/CBot_test.cpp | 82 ++++++++++++++++++++++++++++- 14 files changed, 218 insertions(+), 16 deletions(-) diff --git a/po/colobot.pot b/po/colobot.pot index 72b447ac..30a4c1c8 100644 --- a/po/colobot.pot +++ b/po/colobot.pot @@ -1745,6 +1745,9 @@ msgstr "" msgid "Non-void function needs \"return;\"" msgstr "" +msgid "This parameter needs a default value" +msgstr "" + msgid "Dividing by zero" msgstr "" diff --git a/po/de.po b/po/de.po index cdcc3f39..255608e3 100644 --- a/po/de.po +++ b/po/de.po @@ -1522,6 +1522,9 @@ msgstr "" msgid "This object is not a member of a class" msgstr "Das Objekt ist nicht eine Instanz einer Klasse" +msgid "This parameter needs a default value" +msgstr "" + msgid "This program is read-only, clone it to edit" msgstr "" diff --git a/po/fr.po b/po/fr.po index 71b45fb2..818948a0 100644 --- a/po/fr.po +++ b/po/fr.po @@ -1494,6 +1494,9 @@ msgstr "" msgid "This object is not a member of a class" msgstr "L'objet n'est pas une instance d'une classe" +msgid "This parameter needs a default value" +msgstr "" + msgid "This program is read-only, clone it to edit" msgstr "Ce programme est en lecture-seule, le dupliquer pour pouvoir l'éditer" diff --git a/po/pl.po b/po/pl.po index 19b0f2b7..aaa50b8a 100644 --- a/po/pl.po +++ b/po/pl.po @@ -1496,6 +1496,9 @@ msgstr "Ten objekt jest obecnie zajęty" msgid "This object is not a member of a class" msgstr "Ten obiekt nie jest członkiem klasy" +msgid "This parameter needs a default value" +msgstr "" + msgid "This program is read-only, clone it to edit" msgstr "Ten program jest tylko do odczytu, skopiuj go, aby edytować" diff --git a/po/ru.po b/po/ru.po index c8ed6048..f479b47e 100644 --- a/po/ru.po +++ b/po/ru.po @@ -1510,6 +1510,9 @@ msgstr "" msgid "This object is not a member of a class" msgstr "Этот объект не член класса" +msgid "This parameter needs a default value" +msgstr "" + msgid "This program is read-only, clone it to edit" msgstr "Эта программа только для чтения, для редактирования клонируйте её" diff --git a/src/CBot/CBotDefParam.cpp b/src/CBot/CBotDefParam.cpp index e66c8622..8b54f5a1 100644 --- a/src/CBot/CBotDefParam.cpp +++ b/src/CBot/CBotDefParam.cpp @@ -19,6 +19,9 @@ #include "CBot/CBotDefParam.h" +#include "CBot/CBotInstr/CBotInstrUtils.h" +#include "CBot/CBotInstr/CBotParExpr.h" + #include "CBot/CBotUtils.h" #include "CBot/CBotCStack.h" @@ -33,11 +36,13 @@ namespace CBot CBotDefParam::CBotDefParam() { m_nIdent = 0; + m_expr = nullptr; } //////////////////////////////////////////////////////////////////////////////// CBotDefParam::~CBotDefParam() { + delete m_expr; } //////////////////////////////////////////////////////////////////////////////// @@ -51,6 +56,7 @@ CBotDefParam* CBotDefParam::Compile(CBotToken* &p, CBotCStack* pStack) if (IsOfType(p, ID_OPENPAR)) { CBotDefParam* list = nullptr; + bool prevHasDefault = false; if (!IsOfType(p, ID_CLOSEPAR)) while (true) { @@ -77,6 +83,26 @@ CBotDefParam* CBotDefParam::Compile(CBotToken* &p, CBotCStack* pStack) break; } + if (IsOfType(p, ID_ASS)) // default value assignment + { + CBotCStack* pStk = pStack->TokenStack(nullptr, true); + if (nullptr != (param->m_expr = CBotParExpr::CompileLitExpr(p, pStk))) + { + CBotTypResult valueType = pStk->GetTypResult(CBotVar::GetTypeMode::CLASS_AS_INTRINSIC); + + if (!TypesCompatibles(type, valueType)) + pStack->SetError(CBotErrBadType1, p->GetPrev()); + + prevHasDefault = true; + } + else pStack->SetError(CBotErrNoExpression, p); + delete pStk; + } + else + if (prevHasDefault) pStack->SetError(CBotErrDefaultValue, p->GetPrev()); + + if (!pStack->IsOk()) break; + if ( type.Eq(CBotTypArrayPointer) ) type.SetType(CBotTypArrayBody); CBotVar* var = CBotVar::Create(pp->GetString(), type); // creates the variable // if ( pClass ) var->SetClass(pClass); @@ -109,40 +135,63 @@ bool CBotDefParam::Execute(CBotVar** ppVars, CBotStack* &pj) assert(this != nullptr); CBotDefParam* p = this; + bool useDefault = false; + while ( p != nullptr ) { // creates a local variable on the stack CBotVar* newvar = CBotVar::Create(p->m_token.GetString(), p->m_type); + CBotVar* pVar = nullptr; + CBotStack* pile = nullptr; // stack for default expression + + if (useDefault || (ppVars == nullptr || ppVars[i] == nullptr)) + { + assert(p->m_expr != nullptr); + + pile = pj->AddStack(); + useDefault = true; + + while (pile->IsOk() && !p->m_expr->Execute(pile)); + if (!pile->IsOk()) return pj->Return(pile); // return the error + + pVar = pile->GetVar(); + } + else + pVar = ppVars[i]; + // serves to make the transformation of types: - if ( ppVars != nullptr && ppVars[i] != nullptr ) + if ((useDefault && pVar != nullptr) || + (ppVars != nullptr && pVar != nullptr)) { switch (p->m_type.GetType()) { case CBotTypInt: - newvar->SetValInt(ppVars[i]->GetValInt()); + newvar->SetValInt(pVar->GetValInt()); + newvar->SetInit(pVar->GetInit()); // copy nan break; case CBotTypFloat: - newvar->SetValFloat(ppVars[i]->GetValFloat()); + newvar->SetValFloat(pVar->GetValFloat()); + newvar->SetInit(pVar->GetInit()); // copy nan break; case CBotTypString: - newvar->SetValString(ppVars[i]->GetValString()); + newvar->SetValString(pVar->GetValString()); break; case CBotTypBoolean: - newvar->SetValInt(ppVars[i]->GetValInt()); + newvar->SetValInt(pVar->GetValInt()); break; case CBotTypIntrinsic: - (static_cast(newvar))->Copy(ppVars[i], false); + (static_cast(newvar))->Copy(pVar, false); break; case CBotTypPointer: { - newvar->SetPointer(ppVars[i]->GetPointer()); + newvar->SetPointer(pVar->GetPointer()); newvar->SetType(p->m_type); // keep pointer type } break; case CBotTypArrayPointer: { - newvar->SetPointer(ppVars[i]->GetPointer()); + newvar->SetPointer(pVar->GetPointer()); } break; default: @@ -152,12 +201,19 @@ bool CBotDefParam::Execute(CBotVar** ppVars, CBotStack* &pj) newvar->SetUniqNum(p->m_nIdent); pj->AddVar(newvar); // add a variable p = p->m_next; - i++; + if (!useDefault) i++; + if (pile != nullptr) pile->Delete(); } return true; } +//////////////////////////////////////////////////////////////////////////////// +bool CBotDefParam::HasDefault() +{ + return (m_expr != nullptr); +} + //////////////////////////////////////////////////////////////////////////////// void CBotDefParam::RestoreState(CBotStack* &pj, bool bMain) { diff --git a/src/CBot/CBotDefParam.h b/src/CBot/CBotDefParam.h index c23855db..bc9d9d0a 100644 --- a/src/CBot/CBotDefParam.h +++ b/src/CBot/CBotDefParam.h @@ -63,6 +63,12 @@ public: */ bool Execute(CBotVar** ppVars, CBotStack* &pj); + /*! + * \brief Check if this parameter has a default value expression. + * \return true if the parameter was compiled with a default value. + */ + bool HasDefault(); + /*! * \brief RestoreState * \param pj @@ -96,6 +102,9 @@ private: //! Type of paramteter. CBotTypResult m_type; long m_nIdent; + + //! Default value expression for the parameter. + CBotInstr* m_expr; }; } // namespace CBot diff --git a/src/CBot/CBotEnums.h b/src/CBot/CBotEnums.h index 22022c77..f98d4e96 100644 --- a/src/CBot/CBotEnums.h +++ b/src/CBot/CBotEnums.h @@ -240,6 +240,7 @@ enum CBotError : int CBotErrFuncNotVoid = 5045, //!< function needs return type "void" CBotErrNoClassName = 5046, //!< class name expected CBotErrNoReturn = 5047, //!< non-void function needs "return;" + CBotErrDefaultValue = 5048, //!< this parameter needs a default value // Runtime errors CBotErrZeroDiv = 6000, //!< division by zero diff --git a/src/CBot/CBotInstr/CBotFunction.cpp b/src/CBot/CBotInstr/CBotFunction.cpp index 78d55fb5..0f6a9800 100644 --- a/src/CBot/CBotInstr/CBotFunction.cpp +++ b/src/CBot/CBotInstr/CBotFunction.cpp @@ -503,8 +503,13 @@ CBotFunction* CBotFunction::FindLocalOrPublic(const std::list& lo // parameters are compatible? CBotDefParam* pv = pt->m_param; // expected list of parameters CBotVar* pw = ppVars[i++]; // provided list parameter - while ( pv != nullptr && pw != nullptr) + while ( pv != nullptr && (pw != nullptr || pv->HasDefault()) ) { + if (pw == nullptr) // end of arguments + { + pv = pv->GetNext(); + continue; // skip params with default values + } CBotTypResult paramType = pv->GetTypResult(); CBotTypResult argType = pw->GetTypResult(CBotVar::GetTypeMode::CLASS_AS_INTRINSIC); @@ -561,8 +566,13 @@ CBotFunction* CBotFunction::FindLocalOrPublic(const std::list& lo // parameters sont-ils compatibles ? CBotDefParam* pv = pt->m_param; // list of expected parameters CBotVar* pw = ppVars[i++]; // list of provided parameters - while ( pv != nullptr && pw != nullptr) + while ( pv != nullptr && (pw != nullptr || pv->HasDefault()) ) { + if (pw == nullptr) // end of arguments + { + pv = pv->GetNext(); + continue; // skip params with default values + } CBotTypResult paramType = pv->GetTypResult(); CBotTypResult argType = pw->GetTypResult(CBotVar::GetTypeMode::CLASS_AS_INTRINSIC); @@ -690,7 +700,14 @@ int CBotFunction::DoCall(CBotProgram* program, const std::list& l // initializes the variables as parameters if (pt->m_param != nullptr) { - pt->m_param->Execute(ppVars, pStk3); // cannot be interrupted + if (!pt->m_param->Execute(ppVars, pStk3)) // interupts only if error on a default value + { + if ( pt->m_pProg != program ) + { + pStk3->SetPosError(pToken); // indicates the error on the procedure call + } + return pStack->Return(pStk3); + } } pStk1->IncState(); @@ -812,7 +829,14 @@ int CBotFunction::DoCall(const std::list& localFunctionList, long // initializes the variables as parameters if (pt->m_param != nullptr) { - pt->m_param->Execute(ppVars, pStk3); // cannot be interrupted + if (!pt->m_param->Execute(ppVars, pStk3)) // interupts only if error on a default value + { + if ( pt->m_pProg != pProgCurrent ) + { + pStk3->SetPosError(pToken); // indicates the error on the procedure call + } + return pStack->Return(pStk3); + } } pStk->IncState(); } diff --git a/src/CBot/CBotInstr/CBotNew.cpp b/src/CBot/CBotInstr/CBotNew.cpp index 9fab3824..1c84dc53 100644 --- a/src/CBot/CBotInstr/CBotNew.cpp +++ b/src/CBot/CBotInstr/CBotNew.cpp @@ -200,7 +200,7 @@ bool CBotNew::Execute(CBotStack* &pj) } ppVars[i] = nullptr; - if ( !pClass->ExecuteMethode(m_nMethodeIdent, pThis, ppVars, CBotTypResult(CBotTypVoid), pile2, GetToken())) return false; // interrupt + if ( !pClass->ExecuteMethode(m_nMethodeIdent, pThis, ppVars, CBotTypResult(CBotTypVoid), pile2, &m_vartoken)) return false; // interrupt pThis->ConstructorSet(); // indicates that the constructor has been called } diff --git a/src/CBot/CBotInstr/CBotParExpr.cpp b/src/CBot/CBotInstr/CBotParExpr.cpp index a32363fb..b0c2f1c1 100644 --- a/src/CBot/CBotInstr/CBotParExpr.cpp +++ b/src/CBot/CBotInstr/CBotParExpr.cpp @@ -132,6 +132,16 @@ CBotInstr* CBotParExpr::Compile(CBotToken* &p, CBotCStack* pStack) return pStack->Return(nullptr, pStk); } + return CompileLitExpr(p, pStack); +} + +//////////////////////////////////////////////////////////////////////////////// +CBotInstr* CBotParExpr::CompileLitExpr(CBotToken* &p, CBotCStack* pStack) +{ + CBotCStack* pStk = pStack->TokenStack(); + + CBotToken* pp = p; + // is it a number or DefineNum? if (p->GetType() == TokenTypNum || p->GetType() == TokenTypDef ) diff --git a/src/CBot/CBotInstr/CBotParExpr.h b/src/CBot/CBotInstr/CBotParExpr.h index cd92467b..0bcdd0e1 100644 --- a/src/CBot/CBotInstr/CBotParExpr.h +++ b/src/CBot/CBotInstr/CBotParExpr.h @@ -54,6 +54,14 @@ public: */ static CBotInstr* Compile(CBotToken* &p, CBotCStack* pStack); + /*! + * \brief Compile a literal expression ("string", number, true, false, null, nan, new) + * \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 + * \return The compiled instruction or nullptr on error + */ + static CBotInstr* CompileLitExpr(CBotToken* &p, CBotCStack* pStack); + private: CBotParExpr() = delete; CBotParExpr(const CBotParExpr&) = delete; diff --git a/src/common/restext.cpp b/src/common/restext.cpp index 5d1f012f..035389b4 100644 --- a/src/common/restext.cpp +++ b/src/common/restext.cpp @@ -722,6 +722,7 @@ void InitializeRestext() stringsCbot[CBot::CBotErrFuncNotVoid] = TR("Function needs return type \"void\""); stringsCbot[CBot::CBotErrNoClassName] = TR("Class name expected"); stringsCbot[CBot::CBotErrNoReturn] = TR("Non-void function needs \"return;\""); + stringsCbot[CBot::CBotErrDefaultValue] = TR("This parameter needs a default value"); stringsCbot[CBot::CBotErrZeroDiv] = TR("Dividing by zero"); stringsCbot[CBot::CBotErrNotInit] = TR("Variable not initialized"); diff --git a/test/unit/CBot/CBot_test.cpp b/test/unit/CBot/CBot_test.cpp index 1d02b8a0..2f8bb503 100644 --- a/test/unit/CBot/CBot_test.cpp +++ b/test/unit/CBot/CBot_test.cpp @@ -1594,11 +1594,12 @@ TEST_F(CBotUT, StringFunctions) ); } -TEST_F(CBotUT, DISABLED_TestNANParam_Issue642) +TEST_F(CBotUT, TestNANParam_Issue642) { ExecuteTest( "float test(float x) {\n" - " return x;\n" + " ASSERT(x == nan);\n" + " return x;\n" "}\n" "extern void TestNANParam() {\n" " ASSERT(nan == nan);\n" // TODO: Shouldn't it be nan != nan ?? @@ -2308,3 +2309,80 @@ TEST_F(CBotUT, IncrementDecrementSyntax) CBotErrBadType1 ); } + +TEST_F(CBotUT, ParametersWithDefaultValues) +{ + ExecuteTest( + "extern void ParametersWithDefaultValues() {\n" + " ASSERT(true == Test());\n" + " ASSERT(true == Test(1));\n" + " ASSERT(true == Test(1, 2));\n" + "}\n" + "bool Test(int i = 1, float f = 2.0) {\n" + " return (i == 1) && (f == 2.0);\n" + "}\n" + ); + + ExecuteTest( + "extern void NotUsingDefaultValues() {\n" + " ASSERT(true == Test(2, 4.0));\n" + "}\n" + "bool Test(int i = 1, float f = 2.0) {\n" + " return (i == 2) && (f == 4.0);\n" + "}\n" + ); + + ExecuteTest( + "extern void NextParamNeedsDefaultValue() {\n" + "}\n" + "void Test(int i = 1, float f) {}\n" + "\n", + CBotErrDefaultValue + ); + + ExecuteTest( + "extern void ParamMissingExpression() {\n" + "}\n" + "void Test(int i = 1, float f = ) {}\n" + "\n", + CBotErrNoExpression + ); + + ExecuteTest( + "extern void ParamDefaultBadType() {\n" + "}\n" + "void Test(int i = 1, float f = null) {}\n" + "\n", + CBotErrBadType1 + ); + + ExecuteTest( + "extern void DefaultValuesAmbiguousCall() {\n" + " Test();\n" + "}\n" + "void Test(int i = 1) {}\n" + "void Test(float f = 2.0) {}\n" + "\n", + CBotErrAmbiguousCall + ); + + ExecuteTest( + "extern void AmbiguousCallOneDefault() {\n" + " Test(1);\n" + "}\n" + "void Test(int i, float f = 2) {}\n" + "void Test(int i) {}\n" + "\n", + CBotErrAmbiguousCall + ); + + ExecuteTest( + "extern void DifferentNumberOfDefaultValues() {\n" + " Test(1, 2.0);\n" + "}\n" + "void Test(int i, float f = 2.0) {}\n" + "void Test(int i, float f = 2.0, int ii = 1) {}\n" + "\n", + CBotErrAmbiguousCall + ); +}