diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 4ad7d7fc824..58ff47d59ba 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -1988,37 +1988,33 @@ void CheckClass::checkConst() if (func.isFriend() || func.isStatic() || func.hasVirtualSpecifier()) continue; - // don't warn when returning non-const pointer/reference - { - auto isPointerOrReference = [this](const Token* start, const Token* end) -> bool { - bool inTemplArgList = false, isConstTemplArg = false; - for (const Token* tok = start; tok != end; tok = tok->next()) { - if (tok->str() == "{") // end of trailing return type + // don't suggest const when returning non-const pointer/reference, but still suggest static + auto isPointerOrReference = [this](const Token* start, const Token* end) -> bool { + bool inTemplArgList = false, isConstTemplArg = false; + for (const Token* tok = start; tok != end; tok = tok->next()) { + if (tok->str() == "{") // end of trailing return type + return false; + if (tok->str() == "<") { + if (!tok->link()) + mSymbolDatabase->debugMessage(tok, "debug", "CheckClass::checkConst found unlinked template argument list '" + tok->expressionString() + "'."); + inTemplArgList = true; + } + else if (tok->str() == ">") { + inTemplArgList = false; + isConstTemplArg = false; + } + else if (tok->str() == "const") { + if (!inTemplArgList) return false; - if (tok->str() == "<") { - if (!tok->link()) - mSymbolDatabase->debugMessage(tok, "debug", "CheckClass::checkConst found unlinked template argument list '" + tok->expressionString() + "'."); - inTemplArgList = true; - } - else if (tok->str() == ">") { - inTemplArgList = false; - isConstTemplArg = false; - } - else if (tok->str() == "const") { - if (!inTemplArgList) - return false; - isConstTemplArg = true; - } - else if (!isConstTemplArg && Token::Match(tok, "*|&")) - return true; + isConstTemplArg = true; } - return false; - }; - - if (isPointerOrReference(func.retDef, func.tokenDef)) - continue; - } + else if (!isConstTemplArg && Token::Match(tok, "*|&")) + return true; + } + return false; + }; + const bool returnsPtrOrRef = isPointerOrReference(func.retDef, func.tokenDef); if (func.isOperator()) { // Operator without return type: conversion operator const std::string& opName = func.tokenDef->str(); @@ -2043,7 +2039,8 @@ void CheckClass::checkConst() if (!checkConstFunc(scope, &func, memberAccessed)) continue; - if (func.isConst() && (memberAccessed || func.isOperator())) + const bool suggestStatic = !memberAccessed && !func.isOperator(); + if ((returnsPtrOrRef || func.isConst()) && !suggestStatic) continue; std::string classname = scope->className; @@ -2062,9 +2059,9 @@ void CheckClass::checkConst() functionName += "]"; if (func.isInline()) - checkConstError(func.token, classname, functionName, !memberAccessed && !func.isOperator()); + checkConstError(func.token, classname, functionName, suggestStatic); else // not inline - checkConstError2(func.token, func.tokenDef, classname, functionName, !memberAccessed && !func.isOperator()); + checkConstError2(func.token, func.tokenDef, classname, functionName, suggestStatic); } } } diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp index 20d759f5b4e..8d2d4182c55 100644 --- a/lib/reverseanalyzer.cpp +++ b/lib/reverseanalyzer.cpp @@ -65,7 +65,7 @@ struct ReverseTraversal { return true; } - Token* getParentFunction(Token* tok) + static Token* getParentFunction(Token* tok) { if (!tok) return nullptr; @@ -90,7 +90,7 @@ struct ReverseTraversal { return nullptr; } - Token* getTopFunction(Token* tok) + static Token* getTopFunction(Token* tok) { if (!tok) return nullptr; diff --git a/lib/tokenize.h b/lib/tokenize.h index 791d60ae383..b1d9c50e821 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -303,7 +303,7 @@ class CPPCHECKLIB Tokenizer { * '; int *p(0);' => '; int *p = 0;' */ void simplifyInitVar(); - Token * initVar(Token * tok); + static Token* initVar(Token* tok); /** * Simplify easy constant '?:' operation diff --git a/test/testclass.cpp b/test/testclass.cpp index 830dedb3031..ef21cf9b66f 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -195,6 +195,7 @@ class TestClass : public TestFixture { TEST_CASE(const76); // ticket #10825 TEST_CASE(const77); // ticket #10307, #10311 TEST_CASE(const78); // ticket #10315 + TEST_CASE(const79); // ticket #9861 TEST_CASE(const_handleDefaultParameters); TEST_CASE(const_passThisToMemberOfOtherClass); TEST_CASE(assigningPointerToPointerIsNotAConstOperation); @@ -6068,6 +6069,17 @@ class TestClass : public TestFixture { ASSERT_EQUALS("", errout.str()); } + void const79() { // #9861 + checkConst("class A {\n" + "public:\n" + " char* f() {\n" + " return nullptr;\n" + " }\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:3]: (performance, inconclusive) Technically the member function 'A::f' can be static (but you may consider moving to unnamed namespace).\n", + errout.str()); + } + void const_handleDefaultParameters() { checkConst("struct Foo {\n" " void foo1(int i, int j = 0) {\n"