Skip to content

ValueFlow: avoid various unnecessary copies #7583

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

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
+83 −96
Draft

ValueFlow: avoid various unnecessary copies #7583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1539e61
vf_settokenvalue.cpp: avoid unnecessary copies with `setTokenValueCas…
firewave Jun 16, 2024
208ce8e
valueflow.cpp: avoid unnecessary copies in `valueFlowContainerSize()`
firewave Aug 31, 2024
f216f12
vf_settokenvalue.cpp: avoid unnecessary objects in `setTokenValue()`
firewave Aug 31, 2024
01b8de4
valueflow.cpp: avoid unnecessary copies in `valueFlowFunctionReturn()`
firewave Aug 31, 2024
f7139d9
valueflow.cpp: avoid unnecessary copy in `valueFlowLifetime()`
firewave Aug 31, 2024
fa42d5d
programmemory.cpp: added comment
firewave Aug 31, 2024
18bf89c
valueflow.cpp: avoid unnecessary copy in `valueFlowSymbolicOperators()`
firewave Sep 29, 2024
14f1242
ValueFlow: avoid unnecessary copy in `getLifetimeObjValue()`
firewave Dec 25, 2024
0729894
valueflow.cpp: avoid unnecessary copies with `valueFlowForwardConst()`
firewave Dec 25, 2024
234e25f
valueflow.cpp: avoid unnecessary copy in `valueFlowAfterAssign()`
firewave Dec 25, 2024
0890a09
valueflow.cpp: avoid unnecessary copies in `valueFlowInferCondition()`
firewave Dec 25, 2024
eb35e83
valueflow.cpp: avoid unnecessary copy in `valueFlowContainerSize()`
firewave Dec 25, 2024
6ac7b71
valueflow.cpp: avoid unnecessary copy in `valueFlowSafeFunctions()`
firewave Dec 25, 2024
0c79d5b
valueflow.cpp: avoid unnecessary copies with `valueFlowInjectParamete…
firewave Jan 8, 2025
7c863a4
programmemory.cpp: avoid unnecessary copy in `programMemoryParseCondi…
firewave Aug 7, 2024
56287cc
vf_settokenvalue.cpp: void unnecessary copy in `setTokenValue()`
firewave Jun 10, 2025
762ebed
valueflow.cpp: avoid unnecessary copies in `valueFlowFunctionReturn()`
firewave Jun 10, 2025
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
10 changes: 6 additions & 4 deletions lib/programmemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ std::size_t ExprIdToken::Hash::operator()(ExprIdToken etok) const
void ProgramMemory::setValue(const Token* expr, const ValueFlow::Value& value) {
copyOnWrite();

(*mValues)[expr] = value;
(*mValues)[expr] = value; // copy
ValueFlow::Value subvalue = value;
const Token* subexpr = solveExprValue(
expr,
Expand Down Expand Up @@ -327,11 +327,13 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con
if (endTok && findExpressionChanged(vartok, tok->next(), endTok, settings))
return;
const bool impossible = (tok->str() == "==" && !then) || (tok->str() == "!=" && then);
const ValueFlow::Value& v = then ? truevalue : falsevalue;
pm.setValue(vartok, impossible ? asImpossible(v) : v);
ValueFlow::Value& v = then ? truevalue : falsevalue;
const auto iv = v.intvalue;
// cppcheck-suppress accessMoved - FP #13628
pm.setValue(vartok, impossible ? asImpossible(std::move(v)) : v);
const Token* containerTok = settings.library.getContainerFromYield(vartok, Library::Container::Yield::SIZE);
if (containerTok)
pm.setContainerSizeValue(containerTok, v.intvalue, !impossible);
pm.setContainerSizeValue(containerTok, iv, !impossible);
} else if (Token::simpleMatch(tok, "!")) {
programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then);
} else if (then && Token::simpleMatch(tok, "&&")) {
Expand Down
73 changes: 31 additions & 42 deletions lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1644,7 +1644,7 @@ ValueFlow::Value ValueFlow::getLifetimeObjValue(const Token *tok, bool inconclus
// There should only be one lifetime
if (values.size() != 1)
return ValueFlow::Value{};
return values.front();
return std::move(values.front());
}

template<class Predicate>
Expand Down Expand Up @@ -3115,10 +3115,10 @@ static void valueFlowLifetime(TokenList &tokenlist, ErrorLogger &errorLogger, co
else if (tok->isUnaryOp("&")) {
if (Token::simpleMatch(tok->astParent(), "*"))
continue;
for (const ValueFlow::LifetimeToken& lt : ValueFlow::getLifetimeTokens(tok->astOperand1(), settings)) {
for (ValueFlow::LifetimeToken& lt : ValueFlow::getLifetimeTokens(tok->astOperand1(), settings)) {
if (!settings.certainty.isEnabled(Certainty::inconclusive) && lt.inconclusive)
continue;
ErrorPath errorPath = lt.errorPath;
ErrorPath& errorPath = lt.errorPath;
errorPath.emplace_back(tok, "Address of variable taken here.");

ValueFlow::Value value;
Expand Down Expand Up @@ -3761,7 +3761,7 @@ static void valueFlowSymbolicOperators(const SymbolDatabase& symboldatabase, con
continue;

ValueFlow::Value v = makeSymbolic(arg);
v.errorPath = c.errorPath;
v.errorPath = std::move(c.errorPath);
v.errorPath.emplace_back(tok, "Passed to " + tok->str());
if (c.intvalue == 0)
v.setImpossible();
Expand Down Expand Up @@ -3901,16 +3901,15 @@ template<class ContainerOfValue>
static void valueFlowForwardConst(Token* start,
const Token* end,
const Variable* var,
const ContainerOfValue& values,
const Settings& settings,
int /*unused*/ = 0)
ContainerOfValue values,
const Settings& settings)
{
if (!precedes(start, end))
throw InternalError(var->nameToken(), "valueFlowForwardConst: start token does not precede the end token.");
for (Token* tok = start; tok != end; tok = tok->next()) {
if (tok->varId() == var->declarationId()) {
for (const ValueFlow::Value& value : values)
setTokenValue(tok, value, settings);
for (ValueFlow::Value& value : values)
Copy link
Owner

@danmar danmar Jun 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if more then one token has matching varid and this inner loop is executed twice then you will move values that are already moved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I was relying on the selfcheck/clang-tidy to catch these things.

setTokenValue(tok, std::move(value), settings);
} else {
[&] {
// Follow references
Expand All @@ -3919,7 +3918,7 @@ static void valueFlowForwardConst(Token* start,
return ref.token->varId() == var->declarationId();
});
if (it != refs.end()) {
for (ValueFlow::Value value : values) {
for (ValueFlow::Value& value : values) {
if (refs.size() > 1)
value.setInconclusive();
value.errorPath.insert(value.errorPath.end(), it->errors.cbegin(), it->errors.cend());
Expand All @@ -3935,7 +3934,7 @@ static void valueFlowForwardConst(Token* start,
continue;
if (v.tokvalue->varId() != var->declarationId())
continue;
for (ValueFlow::Value value : values) {
for (ValueFlow::Value& value : values) {
if (!v.isKnown() && value.isImpossible())
continue;
if (v.intvalue != 0) {
Expand All @@ -3955,15 +3954,6 @@ static void valueFlowForwardConst(Token* start,
}
}

static void valueFlowForwardConst(Token* start,
const Token* end,
const Variable* var,
const std::initializer_list<ValueFlow::Value>& values,
const Settings& settings)
{
valueFlowForwardConst(start, end, var, values, settings, 0);
}

static ValueFlow::Value::Bound findVarBound(const Variable* var,
const Token* start,
const Token* end,
Expand Down Expand Up @@ -4091,7 +4081,7 @@ static void valueFlowForwardAssign(Token* const tok,
});
std::list<ValueFlow::Value> constValues;
constValues.splice(constValues.end(), values, it, values.end());
valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), constValues, settings);
valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), std::move(constValues), settings);
}
if (isInitialVarAssign(expr)) {
// Check if variable is only incremented or decremented
Expand All @@ -4107,7 +4097,7 @@ static void valueFlowForwardAssign(Token* const tok,
value.bound = b;
value.invertRange();
value.setImpossible();
valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), {std::move(value)}, settings);
valueFlowForwardConst(nextExpression, endOfVarScope, expr->variable(), std::list<ValueFlow::Value>{std::move(value)}, settings);
}
}
}
Expand Down Expand Up @@ -4315,7 +4305,7 @@ static void valueFlowAfterAssign(TokenList &tokenlist,
continue;
ids.insert(value.tokvalue->exprId());
}
for (ValueFlow::Value value : values) {
for (ValueFlow::Value& value : values) {
if (!value.isSymbolicValue())
continue;
const Token* expr = value.tokvalue;
Expand Down Expand Up @@ -5158,7 +5148,7 @@ static void valueFlowInferCondition(TokenList& tokenlist, const Settings& settin
for (const ValuePtr<InferModel>& model : iteratorModels) {
std::vector<ValueFlow::Value> result =
infer(model, tok->str(), tok->astOperand1()->values(), tok->astOperand2()->values());
for (ValueFlow::Value value : result) {
for (ValueFlow::Value& value : result) {
value.valueType = ValueFlow::Value::ValueType::INT;
setTokenValue(tok, std::move(value), settings);
}
Expand All @@ -5176,8 +5166,7 @@ static void valueFlowInferCondition(TokenList& tokenlist, const Settings& settin
std::vector<ValueFlow::Value> result = infer(makeIntegralInferModel(), "!=", tok->values(), 0);
if (result.size() != 1)
continue;
ValueFlow::Value value = result.front();
setTokenValue(tok, std::move(value), settings);
setTokenValue(tok, std::move(result.front()), settings);
}
}
}
Expand Down Expand Up @@ -5596,7 +5585,7 @@ static void valueFlowInjectParameter(const TokenList& tokenlist,
const Settings& settings,
const Variable* arg,
const Scope* functionScope,
const std::list<ValueFlow::Value>& argvalues)
std::list<ValueFlow::Value> argvalues)
{
// Is argument passed by value or const reference, and is it a known non-class type?
if (arg->isReference() && !arg->isConst() && !arg->isClass())
Expand All @@ -5610,7 +5599,7 @@ static void valueFlowInjectParameter(const TokenList& tokenlist,
valueFlowForward(const_cast<Token*>(functionScope->bodyStart->next()),
functionScope->bodyEnd,
arg->nameToken(),
argvalues,
std::move(argvalues),
tokenlist,
errorLogger,
settings);
Expand Down Expand Up @@ -5836,7 +5825,7 @@ static void valueFlowFunctionDefaultParameter(const TokenList& tokenlist, const
argvalues.push_back(std::move(v));
}
if (!argvalues.empty())
valueFlowInjectParameter(tokenlist, errorLogger, settings, var, scope, argvalues);
valueFlowInjectParameter(tokenlist, errorLogger, settings, var, scope, std::move(argvalues));
}
}
}
Expand Down Expand Up @@ -5921,10 +5910,10 @@ static void valueFlowFunctionReturn(TokenList& tokenlist, ErrorLogger& errorLogg

bool hasKnownValue = false;

for (const ValueFlow::Value& v : getCommonValuesFromTokens(returns)) {
setFunctionReturnValue(function, tok, v, settings, false);
for (ValueFlow::Value& v : getCommonValuesFromTokens(returns)) {
if (v.isKnown())
hasKnownValue = true;
setFunctionReturnValue(function, tok, std::move(v), settings, false);
}

if (hasKnownValue)
Expand All @@ -5950,10 +5939,10 @@ static void valueFlowFunctionReturn(TokenList& tokenlist, ErrorLogger& errorLogg
if (programMemory.empty() && !arguments.empty())
continue;
std::vector<ValueFlow::Value> values = execute(function->functionScope, programMemory, settings);
for (const ValueFlow::Value& v : values) {
for (ValueFlow::Value& v : values) {
if (v.isUninitValue())
continue;
setFunctionReturnValue(function, tok, v, settings);
setFunctionReturnValue(function, tok, std::move(v), settings);
}
}
}
Expand Down Expand Up @@ -6675,7 +6664,7 @@ static void valueFlowContainerSetTokValue(const TokenList& tokenlist, ErrorLogge
value.setKnown();
Token* start = initList->link() ? initList->link() : initList->next();
if (tok->variable() && tok->variable()->isConst()) {
valueFlowForwardConst(start, tok->variable()->scope()->bodyEnd, tok->variable(), {std::move(value)}, settings);
valueFlowForwardConst(start, tok->variable()->scope()->bodyEnd, tok->variable(), std::list<ValueFlow::Value>{std::move(value)}, settings);
} else {
valueFlowForward(start, tok, std::move(value), tokenlist, errorLogger, settings);
}
Expand Down Expand Up @@ -6759,8 +6748,8 @@ static void valueFlowContainerSize(const TokenList& tokenlist,
continue;
}

for (const ValueFlow::Value& value : values) {
valueFlowForward(nameToken->next(), var->nameToken(), value, tokenlist, errorLogger, settings);
for (ValueFlow::Value& value : values) {
valueFlowForward(nameToken->next(), var->nameToken(), std::move(value), tokenlist, errorLogger, settings);
}
}

Expand Down Expand Up @@ -6806,8 +6795,8 @@ static void valueFlowContainerSize(const TokenList& tokenlist,
const Token* constructorArgs = tok;
values = getContainerSizeFromConstructor(constructorArgs, tok->valueType(), settings, true);
}
for (const ValueFlow::Value& value : values)
setTokenValue(tok, value, settings);
for (ValueFlow::Value& value : values)
setTokenValue(tok, std::move(value), settings);
} else if (Token::Match(tok, ";|{|} %var% =") && Token::Match(tok->tokAt(2)->astOperand2(), "[({]") &&
// init list
((tok->tokAt(2) == tok->tokAt(2)->astOperand2()->astParent() && !tok->tokAt(2)->astOperand2()->astOperand2() && tok->tokAt(2)->astOperand2()->str() == "{") ||
Expand All @@ -6820,8 +6809,8 @@ static void valueFlowContainerSize(const TokenList& tokenlist,
Token* rhs = tok->tokAt(2)->astOperand2();
std::vector<ValueFlow::Value> values = getInitListSize(rhs, containerTok->valueType(), settings);
valueFlowContainerSetTokValue(tokenlist, errorLogger, settings, containerTok, rhs);
for (const ValueFlow::Value& value : values)
valueFlowForward(containerTok->next(), containerTok, value, tokenlist, errorLogger, settings);
for (ValueFlow::Value& value : values)
valueFlowForward(containerTok->next(), containerTok, std::move(value), tokenlist, errorLogger, settings);
}
} else if (Token::Match(tok, ". %name% (") && tok->astOperand1() && tok->astOperand1()->valueType() &&
tok->astOperand1()->valueType()->container) {
Expand Down Expand Up @@ -7085,8 +7074,8 @@ static void valueFlowSafeFunctions(const TokenList& tokenlist, const SymbolDatab
argValues.back().valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming " + arg.name() + " size is 1000000");
argValues.back().safe = true;
for (const ValueFlow::Value &value : argValues)
valueFlowForward(const_cast<Token*>(functionScope->bodyStart), arg.nameToken(), value, tokenlist, errorLogger, settings);
for (ValueFlow::Value &value : argValues)
valueFlowForward(const_cast<Token*>(functionScope->bodyStart), arg.nameToken(), std::move(value), tokenlist, errorLogger, settings);
continue;
}

Expand Down
Loading
Loading