Skip to content

Fix Trac 4651: avoid const-statement warnings for overloaded commas #8893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KiritoYG wants to merge 1 commit into
base: main
Choose a base branch
from
+243 −19
Open

Fix Trac 4651: avoid const-statement warnings for overloaded commas #8893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 95 additions & 19 deletions lib/checkother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2300,7 +2300,81 @@ static bool isConstant(const Token* tok) {
return tok && (tok->isEnumerator() || Token::Match(tok, "%bool%|%num%|%str%|%char%|nullptr|NULL"));
}

static bool isConstStatement(const Token *tok, const Library& library, bool platformIndependent, bool isNestedBracket = false)
static bool isBuiltinCommaOperand(const Token* tok, const Settings& settings)
{
if (!tok)
return false;
const auto builtinType = [](const ValueType* vt) {
return vt && (vt->pointer > 0 || vt->type == ValueType::VOID || (vt->isPrimitive() && !vt->isEnum()));
};
// A comma chain can itself return an object through an overloaded operator.
if (tok->str() == ",")
return isBuiltinCommaOperand(tok->astOperand1(), settings) && isBuiltinCommaOperand(tok->astOperand2(), settings);
if (!tok->isAssignmentOp())
return builtinType(tok->valueType());
const ValueType* lhs = tok->astOperand1() ? tok->astOperand1()->valueType() : nullptr;
const ValueType* rhs = tok->astOperand2() ? tok->astOperand2()->valueType() : nullptr;
if (builtinType(lhs) && (tok->str() == "=" || builtinType(rhs)))
return true;

// Assignment value types are copied from the lhs, even when an overload
// returns another type. Do not select an arbitrary overload: only rely on
// the return type when all visible candidates have a built-in result.
for (const ValueType* vt : {lhs, rhs}) {
if (!builtinType(vt) && (!vt || !vt->typeScope))
return false;
if (vt && vt->typeScope) {
// Template aliases/qualifications and inherited overloads cannot
// be resolved reliably by comparing the available parameter types.
if (vt->typeScope->className.find('<') != std::string::npos ||
(vt->typeScope->definedType && !vt->typeScope->definedType->derivedFrom.empty()))
return false;
}
}
if (!tok->scope())
return false;
bool found = false;
const std::string name = "operator" + tok->str();
// Include non-member candidates conservatively, including hidden friends
// and anonymous namespaces, without trying to rank overloads.
for (const Scope& scope : tok->scope()->symdb.scopeList) {
const auto range = scope.functionMap.equal_range(name);
for (auto it = range.first; it != range.second; ++it) {
const Function* function = it->second;
const bool member = scope.isClassOrStruct() && !function->isFriend();
if (member && (!lhs || lhs->typeScope != &scope))
continue;
if (function->argCount() != (member ? 1U : 2U))
continue;
bool matches = true;
for (unsigned int arg = 0; arg < function->argCount(); ++arg) {
const ValueType* operand = member || arg == 1U ? rhs : lhs;
const ValueType* parameter = function->getArgumentVar(arg)->valueType();
const auto match = ValueType::matchParameter(operand, parameter);
if (match == ValueType::MatchResult::UNKNOWN)
return false;
if (match == ValueType::MatchResult::NOMATCH) {
// A user-defined conversion may still make this candidate
// viable; matchParameter does not resolve those conversions.
if (!builtinType(operand) || !builtinType(parameter))
return false;
matches = false;
}
}
if (!matches)
continue;
if (!function->retDef)
return false;
const ValueType result = ValueType::parseDecl(function->retDef, settings);
if (!builtinType(&result))
return false;
found = true;
}
}
return found;
}

