Skip to content

Commit 649a89d

Browse files
amai2012amai2012
committed
Refactoring: Expose some previously local functions to public and add Tokenizer as argument to distinguish between C and C++ code (e.g. in isSameExpression).
Refactoring: Improve type-safety for TestFixture::assertEquals to allow tests with types which were not handled correctly (e.g. unsigned long long)
1 parent e75662a commit 649a89d

File tree

8 files changed

+79
-59
lines changed

8 files changed

+79
-59
lines changed

‎lib/checkcondition.cpp‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -279,13 +279,13 @@ void CheckCondition::comparisonError(const Token *tok, const std::string &bitop,
279279
reportError(tok, Severity::style, "comparisonError", errmsg);
280280
}
281281

282-
static bool isOverlappingCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
282+
bool CheckCondition::isOverlappingCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions) const
283283
{
284284
if (!cond1 || !cond2)
285285
return false;
286286

287287
// same expressions
288-
if (isSameExpression(cond1,cond2,constFunctions))
288+
if (isSameExpression(_tokenizer, cond1,cond2,constFunctions))
289289
return true;
290290

291291
// bitwise overlap for example 'x&7' and 'x==1'
@@ -308,7 +308,7 @@ static bool isOverlappingCond(const Token * const cond1, const Token * const con
308308
if (!num2->isNumber() || MathLib::isNegative(num2->str()))
309309
return false;
310310

311-
if (!isSameExpression(expr1,expr2,constFunctions))
311+
if (!isSameExpression(_tokenizer, expr1,expr2,constFunctions))
312312
return false;
313313

314314
const MathLib::bigint value1 = MathLib::toLongNumber(num1->str());
@@ -361,22 +361,16 @@ void CheckCondition::multiConditionError(const Token *tok, unsigned int line1)
361361
// Detect oppositing inner and outer conditions
362362
//---------------------------------------------------------------------------
363363

364-
/**
365-
* Are two conditions opposite
366-
* @param isNot do you want to know if cond1 is !cond2 or if cond1 and cond2 are non-overlapping. true: cond1==!cond2 false: cond1==true => cond2==false
367-
* @param cond1 condition1
368-
* @param cond2 condition2
369-
*/
370-
static bool isOppositeCond(bool isNot, const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
364+
bool CheckCondition::isOppositeCond(bool isNot, const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions) const
371365
{
372366
if (!cond1 || !cond2)
373367
return false;
374368

375369
if (cond1->str() == "!")
376-
return isSameExpression(cond1->astOperand1(), cond2, constFunctions);
370+
return isSameExpression(_tokenizer, cond1->astOperand1(), cond2, constFunctions);
377371

378372
if (cond2->str() == "!")
379-
return isSameExpression(cond1, cond2->astOperand1(), constFunctions);
373+
return isSameExpression(_tokenizer, cond1, cond2->astOperand1(), constFunctions);
380374

381375
if (!cond1->isComparisonOp() || !cond2->isComparisonOp())
382376
return false;
@@ -385,11 +379,11 @@ static bool isOppositeCond(bool isNot, const Token * const cond1, const Token *
385379

386380
// condition found .. get comparator
387381
std::string comp2;
388-
if (isSameExpression(cond1->astOperand1(), cond2->astOperand1(), constFunctions) &&
389-
isSameExpression(cond1->astOperand2(), cond2->astOperand2(), constFunctions)) {
382+
if (isSameExpression(_tokenizer, cond1->astOperand1(), cond2->astOperand1(), constFunctions) &&
383+
isSameExpression(_tokenizer, cond1->astOperand2(), cond2->astOperand2(), constFunctions)) {
390384
comp2 = cond2->str();
391-
} else if (isSameExpression(cond1->astOperand1(), cond2->astOperand2(), constFunctions) &&
392-
isSameExpression(cond1->astOperand2(), cond2->astOperand1(), constFunctions)) {
385+
} else if (isSameExpression(_tokenizer, cond1->astOperand1(), cond2->astOperand2(), constFunctions) &&
386+
isSameExpression(_tokenizer, cond1->astOperand2(), cond2->astOperand1(), constFunctions)) {
393387
comp2 = cond2->str();
394388
if (comp2[0] == '>')
395389
comp2[0] = '<';
@@ -693,9 +687,9 @@ void CheckCondition::checkIncorrectLogicOperator()
693687
if (!MathLib::isInt(value2) && !MathLib::isFloat(value2))
694688
continue;
695689

696-
if (isSameExpression(comp1, comp2, _settings->library.functionpure))
690+
if (isSameExpression(_tokenizer, comp1, comp2, _settings->library.functionpure))
697691
continue; // same expressions => only report that there are same expressions
698-
if (!isSameExpression(expr1, expr2, _settings->library.functionpure))
692+
if (!isSameExpression(_tokenizer, expr1, expr2, _settings->library.functionpure))
699693
continue;
700694

701695
const bool isfloat = astIsFloat(expr1, true) || MathLib::isFloat(value1) || astIsFloat(expr2, true) || MathLib::isFloat(value2);

‎lib/checkcondition.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ class CPPCHECKLIB CheckCondition : public Check {
9595

9696
private:
9797

98+
bool isOverlappingCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions) const;
99+
/**
100+
* Are two conditions opposite
101+
* @param isNot do you want to know if cond1 is !cond2 or if cond1 and cond2 are non-overlapping. true: cond1==!cond2 false: cond1==true => cond2==false
102+
* @param cond1 condition1
103+
* @param cond2 condition2
104+
*/
105+
bool isOppositeCond(bool isNot, const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions) const;
98106
void assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result);
99107
void mismatchingBitAndError(const Token *tok1, const MathLib::bigint num1, const Token *tok2, const MathLib::bigint num2);
100108
void badBitmaskCheckError(const Token *tok);

‎lib/checkother.cpp‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ bool astIsFloat(const Token *tok, bool unknown)
5959
return unknown;
6060
}
6161

62-
static bool isConstExpression(const Token *tok, const std::set<std::string> &constFunctions)
62+
bool isConstExpression(const Token *tok, const std::set<std::string> &constFunctions)
6363
{
6464
if (!tok)
6565
return true;
@@ -77,21 +77,23 @@ static bool isConstExpression(const Token *tok, const std::set<std::string> &con
7777
return isConstExpression(tok->astOperand1(),constFunctions) && isConstExpression(tok->astOperand2(),constFunctions);
7878
}
7979

80-
bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions)
80+
bool isSameExpression(const Tokenizer *tokenizer, const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions)
8181
{
8282
if (tok1 == nullptr && tok2 == nullptr)
8383
return true;
8484
if (tok1 == nullptr || tok2 == nullptr)
8585
return false;
86-
if (tok1->str() == "." && tok1->astOperand1() && tok1->astOperand1()->str() == "this")
87-
tok1 = tok1->astOperand2();
88-
if (tok2->str() == "." && tok2->astOperand1() && tok2->astOperand1()->str() == "this")
89-
tok2 = tok2->astOperand2();
86+
if (tokenizer->isCPP()) {
87+
if (tok1->str() == "." && tok1->astOperand1() && tok1->astOperand1()->str() == "this")
88+
tok1 = tok1->astOperand2();
89+
if (tok2->str() == "." && tok2->astOperand1() && tok2->astOperand1()->str() == "this")
90+
tok2 = tok2->astOperand2();
91+
}
9092
if (tok1->varId() != tok2->varId() || tok1->str() != tok2->str()) {
9193
if ((Token::Match(tok1,"<|>") && Token::Match(tok2,"<|>")) ||
9294
(Token::Match(tok1,"<=|>=") && Token::Match(tok2,"<=|>="))) {
93-
return isSameExpression(tok1->astOperand1(), tok2->astOperand2(), constFunctions) &&
94-
isSameExpression(tok1->astOperand2(), tok2->astOperand1(), constFunctions);
95+
return isSameExpression(tokenizer, tok1->astOperand1(), tok2->astOperand2(), constFunctions) &&
96+
isSameExpression(tokenizer, tok1->astOperand2(), tok2->astOperand1(), constFunctions);
9597
}
9698
return false;
9799
}
@@ -145,18 +147,18 @@ bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::
145147
return false;
146148
}
147149
bool noncommuative_equals =
148-
isSameExpression(tok1->astOperand1(), tok2->astOperand1(), constFunctions);
150+
isSameExpression(tokenizer, tok1->astOperand1(), tok2->astOperand1(), constFunctions);
149151
noncommuative_equals = noncommuative_equals &&
150-
isSameExpression(tok1->astOperand2(), tok2->astOperand2(), constFunctions);
152+
isSameExpression(tokenizer, tok1->astOperand2(), tok2->astOperand2(), constFunctions);
151153

152154
if (noncommuative_equals)
153155
return true;
154156

155157
const bool commutative = tok1->astOperand1() && tok1->astOperand2() && Token::Match(tok1, "%or%|%oror%|+|*|&|&&|^|==|!=");
156158
bool commuative_equals = commutative &&
157-
isSameExpression(tok1->astOperand2(), tok2->astOperand1(), constFunctions);
159+
isSameExpression(tokenizer, tok1->astOperand2(), tok2->astOperand1(), constFunctions);
158160
commuative_equals = commuative_equals &&
159-
isSameExpression(tok1->astOperand1(), tok2->astOperand2(), constFunctions);
161+
isSameExpression(tokenizer, tok1->astOperand1(), tok2->astOperand2(), constFunctions);
160162

161163
return commuative_equals;
162164
}
@@ -195,13 +197,13 @@ void CheckOther::checkCastIntToCharAndBack()
195197
if (var && var->typeEndToken()->str() == "char" && !var->typeEndToken()->isSigned()) {
196198
checkCastIntToCharAndBackError(tok, tok->strAt(2));
197199
}
198-
} else if (Token::Match(tok, "EOF %comp% ( %var% = std :: cin . get (") || Token::Match(tok, "EOF %comp% ( %var% = cin . get (")) {
200+
} else if (_tokenizer->isCPP() && Token::Match(tok, "EOF %comp% ( %var% = std :: cin . get (") || Token::Match(tok, "EOF %comp% ( %var% = cin . get (")) {
199201
tok = tok->tokAt(3);
200202
const Variable *var = tok->variable();
201203
if (var && var->typeEndToken()->str() == "char" && !var->typeEndToken()->isSigned()) {
202204
checkCastIntToCharAndBackError(tok, "cin.get");
203205
}
204-
} else if (Token::Match(tok, "%var% = std :: cin . get (") || Token::Match(tok, "%var% = cin . get (")) {
206+
} else if (_tokenizer->isCPP() && Token::Match(tok, "%var% = std :: cin . get (") || Token::Match(tok, "%var% = cin . get (")) {
205207
const Variable *var = tok->variable();
206208
if (var && var->typeEndToken()->str() == "char" && !var->typeEndToken()->isSigned()) {
207209
vars[tok->varId()] = "cin.get";
@@ -2181,7 +2183,7 @@ namespace {
21812183
}
21822184
}
21832185

2184-
static bool isWithoutSideEffects(const Tokenizer *tokenizer, const Token* tok)
2186+
bool isWithoutSideEffects(const Tokenizer *tokenizer, const Token* tok)
21852187
{
21862188
if (!tokenizer->isCPP())
21872189
return true;
@@ -2217,7 +2219,7 @@ void CheckOther::checkDuplicateExpression()
22172219
if (tok->isOp() && tok->astOperand1() && !Token::Match(tok, "+|*|<<|>>")) {
22182220
if (Token::Match(tok, "==|!=|-") && astIsFloat(tok->astOperand1(), true))
22192221
continue;
2220-
if (isSameExpression(tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
2222+
if (isSameExpression(_tokenizer, tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
22212223
if (isWithoutSideEffects(_tokenizer, tok->astOperand1())) {
22222224
const bool assignment = tok->str() == "=";
22232225
if (assignment)
@@ -2236,16 +2238,16 @@ void CheckOther::checkDuplicateExpression()
22362238
}
22372239
}
22382240
} else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative
2239-
if (tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(tok->astOperand2(), tok->astOperand1()->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer, tok->astOperand2()))
2241+
if (tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(_tokenizer, tok->astOperand2(), tok->astOperand1()->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer, tok->astOperand2()))
22402242
duplicateExpressionError(tok->astOperand2(), tok->astOperand2(), tok->str());
22412243
else if (tok->astOperand2()) {
22422244
const Token *ast1 = tok->astOperand1();
22432245
while (ast1 && tok->str() == ast1->str()) {
2244-
if (isSameExpression(ast1->astOperand1(), tok->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer, ast1->astOperand1()))
2246+
if (isSameExpression(_tokenizer, ast1->astOperand1(), tok->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer, ast1->astOperand1()))
22452247
// TODO: warn if variables are unchanged. See #5683
22462248
// Probably the message should be changed to 'duplicate expressions X in condition or something like that'.
22472249
;//duplicateExpressionError(ast1->astOperand1(), tok->astOperand2(), tok->str());
2248-
else if (isSameExpression(ast1->astOperand2(), tok->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer, ast1->astOperand2()))
2250+
else if (isSameExpression(_tokenizer, ast1->astOperand2(), tok->astOperand2(), _settings->library.functionpure) && isWithoutSideEffects(_tokenizer, ast1->astOperand2()))
22492251
duplicateExpressionError(ast1->astOperand2(), tok->astOperand2(), tok->str());
22502252
if (!isConstExpression(ast1->astOperand2(), _settings->library.functionpure))
22512253
break;
@@ -2254,7 +2256,7 @@ void CheckOther::checkDuplicateExpression()
22542256
}
22552257
}
22562258
} else if (tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") {
2257-
if (isSameExpression(tok->astOperand1(), tok->astOperand2(), temp))
2259+
if (isSameExpression(_tokenizer, tok->astOperand1(), tok->astOperand2(), temp))
22582260
duplicateExpressionTernaryError(tok);
22592261
}
22602262
}

‎lib/checkother.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,12 @@
2828
class Function;
2929
class Variable;
3030

31+
bool isConstExpression(const Token *tok, const std::set<std::string> &constFunctions);
32+
3133
/** Is expressions same? */
32-
bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions);
34+
bool isSameExpression(const Tokenizer *tokenizer, const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions);
35+
36+
bool isWithoutSideEffects(const Tokenizer *tokenizer, const Token* tok);
3337

3438
/** Is expression of floating point type? */
3539
bool astIsFloat(const Token *tok, bool unknown);

‎test/testlibrary.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ class TestLibrary : public TestFixture {
366366

367367
const struct Library::PodType *type = library.podtype("s16");
368368
ASSERT_EQUALS(2U, type ? type->size : 0U);
369-
ASSERT_EQUALS(0, type ? type->sign : '?');
369+
ASSERT_EQUALS((char)0, type ? type->sign : '?');
370370
}
371371

372372
void container() const {

‎test/testmathlib.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,11 @@ class TestMathLib : public TestFixture {
264264
ASSERT_EQUALS(-1 , MathLib::toLongNumber("-10.E-1"));
265265
ASSERT_EQUALS(100 , MathLib::toLongNumber("+10.0E+1"));
266266
ASSERT_EQUALS(-1 , MathLib::toLongNumber("-10.0E-1"));
267+
268+
ASSERT_EQUALS(-8552249625308161526, MathLib::toLongNumber("0x89504e470d0a1a0a"));
269+
ASSERT_EQUALS(-8481036456200365558, MathLib::toLongNumber("0x8a4d4e470d0a1a0a"));
270+
ASSERT_EQUALS(9894494448401390090, MathLib::toULongNumber("0x89504e470d0a1a0a"));
271+
ASSERT_EQUALS(9965707617509186058, MathLib::toULongNumber("0x8a4d4e470d0a1a0a"));
267272

268273
// zero input
269274
ASSERT_EQUALS(0 , MathLib::toULongNumber("0"));

‎test/testsuite.cpp‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ void TestFixture::assert_(const char *filename, unsigned int linenr, bool condit
129129
}
130130
}
131131

132-
void TestFixture::assertEquals(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg) const
132+
void TestFixture::_assertEquals(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg) const
133133
{
134134
if (expected != actual) {
135135
++fails_counter;
@@ -155,23 +155,6 @@ void TestFixture::assertEquals(const char *filename, unsigned int linenr, const
155155
}
156156
}
157157
}
158-
void TestFixture::assertEquals(const char *filename, unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg) const
159-
{
160-
assertEquals(filename, linenr, std::string(expected), actual, msg);
161-
}
162-
void TestFixture::assertEquals(const char *filename, unsigned int linenr, const char expected[], const char actual[], const std::string &msg) const
163-
{
164-
assertEquals(filename, linenr, std::string(expected), std::string(actual), msg);
165-
}
166-
void TestFixture::assertEquals(const char *filename, unsigned int linenr, const std::string& expected, const char actual[], const std::string &msg) const
167-
{
168-
assertEquals(filename, linenr, expected, std::string(actual), msg);
169-
}
170-
171-
void TestFixture::assertEquals(const char *filename, unsigned int linenr, long long expected, long long actual, const std::string &msg) const
172-
{
173-
assertEquals(filename, linenr, MathLib::toString(expected), MathLib::toString(actual), msg);
174-
}
175158

176159
void TestFixture::assertEqualsDouble(const char *filename, unsigned int linenr, double expected, double actual, const std::string &msg) const
177160
{

‎test/testsuite.h‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,36 @@ class TestFixture : public ErrorLogger {
5151

5252
void assert_(const char *filename, unsigned int linenr, bool condition) const;
5353
void todoAssert(const char *filename, unsigned int linenr, bool condition) const;
54-
54+
void _assertEquals(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg) const;
55+
56+
template <typename E, typename A>
57+
void assertEquals(const char *filename, unsigned int linenr, const E expected, const A actual, const std::string &msg = emptyString) const {
58+
_assertEquals(filename, linenr, MathLib::toString(expected), MathLib::toString(actual), msg);
59+
}
60+
61+
template<>
62+
void assertEquals(const char *filename, unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg) const
63+
{
64+
assertEquals(filename, linenr, std::string(expected), actual, msg);
65+
}
66+
template<>
67+
void assertEquals(const char *filename, unsigned int linenr, const char expected[], const char actual[], const std::string &msg) const
68+
{
69+
assertEquals(filename, linenr, std::string(expected), std::string(actual), msg);
70+
}
71+
template<>
72+
void assertEquals(const char *filename, unsigned int linenr, const std::string& expected, const char actual[], const std::string &msg) const
73+
{
74+
assertEquals(filename, linenr, expected, std::string(actual), msg);
75+
}
76+
/*
5577
void assertEquals(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg = emptyString) const;
5678
void assertEquals(const char *filename, unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg = emptyString) const;
5779
void assertEquals(const char *filename, unsigned int linenr, const char expected[], const char actual[], const std::string &msg = emptyString) const;
5880
void assertEquals(const char *filename, unsigned int linenr, const std::string& expected, const char actual[], const std::string &msg = emptyString) const;
5981
void assertEquals(const char *filename, unsigned int linenr, long long expected, long long actual, const std::string &msg = emptyString) const;
82+
void assertEquals(const char *filename, unsigned int linenr, unsigned long long expected, unsigned long long actual, const std::string &msg = emptyString) const;
83+
*/
6084
void assertEqualsDouble(const char *filename, unsigned int linenr, double expected, double actual, const std::string &msg = emptyString) const;
6185

6286
void todoAssertEquals(const char *filename, unsigned int linenr, const std::string &wanted,

0 commit comments

Comments
 (0)