This is an automated email from the git hooks/post-receive script. odyx pushed a commit to branch debian/master in repository colobot.
commit baba6081b34f1c5a1fcf80ce75ec8e419e45973e Author: melex750 <[email protected]> Date: Tue Jan 24 14:41:22 2017 -0500 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 04eda13..72b447a 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 440febf..cdcc3f3 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 98c7549..71b45fb 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 f298098..19b0f2b 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 4042a18..c8ed604 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 a34c9e7..22022c7 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 f08ddd9..78d55fb 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 bab5e2b..a23e2fd 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 c0a1d1e..92da9b4 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<std::string, CBotInstr*> CBotIf::GetDebugLinks() { auto links = CBotInstr::GetDebugLinks(); diff --git a/src/CBot/CBotInstr/CBotIf.h b/src/CBot/CBotInstr/CBotIf.h index 94b2d25..3df400e 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<std::string, CBotInstr*> GetDebugLinks() override; diff --git a/src/CBot/CBotInstr/CBotInstr.cpp b/src/CBot/CBotInstr/CBotInstr.cpp index 2ae0423..1d30f67 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<std::string, CBotInstr*> CBotInstr::GetDebugLinks() { return { diff --git a/src/CBot/CBotInstr/CBotInstr.h b/src/CBot/CBotInstr/CBotInstr.h index ab99136..a96550d 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 0ae396f..bcbd3bb 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<std::string, CBotInstr*> CBotListInstr::GetDebugLinks() { auto links = CBotInstr::GetDebugLinks(); diff --git a/src/CBot/CBotInstr/CBotListInstr.h b/src/CBot/CBotInstr/CBotListInstr.h index e0923e0..659784f 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<std::string, CBotInstr*> GetDebugLinks() override; diff --git a/src/CBot/CBotInstr/CBotReturn.cpp b/src/CBot/CBotInstr/CBotReturn.cpp index acbd3d8..5979989 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<std::string, CBotInstr*> CBotReturn::GetDebugLinks() { auto links = CBotInstr::GetDebugLinks(); diff --git a/src/CBot/CBotInstr/CBotReturn.h b/src/CBot/CBotInstr/CBotReturn.h index 237571a..9d14a3e 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<std::string, CBotInstr*> GetDebugLinks() override; diff --git a/src/common/restext.cpp b/src/common/restext.cpp index 93bab6c..5d1f012 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 71455e2..1d02b8a 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<CBotError>(-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" ); } -- Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-games/colobot.git _______________________________________________ Pkg-games-commits mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-games-commits
