[colobot] 45/100: Add checking for return statements in functions
Didier Raboud
odyx at moszumanska.debian.org
Thu Jun 1 18:10:17 UTC 2017
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 <melex750 at users.noreply.github.com>
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
More information about the Pkg-games-commits
mailing list