From 250ea57e8b2dfd081116ae93b07122e582c87363 Mon Sep 17 00:00:00 2001 From: melex750 Date: Thu, 11 Apr 2019 04:15:27 -0400 Subject: [PATCH] Fix arithmetic operations with integers (#993) Also fixed unsigned right shift operator ">>>" --- src/CBot/CBotInstr/CBotInstrUtils.cpp | 7 +++ src/CBot/CBotToken.cpp | 2 +- src/CBot/CBotVar/CBotVar.cpp | 5 ++ src/CBot/CBotVar/CBotVar.h | 1 + src/CBot/CBotVar/CBotVarInt.cpp | 26 +------- src/CBot/CBotVar/CBotVarInt.h | 18 +++--- src/CBot/CBotVar/CBotVarValue.h | 91 ++++++++++++++++++++------- test/unit/CBot/CBot_test.cpp | 14 +++++ 8 files changed, 109 insertions(+), 55 deletions(-) diff --git a/src/CBot/CBotInstr/CBotInstrUtils.cpp b/src/CBot/CBotInstr/CBotInstrUtils.cpp index 04c42211..774a4e37 100644 --- a/src/CBot/CBotInstr/CBotInstrUtils.cpp +++ b/src/CBot/CBotInstr/CBotInstrUtils.cpp @@ -129,6 +129,13 @@ bool TypeCompatible(CBotTypResult& type1, CBotTypResult& type2, int op) return true; } + if (op == ID_ASR || op == ID_SR || op == ID_SL || + op == ID_ASSOR || op == ID_ASSSL || op == ID_ASSSR || + op == ID_ASSAND || op == ID_ASSXOR || op == ID_ASSASR) + { + if (max > CBotTypLong) return false; + } + type1.SetType(max); type2.SetType(max); return true; diff --git a/src/CBot/CBotToken.cpp b/src/CBot/CBotToken.cpp index ea31fad9..b3a5a461 100644 --- a/src/CBot/CBotToken.cpp +++ b/src/CBot/CBotToken.cpp @@ -103,7 +103,7 @@ static const boost::bimap KEYWORDS = makeBimap>>="}, {ID_ASSASR, ">>="}, {ID_SL, "<<"}, - {ID_SR, ">>"}, + {ID_SR, ">>>"}, {ID_ASR, ">>"}, {ID_INC, "++"}, {ID_DEC, "--"}, diff --git a/src/CBot/CBotVar/CBotVar.cpp b/src/CBot/CBotVar/CBotVar.cpp index 4886c033..d9947e5b 100644 --- a/src/CBot/CBotVar/CBotVar.cpp +++ b/src/CBot/CBotVar/CBotVar.cpp @@ -752,6 +752,11 @@ CBotClass* CBotVar::GetClass() return nullptr; } +CBotVar::operator bool() +{ + return static_cast(GetValInt()); +} + CBotVar::operator int() { return GetValInt(); diff --git a/src/CBot/CBotVar/CBotVar.h b/src/CBot/CBotVar/CBotVar.h index 980b8a71..1a9c1498 100644 --- a/src/CBot/CBotVar/CBotVar.h +++ b/src/CBot/CBotVar/CBotVar.h @@ -444,6 +444,7 @@ public: */ //@{ + operator bool(); operator int(); operator float(); operator std::string(); diff --git a/src/CBot/CBotVar/CBotVarInt.cpp b/src/CBot/CBotVar/CBotVarInt.cpp index 852566f9..cd602614 100644 --- a/src/CBot/CBotVar/CBotVarInt.cpp +++ b/src/CBot/CBotVar/CBotVarInt.cpp @@ -58,33 +58,9 @@ void CBotVarInt::Dec() m_defnum.clear(); } -void CBotVarInt::XOr(CBotVar* left, CBotVar* right) -{ - SetValInt(left->GetValInt() ^ right->GetValInt()); -} -void CBotVarInt::And(CBotVar* left, CBotVar* right) -{ - SetValInt(left->GetValInt() & right->GetValInt()); -} -void CBotVarInt::Or(CBotVar* left, CBotVar* right) -{ - SetValInt(left->GetValInt() | right->GetValInt()); -} - -void CBotVarInt::SL(CBotVar* left, CBotVar* right) -{ - SetValInt(left->GetValInt() << right->GetValInt()); -} -void CBotVarInt::ASR(CBotVar* left, CBotVar* right) -{ - SetValInt(left->GetValInt() >> right->GetValInt()); -} void CBotVarInt::SR(CBotVar* left, CBotVar* right) { - int source = left->GetValInt(); - int shift = right->GetValInt(); - if (shift >= 1) source &= 0x7fffffff; - SetValInt(source >> shift); + SetValInt(static_cast(left->GetValInt()) >> right->GetValInt()); } void CBotVarInt::Not() diff --git a/src/CBot/CBotVar/CBotVarInt.h b/src/CBot/CBotVar/CBotVarInt.h index 5e04135c..6fb51208 100644 --- a/src/CBot/CBotVar/CBotVarInt.h +++ b/src/CBot/CBotVar/CBotVarInt.h @@ -27,10 +27,10 @@ namespace CBot /** * \brief CBotVar subclass for managing integer values (::CBotTypInt) */ -class CBotVarInt : public CBotVarNumber +class CBotVarInt : public CBotVarInteger { public: - CBotVarInt(const CBotToken &name) : CBotVarNumber(name) {} + CBotVarInt(const CBotToken &name) : CBotVarInteger(name) {} void SetValInt(int val, const std::string& s = "") override; std::string GetValString() override; @@ -40,19 +40,21 @@ public: void Neg() override; void Inc() override; void Dec() override; - - void XOr(CBotVar* left, CBotVar* right) override; - void Or(CBotVar* left, CBotVar* right) override; - void And(CBotVar* left, CBotVar* right) override; void Not() override; - void SL(CBotVar* left, CBotVar* right) override; void SR(CBotVar* left, CBotVar* right) override; - void ASR(CBotVar* left, CBotVar* right) override; bool Save0State(std::ostream &ostr) override; bool Save1State(std::ostream &ostr) override; +protected: + + void SetValue(int val) override + { + CBotVarNumberBase::SetValue(val); + m_defnum.clear(); + } + protected: //! The name if given by DefineNum. std::string m_defnum; diff --git a/src/CBot/CBotVar/CBotVarValue.h b/src/CBot/CBotVar/CBotVarValue.h index 5e58075d..9c0b0361 100644 --- a/src/CBot/CBotVar/CBotVarValue.h +++ b/src/CBot/CBotVar/CBotVarValue.h @@ -74,6 +74,13 @@ public: return s.str(); } +protected: + virtual void SetValue(T val) + { + this->m_val = val; + this->m_binit = CBotVar::InitType::DEF; + } + protected: //! The value T m_val; @@ -93,14 +100,12 @@ public: void SetValInt(int val, const std::string &s = "") override { - this->m_val = static_cast(val); - this->m_binit = CBotVar::InitType::DEF; + this->SetValue(static_cast(val)); } void SetValFloat(float val) override { - this->m_val = static_cast(val); - this->m_binit = CBotVar::InitType::DEF; + this->SetValue(static_cast(val)); } int GetValInt() override @@ -116,11 +121,11 @@ public: bool Eq(CBotVar* left, CBotVar* right) override { - return left->GetValFloat() == right->GetValFloat(); + return static_cast(*left) == static_cast(*right); } bool Ne(CBotVar* left, CBotVar* right) override { - return left->GetValFloat() != right->GetValFloat(); + return static_cast(*left) != static_cast(*right); } }; @@ -135,33 +140,33 @@ public: void Mul(CBotVar* left, CBotVar* right) override { - this->SetValFloat(left->GetValFloat() * right->GetValFloat()); + this->SetValue(static_cast(*left) * static_cast(*right)); } void Power(CBotVar* left, CBotVar* right) override { - this->SetValFloat(pow(left->GetValFloat(), right->GetValFloat())); + this->SetValue(pow(static_cast(*left), static_cast(*right))); } CBotError Div(CBotVar* left, CBotVar* right) override { - float r = right->GetValFloat(); - if (r == 0) return CBotErrZeroDiv; - this->SetValFloat(left->GetValFloat() / r); + T r = static_cast(*right); + if ( r == static_cast(0) ) return CBotErrZeroDiv; + this->SetValue(static_cast(*left) / r); return CBotNoErr; } CBotError Modulo(CBotVar* left, CBotVar* right) override { - float r = right->GetValFloat(); - if (r == 0) return CBotErrZeroDiv; - this->SetValFloat(fmod(left->GetValFloat(), r)); + T r = static_cast(*right); + if ( r == static_cast(0) ) return CBotErrZeroDiv; + this->SetValue(fmod(static_cast(*left), r)); return CBotNoErr; } void Add(CBotVar* left, CBotVar* right) override { - this->SetValFloat(left->GetValFloat() + right->GetValFloat()); + this->SetValue(static_cast(*left) + static_cast(*right)); } void Sub(CBotVar* left, CBotVar* right) override { - this->SetValFloat(left->GetValFloat() - right->GetValFloat()); + this->SetValue(static_cast(*left) - static_cast(*right)); } void Neg() override @@ -179,21 +184,65 @@ public: bool Lo(CBotVar* left, CBotVar* right) override { - return left->GetValFloat() < right->GetValFloat(); + return static_cast(*left) < static_cast(*right); } bool Hi(CBotVar* left, CBotVar* right) override { - return left->GetValFloat() > right->GetValFloat(); + return static_cast(*left) > static_cast(*right); } bool Ls(CBotVar* left, CBotVar* right) override { - return left->GetValFloat() <= right->GetValFloat(); + return static_cast(*left) <= static_cast(*right); } bool Hs(CBotVar* left, CBotVar* right) override { - return left->GetValFloat() >= right->GetValFloat(); + return static_cast(*left) >= static_cast(*right); } }; -} +/** + * \brief An integer variable (byte, short, char, int, long) + */ +template +class CBotVarInteger : public CBotVarNumber +{ +public: + CBotVarInteger(const CBotToken &name) : CBotVarNumber(name) {} + CBotError Modulo(CBotVar* left, CBotVar* right) override + { + T r = static_cast(*right); + if ( r == static_cast(0) ) return CBotErrZeroDiv; + this->SetValue(static_cast(*left) % r); + return CBotNoErr; + } + + void XOr(CBotVar* left, CBotVar* right) override + { + this->SetValue(static_cast(*left) ^ static_cast(*right)); + } + void And(CBotVar* left, CBotVar* right) override + { + this->SetValue(static_cast(*left) & static_cast(*right)); + } + void Or(CBotVar* left, CBotVar* right) override + { + this->SetValue(static_cast(*left) | static_cast(*right)); + } + + void SL(CBotVar* left, CBotVar* right) override + { + this->SetValue(static_cast(*left) << right->GetValInt()); + } + void ASR(CBotVar* left, CBotVar* right) override + { + this->SetValue(static_cast(*left) >> right->GetValInt()); + } + + void Not() override + { + this->m_val = ~(this->m_val); + } +}; + +} // namespace CBot diff --git a/test/unit/CBot/CBot_test.cpp b/test/unit/CBot/CBot_test.cpp index f16637bd..6f2201b9 100644 --- a/test/unit/CBot/CBot_test.cpp +++ b/test/unit/CBot/CBot_test.cpp @@ -605,6 +605,20 @@ TEST_F(CBotUT, VarImplicitCast) ); } +TEST_F(CBotUT, IntegerMathNearLimits_Issue993) +{ + ExecuteTest( + "extern void Test_Issue993() {\n" + " ASSERT(-2147483600 * 1 == -2147483600);\n" + " ASSERT( 2147483600 * 1 == 2147483600);\n" + " ASSERT( 2147483646 * 1 == 2147483646);\n" + " ASSERT( 2147483646 * -1 == -2147483646);\n" + " ASSERT( 2147483000 * -1 == -2147483000);\n" + " ASSERT( 2147483000 * 1 == 2147483000);\n" + "}\n" + ); +} + TEST_F(CBotUT, ToString) { ExecuteTest(