Skip to content

Commit bb52a63

Browse files
pfultz2danmar
pfultz2
authored and
danmar
committed
Add check for const variables
When a local reference is declared, this will check if that local reference can be declared as `const`.
1 parent 4c3191e commit bb52a63

File tree

8 files changed

+780
-27
lines changed

8 files changed

+780
-27
lines changed

‎lib/astutils.cpp‎

Lines changed: 103 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,17 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
176176
return ret;
177177
}
178178

179+
bool isFunctionCall(const Token* tok)
180+
{
181+
if (Token::Match(tok, "%name% ("))
182+
return true;
183+
if (Token::Match(tok, "%name% <") && Token::simpleMatch(tok->next()->link(), "> ("))
184+
return true;
185+
if (Token::Match(tok, "%name% ::"))
186+
return isFunctionCall(tok->tokAt(2));
187+
return false;
188+
}
189+
179190
static bool hasToken(const Token * startTok, const Token * stopTok, const Token * tok)
180191
{
181192
for (const Token * tok2 = startTok; tok2 != stopTok; tok2 = tok2->next()) {
@@ -227,8 +238,10 @@ bool precedes(const Token * tok1, const Token * tok2)
227238
return tok1->index() < tok2->index();
228239
}
229240

230-
static bool isAliased(const Token * startTok, const Token * endTok, nonneg int varid)
241+
bool isAliased(const Token *startTok, const Token *endTok, nonneg int varid)
231242
{
243+
if (!precedes(startTok, endTok))
244+
return false;
232245
for (const Token *tok = startTok; tok != endTok; tok = tok->next()) {
233246
if (Token::Match(tok, "= & %varid% ;", varid))
234247
return true;
@@ -248,6 +261,18 @@ static bool isAliased(const Token * startTok, const Token * endTok, nonneg int v
248261
return false;
249262
}
250263

264+
bool isAliased(const Variable *var)
265+
{
266+
if (!var)
267+
return false;
268+
if (!var->scope())
269+
return false;
270+
const Token *start = var->declEndToken();
271+
if (!start)
272+
return false;
273+
return isAliased(start, var->scope()->bodyEnd, var->declarationId());
274+
}
275+
251276
static bool exprDependsOnThis(const Token *expr, nonneg int depth)
252277
{
253278
if (!expr)
@@ -814,6 +839,19 @@ bool isVariableChangedByFunctionCall(const Token *tok, nonneg int varid, const S
814839
isVariableChangedByFunctionCall(tok->astOperand2(), varid, settings, inconclusive);
815840
}
816841

842+
static bool isScopeBracket(const Token *tok)
843+
{
844+
if (!Token::Match(tok, "{|}"))
845+
return false;
846+
if (!tok->scope())
847+
return false;
848+
if (tok->str() == "{")
849+
return tok->scope()->bodyStart == tok;
850+
if (tok->str() == "}")
851+
return tok->scope()->bodyEnd == tok;
852+
return false;
853+
}
854+
817855
bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool *inconclusive)
818856
{
819857
if (!tok)
@@ -832,7 +870,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
832870
parent = parent->astParent();
833871

834872
// passing variable to subfunction?
835-
if (Token::Match(parent, "[(,]"))
873+
if (Token::Match(parent, "[(,{]"))
836874
;
837875
else if (Token::simpleMatch(parent, ":")) {
838876
while (Token::Match(parent, "[?:]"))
@@ -847,24 +885,24 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
847885

848886
// goto start of function call and get argnr
849887
int argnr = 0;
850-
while (tok && !Token::Match(tok, "[;{}]")) {
888+
while (tok && !Token::simpleMatch(tok, ";") && !isScopeBracket(tok)) {
851889
if (tok->str() == ",")
852890
++argnr;
853891
else if (tok->str() == ")")
854892
tok = tok->link();
855-
else if (Token::Match(tok->previous(), "%name% ("))
893+
else if (Token::Match(tok->previous(), "%name% (|{"))
856894
break;
857-
else if (Token::simpleMatch(tok->previous(), "> (") && tok->previous()->link())
895+
else if (Token::Match(tok->previous(), "> (|{") && tok->previous()->link())
858896
break;
859897
tok = tok->previous();
860898
}
861-
if (!tok || tok->str() != "(")
899+
if (!Token::Match(tok, "{|("))
862900
return false;
863-
const bool possiblyPassedByReference = (tok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)]"));
901+
const bool possiblyPassedByReference = (tok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]"));
864902
tok = tok->previous();
865903
if (tok && tok->link() && tok->str() == ">")
866904
tok = tok->link()->previous();
867-
if (!Token::Match(tok, "%name% [(<]"))
905+
if (!Token::Match(tok, "%name% [({<]"))
868906
return false; // not a function => variable not changed
869907

870908
// Constructor call
@@ -932,6 +970,8 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
932970

933971
bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp)
934972
{
973+
if (!precedes(start, end))
974+
return false;
935975
for (const Token *tok = start; tok != end; tok = tok->next()) {
936976
if (tok->varId() != varid) {
937977
if (globalvar && Token::Match(tok, "%name% ("))
@@ -941,20 +981,32 @@ bool isVariableChanged(const Token *start, const Token *end, const nonneg int va
941981
}
942982

943983
const Token *tok2 = tok;
944-
while (Token::simpleMatch(tok2->astParent(), "*"))
984+
while (Token::simpleMatch(tok2->astParent(), "*") || (Token::simpleMatch(tok2->astParent(), ".") && !Token::simpleMatch(tok2->astParent()->astParent(), "(")) ||
985+
(Token::simpleMatch(tok2->astParent(), "[") && tok2 == tok2->astParent()->astOperand1()))
945986
tok2 = tok2->astParent();
946987

947988
if (Token::Match(tok2->astParent(), "++|--"))
948989
return true;
949990

950-
if (tok2->astParent() && tok2->astParent()->isAssignmentOp() && tok2 == tok2->astParent()->astOperand1())
951-
return true;
991+
if (tok2->astParent() && tok2->astParent()->isAssignmentOp()) {
992+
if (tok2 == tok2->astParent()->astOperand1())
993+
return true;
994+
// Check if assigning to a non-const lvalue
995+
const Variable * var = getLHSVariable(tok2->astParent());
996+
if (var && var->isReference() && !var->isConst() && var->nameToken() && var->nameToken()->next() == tok2->astParent()) {
997+
if (!var->isLocal() || isVariableChanged(var, settings, cpp))
998+
return true;
999+
}
1000+
}
9521001

9531002
if (isLikelyStreamRead(cpp, tok->previous()))
9541003
return true;
9551004

1005+
if (isLikelyStream(cpp, tok2))
1006+
return true;
1007+
9561008
// Member function call
957-
if (Token::Match(tok, "%name% . %name% (")) {
1009+
if (tok->variable() && Token::Match(tok2->astParent(), ". %name%") && isFunctionCall(tok2->astParent()->next()) && tok2->astParent()->astOperand1() == tok2) {
9581010
const Variable * var = tok->variable();
9591011
bool isConst = var && var->isConst();
9601012
if (!isConst && var) {
@@ -966,25 +1018,42 @@ bool isVariableChanged(const Token *start, const Token *end, const nonneg int va
9661018
const Function * fun = ftok->function();
9671019
if (!isConst && (!fun || !fun->isConst()))
9681020
return true;
1021+
else
1022+
continue;
9691023
}
9701024

971-
const Token *ftok = tok;
972-
while (ftok && (!Token::Match(ftok, "[({[]") || ftok->isCast()))
1025+
const Token *ftok = tok2;
1026+
while (ftok && (!Token::Match(ftok, "[({]") || ftok->isCast()))
9731027
ftok = ftok->astParent();
9741028

975-
if (ftok && Token::Match(ftok->link(), ") !!{")) {
1029+
if (ftok && Token::Match(ftok->link(), ")|} !!{")) {
1030+
const Token * ptok = tok2;
1031+
while (Token::Match(ptok->astParent(), ".|::|["))
1032+
ptok = ptok->astParent();
9761033
bool inconclusive = false;
977-
bool isChanged = isVariableChangedByFunctionCall(tok, settings, &inconclusive);
1034+
bool isChanged = isVariableChangedByFunctionCall(ptok, settings, &inconclusive);
9781035
isChanged |= inconclusive;
9791036
if (isChanged)
9801037
return true;
9811038
}
9821039

983-
const Token *parent = tok->astParent();
1040+
const Token *parent = tok2->astParent();
9841041
while (Token::Match(parent, ".|::"))
9851042
parent = parent->astParent();
9861043
if (parent && parent->tokType() == Token::eIncDecOp)
9871044
return true;
1045+
1046+
if (Token::simpleMatch(tok2->astParent(), ":") && tok2->astParent()->astParent() && Token::simpleMatch(tok2->astParent()->astParent()->previous(), "for (")) {
1047+
const Token * varTok = tok2->astParent()->previous();
1048+
if (!varTok)
1049+
continue;
1050+
const Variable * loopVar = varTok->variable();
1051+
if (!loopVar)
1052+
continue;
1053+
if (!loopVar->isConst() && loopVar->isReference() && isVariableChanged(loopVar, settings, cpp))
1054+
return true;
1055+
continue;
1056+
}
9881057
}
9891058
return false;
9901059
}
@@ -1072,6 +1141,23 @@ const Token *findLambdaEndToken(const Token *first)
10721141
return nullptr;
10731142
}
10741143

1144+
bool isLikelyStream(bool cpp, const Token *stream)
1145+
{
1146+
if (!cpp)
1147+
return false;
1148+
1149+
if (!stream)
1150+
return false;
1151+
1152+
if (!Token::Match(stream->astParent(), "&|<<|>>") || !stream->astParent()->isBinaryOp())
1153+
return false;
1154+
1155+
if (stream->astParent()->astOperand1() != stream)
1156+
return false;
1157+
1158+
return !astIsIntegral(stream, false);
1159+
}
1160+
10751161
bool isLikelyStreamRead(bool cpp, const Token *op)
10761162
{
10771163
if (!cpp)

‎lib/astutils.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ bool isVariableChanged(const Token *start, const Token *end, const nonneg int va
137137

138138
bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp);
139139

140+
bool isAliased(const Token *startTok, const Token *endTok, unsigned int varid);
141+
142+
bool isAliased(const Variable *var);
143+
140144
/** Determines the number of arguments - if token is a function call or macro
141145
* @param start token which is supposed to be the function/macro name.
142146
* \return Number of arguments
@@ -157,6 +161,8 @@ const Token *findLambdaStartToken(const Token *last);
157161
*/
158162
const Token *findLambdaEndToken(const Token *first);
159163

164+
bool isLikelyStream(bool cpp, const Token *stream);
165+
160166
/**
161167
* do we see a likely write of rhs through overloaded operator
162168
* s >> x;

‎lib/checkother.cpp‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,89 @@ void CheckOther::passedByValueError(const Token *tok, const std::string &parname
12571257
"as a const reference which is usually faster and recommended in C++.", CWE398, inconclusive);
12581258
}
12591259

1260+
static bool isUnusedVariable(const Variable *var)
1261+
{
1262+
if (!var)
1263+
return false;
1264+
if (!var->scope())
1265+
return false;
1266+
const Token *start = var->declEndToken();
1267+
if (!start)
1268+
return false;
1269+
if (Token::Match(start, "; %varid% =", var->declarationId()))
1270+
start = start->tokAt(2);
1271+
return !Token::findmatch(start->next(), "%varid%", var->scope()->bodyEnd, var->declarationId());
1272+
}
1273+
1274+
void CheckOther::checkConstVariable()
1275+
{
1276+
if (!mSettings->isEnabled(Settings::STYLE) || mTokenizer->isC())
1277+
return;
1278+
1279+
const SymbolDatabase *const symbolDatabase = mTokenizer->getSymbolDatabase();
1280+
1281+
for (const Variable *var : symbolDatabase->variableList()) {
1282+
if (!var)
1283+
continue;
1284+
if (!var->isReference())
1285+
continue;
1286+
if (var->isRValueReference())
1287+
continue;
1288+
if (var->isConst())
1289+
continue;
1290+
if (!var->scope())
1291+
continue;
1292+
const Scope *scope = var->scope();
1293+
if (!scope->function)
1294+
continue;
1295+
Function *function = scope->function;
1296+
if (var->isArgument()) {
1297+
if (function->isImplicitlyVirtual() || function->templateDef)
1298+
continue;
1299+
if (isUnusedVariable(var))
1300+
continue;
1301+
const Token * memberTok = Token::findmatch(function->constructorMemberInitialization(), "%var% ( %varid% )", scope->bodyStart, var->declarationId());
1302+
if (memberTok && memberTok->variable() && memberTok->variable()->isReference())
1303+
continue;
1304+
}
1305+
if (var->isGlobal())
1306+
continue;
1307+
if (var->isStatic())
1308+
continue;
1309+
if (var->isArray())
1310+
continue;
1311+
if (var->isEnumType())
1312+
continue;
1313+
if (var->isVolatile())
1314+
continue;
1315+
if (isAliased(var))
1316+
continue;
1317+
if (isVariableChanged(var, mSettings, mTokenizer->isCPP()))
1318+
continue;
1319+
if (Function::returnsReference(function) &&
1320+
Token::findmatch(var->nameToken(), "return %varid% ;|[", scope->bodyEnd, var->declarationId()))
1321+
continue;
1322+
// Skip if address is taken
1323+
if (Token::findmatch(var->nameToken(), "& %varid%", scope->bodyEnd, var->declarationId()))
1324+
continue;
1325+
constVariableError(var);
1326+
}
1327+
}
1328+
1329+
void CheckOther::constVariableError(const Variable *var)
1330+
{
1331+
const Token *tok = nullptr;
1332+
std::string name = "x";
1333+
std::string id = "Variable";
1334+
if (var) {
1335+
tok = var->nameToken();
1336+
name = var->name();
1337+
if (var->isArgument())
1338+
id = "Parameter";
1339+
}
1340+
reportError(tok, Severity::style, "const" + id, id + " '" + name + "' can be declared with const", CWE398, false);
1341+
}
1342+
12601343
//---------------------------------------------------------------------------
12611344
// Check usage of char variables..
12621345
//---------------------------------------------------------------------------

‎lib/checkother.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ class CPPCHECKLIB CheckOther : public Check {
8989
checkOther.checkRedundantCopy();
9090
checkOther.clarifyCalculation();
9191
checkOther.checkPassByReference();
92+
checkOther.checkConstVariable();
9293
checkOther.checkComparisonFunctionIsAlwaysTrueOrFalse();
9394
checkOther.checkInvalidFree();
9495
checkOther.clarifyStatement();
@@ -119,6 +120,8 @@ class CPPCHECKLIB CheckOther : public Check {
119120
/** @brief %Check for function parameters that should be passed by reference */
120121
void checkPassByReference();
121122

123+
void checkConstVariable();
124+
122125
/** @brief Using char variable as array index / as operand in bit operation */
123126
void checkCharVariable();
124127

@@ -218,6 +221,7 @@ class CPPCHECKLIB CheckOther : public Check {
218221
void cstyleCastError(const Token *tok);
219222
void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive);
220223
void passedByValueError(const Token *tok, const std::string &parname, bool inconclusive);
224+
void constVariableError(const Variable *var);
221225
void constStatementError(const Token *tok, const std::string &type, bool inconclusive);
222226
void signedCharArrayIndexError(const Token *tok);
223227
void unknownSignCharArrayIndexError(const Token *tok);
@@ -288,6 +292,7 @@ class CPPCHECKLIB CheckOther : public Check {
288292
c.checkCastIntToCharAndBackError(nullptr, "func_name");
289293
c.cstyleCastError(nullptr);
290294
c.passedByValueError(nullptr, "parametername", false);
295+
c.constVariableError(nullptr);
291296
c.constStatementError(nullptr, "type", false);
292297
c.signedCharArrayIndexError(nullptr);
293298
c.unknownSignCharArrayIndexError(nullptr);
@@ -390,7 +395,8 @@ class CPPCHECKLIB CheckOther : public Check {
390395
"- find unused 'goto' labels.\n"
391396
"- function declaration and definition argument names different.\n"
392397
"- function declaration and definition argument order different.\n"
393-
"- shadow variable.\n";
398+
"- shadow variable.\n"
399+
"- variable can be declared const.\n";
394400
}
395401
};
396402
/// @}

0 commit comments

Comments
 (0)