From 7c93f51885a9c57bf57d7cd54012b51d0ef17d80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl=20Michael=20Gr=C3=BCner=20Monz=C3=B3n?= Date: Sat, 18 Apr 2020 01:26:24 -0600 Subject: [PATCH] Consider pre{inc,dec}rements on assert checks (#2605) * Consider pre{inc,dec}rements on assert checks * Simplify code by using new AST APIs * Fix assert test with invalid syntax --- lib/checkassert.cpp | 19 +++++++++++-------- test/testassert.cpp | 9 ++++++++- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/checkassert.cpp b/lib/checkassert.cpp index a5edcfcd6..e51ce1b5f 100644 --- a/lib/checkassert.cpp +++ b/lib/checkassert.cpp @@ -120,13 +120,16 @@ void CheckAssert::assignmentInAssertError(const Token *tok, const std::string& v // checks if side effects happen on the variable prior to tmp void CheckAssert::checkVariableAssignment(const Token* assignTok, const Scope *assertionScope) { - const Variable* prevVar = assignTok->previous()->variable(); - if (!prevVar) + if (!assignTok->isAssignmentOp() && assignTok->tokType() != Token::eIncDecOp) + return; + + const Variable* var = assignTok->astOperand1()->variable(); + if (!var) return; // Variable declared in inner scope in assert => don't warn - if (assertionScope != prevVar->scope()) { - const Scope *s = prevVar->scope(); + if (assertionScope != var->scope()) { + const Scope *s = var->scope(); while (s && s != assertionScope) s = s->nestedIn; if (s == assertionScope) @@ -135,12 +138,12 @@ void CheckAssert::checkVariableAssignment(const Token* assignTok, const Scope *a // assignment if (assignTok->isAssignmentOp() || assignTok->tokType() == Token::eIncDecOp) { - if (prevVar->isConst()) + if (var->isConst()) { return; - - assignmentInAssertError(assignTok, prevVar->name()); + } + assignmentInAssertError(assignTok, var->name()); } - // TODO: function calls on prevVar + // TODO: function calls on var } bool CheckAssert::inSameScope(const Token* returnTok, const Token* assignTok) diff --git a/test/testassert.cpp b/test/testassert.cpp index ef093c87c..2ef4b7413 100644 --- a/test/testassert.cpp +++ b/test/testassert.cpp @@ -183,7 +183,7 @@ private: ASSERT_EQUALS("", errout.str()); check("void f(int a, int b) {\n" - " assert(a == 2 && b = 1);\n" + " assert(a == 2 && (b = 1));\n" " return a;\n" "}\n" ); @@ -220,6 +220,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str()); + check("void f() {\n" + " int a = 0;\n" + " assert(--a);\n" + " return a;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Assert statement modifies 'a'.\n", errout.str()); + check("void f() {\n" " assert(std::all_of(first, last, []() {\n" " auto tmp = x.someValue();\n"