static bool isConstStatement(const Token *tok, const Settings& settings, bool platformIndependent, bool isNestedBracket = false)
{
if (!tok)
return false;
Expand All @@ -2322,7 +2396,7 @@ static bool isConstStatement(const Token *tok, const Library& library, bool plat
tok2 = tok2->astParent();
}
if (Token::Match(tok, "&&|%oror%"))
return isConstStatement(tok->astOperand1(), library, platformIndependent) && isConstStatement(tok->astOperand2(), library, platformIndependent);
return isConstStatement(tok->astOperand1(), settings, platformIndependent) && isConstStatement(tok->astOperand2(), settings, platformIndependent);
if (Token::Match(tok, "!|~|%cop%") && (tok->astOperand1() || tok->astOperand2()))
return true;
if (Token::simpleMatch(tok->previous(), "sizeof ("))
Expand All @@ -2332,38 +2406,40 @@ static bool isConstStatement(const Token *tok, const Library& library, bool plat
if (isCPPCast(tok)) {
if (Token::simpleMatch(tok->astOperand1(), "dynamic_cast") && Token::simpleMatch(tok->astOperand1()->linkAt(1)->previous(), "& >"))
return false;
return isWithoutSideEffects(tok) && isConstStatement(tok->astOperand2(), library, platformIndependent);
return isWithoutSideEffects(tok) && isConstStatement(tok->astOperand2(), settings, platformIndependent);
}
if (tok->isCast() && tok->next() && tok->next()->isStandardType())
return isWithoutSideEffects(tok->astOperand1()) && isConstStatement(tok->astOperand1(), library, platformIndependent);
return isWithoutSideEffects(tok->astOperand1()) && isConstStatement(tok->astOperand1(), settings, platformIndependent);
if (Token::simpleMatch(tok, "."))
return isConstStatement(tok->astOperand2(), library, platformIndependent);
return isConstStatement(tok->astOperand2(), settings, platformIndependent);
if (Token::simpleMatch(tok, ",")) {
if (tok->isCpp() && (!isBuiltinCommaOperand(tok->astOperand1(), settings) || !isBuiltinCommaOperand(tok->astOperand2(), settings)))
return false;
if (tok->astParent()) // warn about const statement on rhs at the top level
return isConstStatement(tok->astOperand1(), library, platformIndependent) &&
isConstStatement(tok->astOperand2(), library, platformIndependent);
return isConstStatement(tok->astOperand1(), settings, platformIndependent) &&
isConstStatement(tok->astOperand2(), settings, platformIndependent);

const Token* lml = previousBeforeAstLeftmostLeaf(tok); // don't warn about matrix/vector assignment (e.g. Eigen)
if (lml)
lml = lml->next();
const Token* stream = lml;
while (stream && Token::Match(stream->astParent(), ".|[|(|*"))
stream = stream->astParent();
return (!stream || !isLikelyStream(stream)) && isConstStatement(tok->astOperand2(), library, platformIndependent);
return (!stream || !isLikelyStream(stream)) && isConstStatement(tok->astOperand2(), settings, platformIndependent);
}
if (Token::simpleMatch(tok, "?") && Token::simpleMatch(tok->astOperand2(), ":")) // ternary operator
return isConstStatement(tok->astOperand1(), library, platformIndependent) &&
isConstStatement(tok->astOperand2()->astOperand1(), library, platformIndependent) &&
isConstStatement(tok->astOperand2()->astOperand2(), library, platformIndependent);
return isConstStatement(tok->astOperand1(), settings, platformIndependent) &&
isConstStatement(tok->astOperand2()->astOperand1(), settings, platformIndependent) &&
isConstStatement(tok->astOperand2()->astOperand2(), settings, platformIndependent);
if (isBracketAccess(tok) && isWithoutSideEffects(tok->astOperand1(), /*checkArrayAccess*/ true, /*checkReference*/ false)) {
const bool isChained = succeeds(tok->astParent(), tok);
if (Token::simpleMatch(tok->astParent(), "[")) {
if (isChained)
return isConstStatement(tok->astOperand2(), library, platformIndependent) &&
isConstStatement(tok->astParent(), library, platformIndependent);
return isNestedBracket && isConstStatement(tok->astOperand2(), library, platformIndependent);
return isConstStatement(tok->astOperand2(), settings, platformIndependent) &&
isConstStatement(tok->astParent(), settings, platformIndependent);
return isNestedBracket && isConstStatement(tok->astOperand2(), settings, platformIndependent);
}
return isConstStatement(tok->astOperand2(), library, platformIndependent, /*isNestedBracket*/ !isChained);
return isConstStatement(tok->astOperand2(), settings, platformIndependent, /*isNestedBracket*/ !isChained);
}
if (!tok->astParent() && findLambdaEndToken(tok))
return true;
Expand All @@ -2379,7 +2455,7 @@ static bool isConstStatement(const Token *tok, const Library& library, bool plat
funcStr.insert(0, tok2->strAt(-2) + "::");
tok2 = tok2->tokAt(-2);
}
if (library.functions().count(funcStr) > 0)
if (settings.library.functions().count(funcStr) > 0)
return true;
}
return false;
Expand Down Expand Up @@ -2468,7 +2544,7 @@ void CheckOtherImpl::checkIncompleteStatement()
// Skip statement expressions
if (Token::simpleMatch(rtok, "; } )") || Token::simpleMatch(tok->next(), "; } )"))
continue;
if (!isConstStatement(tok, mSettings.library, false))
if (!isConstStatement(tok, mSettings, false))
continue;
if (isVoidStmt(tok))
continue;
Expand Down Expand Up @@ -2657,7 +2733,7 @@ void CheckOtherImpl::checkMisusedScopedObject()
if (Token::simpleMatch(parTok, "<") && parTok->link())
parTok = parTok->link()->next();
if (const Token* arg = parTok->astOperand2()) {
if (!isConstStatement(arg, mSettings.library, false))
if (!isConstStatement(arg, mSettings, false))
continue;
if (parTok->str() == "(") {
if (arg->varId() && !(arg->variable() && arg->variable()->nameToken() != arg))
Expand Down Expand Up @@ -3138,7 +3214,7 @@ void CheckOtherImpl::checkDuplicateExpression()

else if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2()) &&
!isVariableChanged(tok->astParent(), /*indirect*/ 0, mSettings) &&
isConstStatement(tok->astOperand1(), mSettings.library, true) && isConstStatement(tok->astOperand2(), mSettings.library, true))
isConstStatement(tok->astOperand1(), mSettings, true) && isConstStatement(tok->astOperand2(), mSettings, true))
duplicateValueTernaryError(tok);
}
}
Expand Down
148 changes: 148 additions & 0 deletions test/testincompletestatement.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ class TestIncompleteStatement : public TestFixture {
TEST_CASE(mapindex);
TEST_CASE(commaoperator1);
TEST_CASE(commaoperator2);
TEST_CASE(commaoperatorOverloaded);
TEST_CASE(commaoperatorBuiltin);
TEST_CASE(commaoperatorOverloadCandidates);
TEST_CASE(redundantstmts);
TEST_CASE(vardecl);
TEST_CASE(archive); // ar & x
Expand Down Expand Up @@ -434,6 +437,151 @@ class TestIncompleteStatement : public TestFixture {
ASSERT_EQUALS("", errout_str());
}

void commaoperatorOverloaded() { // #4651
check("void f() {\n"
" using namespace boost::assign;\n"
" std::vector<int> values;\n"
" values += 2, 2;\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("struct Collector {\n"
" Collector& operator+=(int);\n"
" Collector& operator,(int);\n"
"};\n"
"void f(Collector& values) {\n"
" values += 1, 2, 3;\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("struct Collector {};\n"
"Collector& operator+=(Collector&, int);\n"
"Collector& operator,(Collector&, int);\n"
"void f(Collector& values) {\n"
" values += 1, 2;\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("void f(Unknown& values) {\n"
" values += 1, 2;\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("struct Collector { void operator,(int); };\n"
"void f(Collector& values) {\n"
" values, 2;\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("enum Value { First };\n"
"void operator,(Value, int);\n"
"void f(Value value) {\n"
" value, 2;\n"
"}\n");
ASSERT_EQUALS("", errout_str());
}

void commaoperatorBuiltin() {
check("void f(int& value) {\n"
" value += 1, 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2:15]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());

check("struct S { operator int() const; };\n"
"void f(S value) {\n"
" int i;\n"
" i = value, 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4:14]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());

check("void f(int*& value) {\n"
" value = nullptr, 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2:20]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());

check("void f(int* value) {\n"
" value += 1, 2;\n"
"}\n", dinit(CheckOptions, $.cpp = false));
ASSERT_EQUALS("[test.c:2:15]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());

check("struct Collector { int operator+=(int); };\n"
"void f(Collector& values) {\n"
" values += 1, 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3:16]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());

check("struct Collector { void operator+=(int); };\n"
"void f(Collector& values) {\n"
" values += 1, 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3:16]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());

check("struct Collector {};\n"
"int operator+=(Collector&, int);\n"
"void f(Collector& values) {\n"
" values += 1, 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4:16]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());

check("struct Collector { void operator,(int); };\n"
"void f(Collector& values) {\n"
" (void)values, 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());

check("struct Collector { Collector* operator+=(int); };\n"
"void f(Collector& values) {\n"
" values += 1, 2;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3:16]: (warning) Found suspicious operator ',', result is not used. [constStatement]\n", errout_str());
}

void commaoperatorOverloadCandidates() {
check("struct Collector { int operator+=(double); };\n"
"namespace {\n"
" Collector& operator+=(Collector&, int);\n"
" Collector& operator,(Collector&, int);\n"
"}\n"
"void f(Collector& values) { values += 1, 2; }\n");
ASSERT_EQUALS("", errout_str());

check("struct Collector {\n"
" int operator+=(double);\n"
" Collector& operator+=(int);\n"
" void operator,(int);\n"
"};\n"
"void f(Collector& values) { values += 1, 2; }\n");
ASSERT_EQUALS("", errout_str());

check("struct Collector {\n"
" Collector& operator+=(int);\n"
" int operator+=(double);\n"
" void operator,(int);\n"
"};\n"
"void f(Collector& values) { values += 1, 2; }\n");
ASSERT_EQUALS("", errout_str());

check("namespace N { struct Tag {}; }\n"
"template<class T> struct Box { int operator+=(double); };\n"
"namespace N {\n"
" Box<Tag>& operator+=(Box<Tag>&, int);\n"
" Box<Tag>& operator,(Box<Tag>&, int);\n"
"}\n"
"void f(Box<N::Tag>& values) { values += 1, 2; }\n");
ASSERT_EQUALS("", errout_str());

check("struct B {};\n"
"struct A { operator B() const; };\n"
"struct Collector {\n"
" int operator+=(A) &&;\n"
" Collector& operator+=(B) &;\n"
" void operator,(int);\n"
"};\n"
"void f(Collector& values, A a) { values += a, 2; }\n");
ASSERT_EQUALS("", errout_str());

}

// #8451
void redundantstmts() {
check("void f1(int x) {\n"
Expand Down
Loading