Skip to content

Commit 4b72f01

Browse files
committed
clarify calculation: Fixed FP for 'a = (*p ? 1 : 2)'
1 parent 4a46029 commit 4b72f01

5 files changed

Lines changed: 51 additions & 42 deletions

File tree

lib/checkother.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ void CheckOther::clarifyCalculation()
180180
const Scope * scope = symbolDatabase->functionScopes[i];
181181
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
182182
// ? operator where lhs is arithmetical expression
183-
if (tok->str() != "?" || !tok->astOperand1() || !tok->astOperand1()->isArithmeticalOp())
183+
if (tok->str() != "?" || !tok->astOperand1() || !tok->astOperand1()->isArithmeticalOp() || !tok->astOperand1()->isCalculation())
184184
continue;
185185

186186
// Is code clarified by parentheses already?

lib/checksizeof.cpp

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include "symboldatabase.h"
2323
#include <algorithm>
2424
#include <cctype>
25-
#include <stack>
2625
//---------------------------------------------------------------------------
2726

2827
// Register this check class (by creating a static instance of it)
@@ -219,45 +218,6 @@ void CheckSizeof::sizeofsizeofError(const Token *tok)
219218
}
220219

221220
//-----------------------------------------------------------------------------
222-
//-----------------------------------------------------------------------------
223-
static bool isCalculation(const Token *op)
224-
{
225-
if (!op)
226-
return false;
227-
228-
if (!Token::Match(op, "%cop%|++|--"))
229-
return false;
230-
231-
if (Token::Match(op, "*|&")) {
232-
// dereference or address-of?
233-
if (!op->astOperand2())
234-
return false;
235-
236-
if (op->astOperand2()->str() == "[")
237-
return false;
238-
239-
// type specification?
240-
std::stack<const Token *> operands;
241-
operands.push(op);
242-
while (!operands.empty()) {
243-
const Token *item = operands.top();
244-
operands.pop();
245-
if (item->isNumber() || item->varId() > 0)
246-
return true;
247-
if (item->astOperand1())
248-
operands.push(item->astOperand1());
249-
if (item->astOperand2())
250-
operands.push(item->astOperand2());
251-
else if (Token::Match(item, "*|&"))
252-
return false;
253-
}
254-
255-
// type specification => return false
256-
return false;
257-
}
258-
259-
return true;
260-
}
261221

262222
void CheckSizeof::sizeofCalculation()
263223
{
@@ -267,7 +227,7 @@ void CheckSizeof::sizeofCalculation()
267227
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
268228
if (Token::simpleMatch(tok, "sizeof (")) {
269229
const Token *argument = tok->next()->astOperand2();
270-
if (isCalculation(argument) && (!argument->isExpandedMacro() || _settings->inconclusive))
230+
if (argument && argument->isCalculation() && (!argument->isExpandedMacro() || _settings->inconclusive))
271231
sizeofCalculationError(argument, argument->isExpandedMacro());
272232
}
273233
}

lib/token.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <cctype>
2828
#include <sstream>
2929
#include <map>
30+
#include <stack>
3031

3132
Token::Token(Token **t) :
3233
tokensBack(t),
@@ -1143,6 +1144,42 @@ void Token::astOperand2(Token *tok)
11431144
_astOperand2 = tok;
11441145
}
11451146

1147+
bool Token::isCalculation() const
1148+
{
1149+
if (!Token::Match(this, "%cop%|++|--"))
1150+
return false;
1151+
1152+
if (Token::Match(this, "*|&")) {
1153+
// dereference or address-of?
1154+
if (!this->astOperand2())
1155+
return false;
1156+
1157+
if (this->astOperand2()->str() == "[")
1158+
return false;
1159+
1160+
// type specification?
1161+
std::stack<const Token *> operands;
1162+
operands.push(this);
1163+
while (!operands.empty()) {
1164+
const Token *op = operands.top();
1165+
operands.pop();
1166+
if (op->isNumber() || op->varId() > 0)
1167+
return true;
1168+
if (op->astOperand1())
1169+
operands.push(op->astOperand1());
1170+
if (op->astOperand2())
1171+
operands.push(op->astOperand2());
1172+
else if (Token::Match(op, "*|&"))
1173+
return false;
1174+
}
1175+
1176+
// type specification => return false
1177+
return false;
1178+
}
1179+
1180+
return true;
1181+
}
1182+
11461183
void Token::printAst() const
11471184
{
11481185
bool title = false;

lib/token.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,15 @@ class CPPCHECKLIB Token {
672672
return ret;
673673
}
674674

675+
/**
676+
* Is current token a calculation? Only true for operands.
677+
* For '*' and '&' tokens it is looked up if this is a
678+
* dereference or address-of. A dereference or address-of is not
679+
* counted as a calculation.
680+
* @return returns true if current token is a calculation
681+
*/
682+
bool isCalculation() const;
683+
675684
void clearAst() {
676685
_astOperand1 = _astOperand2 = _astParent = NULL;
677686
}

test/testother.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4174,6 +4174,9 @@ class TestOther : public TestFixture {
41744174
" return shift < sizeof(int64_t)*8 ? 1 : 2;\n"
41754175
"}");
41764176
ASSERT_EQUALS("", errout.str());
4177+
4178+
check("void f() { a = *p ? 1 : 2; }");
4179+
ASSERT_EQUALS("", errout.str());
41774180
}
41784181

41794182
void clarifyStatement() {

0 commit comments

Comments
 (0)