Skip to content

avoid some unnecessary copies around ErrorMessage construction #4665

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

Merged
firewave merged 1 commit into from
Jun 24, 2025
Merged

avoid some unnecessary copies around ErrorMessage construction #4665

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
2 changes: 1 addition & 1 deletion gui/checkthread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ void CheckThread::parseClangErrors(const QString &tool, const QString &file0, QS
const std::string f0 = file0.toStdString();
const std::string msg = e.message.toStdString();
const std::string id = e.errorId.toStdString();
ErrorMessage errmsg(callstack, f0, e.severity, msg, id, Certainty::normal);
ErrorMessage errmsg(std::move(callstack), f0, e.severity, msg, id, Certainty::normal);
mResult.reportErr(errmsg);
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/check.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ void Check::reportError(const std::list<const Token *> &callstack, Severity seve
writeToErrorList(errmsg);
}

void Check::reportError(const ErrorPath &errorPath, Severity severity, const char id[], const std::string &msg, const CWE &cwe, Certainty certainty)
void Check::reportError(ErrorPath errorPath, Severity severity, const char id[], const std::string &msg, const CWE &cwe, Certainty certainty)
{
// TODO: report debug warning when error is for a disabled severity
const ErrorMessage errmsg(errorPath, mTokenizer ? &mTokenizer->list : nullptr, severity, id, msg, cwe, certainty);
const ErrorMessage errmsg(std::move(errorPath), mTokenizer ? &mTokenizer->list : nullptr, severity, id, msg, cwe, certainty);
if (mErrorLogger)
mErrorLogger->reportErr(errmsg);
else
Expand Down
7 changes: 1 addition & 6 deletions lib/check.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,10 @@ class CPPCHECKLIB Check {
reportError(callstack, severity, id, msg, cwe, certainty);
}

/** report an error */
void reportError(const std::list<const Token *> &callstack, Severity severity, const std::string &id, const std::string &msg) {
reportError(callstack, severity, id, msg, CWE(0U), Certainty::normal);
}

/** report an error */
void reportError(const std::list<const Token *> &callstack, Severity severity, const std::string &id, const std::string &msg, const CWE &cwe, Certainty certainty);

void reportError(const ErrorPath &errorPath, Severity severity, const char id[], const std::string &msg, const CWE &cwe, Certainty certainty);
void reportError(ErrorPath errorPath, Severity severity, const char id[], const std::string &msg, const CWE &cwe, Certainty certainty);

/** log checker */
void logChecker(const char id[]);
Expand Down
16 changes: 8 additions & 8 deletions lib/checkautovariables.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const Val
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string msg = "Returning " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "returnDanglingLifetime", msg + " that will be invalid when returning.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
reportError(std::move(errorPath), Severity::error, "returnDanglingLifetime", msg + " that will be invalid when returning.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
}

void CheckAutoVariables::errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val)
Expand All @@ -704,7 +704,7 @@ void CheckAutoVariables::errorInvalidLifetime(const Token *tok, const ValueFlow:
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string msg = "Using " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "invalidLifetime", msg + " that is out of scope.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
reportError(std::move(errorPath), Severity::error, "invalidLifetime", msg + " that is out of scope.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
}

void CheckAutoVariables::errorDanglingTemporaryLifetime(const Token* tok, const ValueFlow::Value* val, const Token* tempTok)
Expand All @@ -714,7 +714,7 @@ void CheckAutoVariables::errorDanglingTemporaryLifetime(const Token* tok, const
std::string msg = "Using " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tempTok, "Temporary created here.");
errorPath.emplace_back(tok, "");
reportError(errorPath,
reportError(std::move(errorPath),
Severity::error,
"danglingTemporaryLifetime",
msg + " that is a temporary.",
Expand All @@ -730,21 +730,21 @@ void CheckAutoVariables::errorDanglngLifetime(const Token *tok, const ValueFlow:
std::string msg = isStatic ? "Static" : "Non-local";
msg += " variable '" + tokName + "' will use " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "danglingLifetime", msg + ".", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
reportError(std::move(errorPath), Severity::error, "danglingLifetime", msg + ".", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
}

void CheckAutoVariables::errorDanglingTempReference(const Token* tok, ErrorPath errorPath, bool inconclusive)
{
errorPath.emplace_back(tok, "");
reportError(
errorPath, Severity::error, "danglingTempReference", "Using reference to dangling temporary.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
std::move(errorPath), Severity::error, "danglingTempReference", "Using reference to dangling temporary.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
}

void CheckAutoVariables::errorReturnReference(const Token* tok, ErrorPath errorPath, bool inconclusive)
{
errorPath.emplace_back(tok, "");
reportError(
errorPath, Severity::error, "returnReference", "Reference to local variable returned.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
std::move(errorPath), Severity::error, "returnReference", "Reference to local variable returned.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
}

void CheckAutoVariables::errorDanglingReference(const Token *tok, const Variable *var, ErrorPath errorPath)
Expand All @@ -753,14 +753,14 @@ void CheckAutoVariables::errorDanglingReference(const Token *tok, const Variable
std::string varName = var ? var->name() : "y";
std::string msg = "Non-local reference variable '" + tokName + "' to local variable '" + varName + "'";
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "danglingReference", msg, CWE562, Certainty::normal);
reportError(std::move(errorPath), Severity::error, "danglingReference", msg, CWE562, Certainty::normal);
}

void CheckAutoVariables::errorReturnTempReference(const Token* tok, ErrorPath errorPath, bool inconclusive)
{
errorPath.emplace_back(tok, "");
reportError(
errorPath, Severity::error, "returnTempReference", "Reference to temporary returned.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
std::move(errorPath), Severity::error, "returnTempReference", "Reference to temporary returned.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
}

void CheckAutoVariables::errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val)
Expand Down
12 changes: 6 additions & 6 deletions lib/checkbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,7 @@ void CheckBufferOverrun::argumentSizeError(const Token *tok, const std::string &
errorPath.emplace_back(paramVar->nameToken(), "Passing buffer '" + paramVar->name() + "' to function that is declared here");
errorPath.emplace_back(tok, "");

reportError(errorPath, Severity::warning, "argumentSize",
reportError(std::move(errorPath), Severity::warning, "argumentSize",
"$symbol:" + functionName + '\n' +
"Buffer '" + paramExpression + "' is too small, the function '" + functionName + "' expects a bigger buffer in " + strParamNum + " argument", CWE_ARGUMENT_SIZE, Certainty::normal);
}
Expand Down Expand Up @@ -1014,7 +1014,7 @@ bool CheckBufferOverrun::analyseWholeProgram1(const std::map<std::string, std::l
{
const CTU::FileInfo::FunctionCall *functionCall = nullptr;

const std::list<ErrorMessage::FileLocation> &locationList =
std::list<ErrorMessage::FileLocation> locationList =
CTU::FileInfo::getErrorPath(CTU::FileInfo::InvalidValueType::bufferOverflow,
unsafeUsage,
callsMap,
Expand Down Expand Up @@ -1042,7 +1042,7 @@ bool CheckBufferOverrun::analyseWholeProgram1(const std::map<std::string, std::l
cwe = CWE_POINTER_ARITHMETIC_OVERFLOW;
}

const ErrorMessage errorMessage(locationList,
const ErrorMessage errorMessage(std::move(locationList),
file0,
Severity::error,
errmsg,
Expand Down Expand Up @@ -1138,7 +1138,7 @@ void CheckBufferOverrun::objectIndexError(const Token *tok, const ValueFlow::Val
}
errorPath.emplace_back(tok, "");
std::string verb = known ? "is" : "might be";
reportError(errorPath,
reportError(std::move(errorPath),
known ? Severity::error : Severity::warning,
"objectIndex",
"The address of variable '" + name + "' " + verb + " accessed at non-zero index.",
Expand Down Expand Up @@ -1204,9 +1204,9 @@ void CheckBufferOverrun::negativeArraySizeError(const Token* tok)
void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token* tok, const ValueFlow::Value* value)
{
const std::string msg = "Memory allocation size is negative.";
const ErrorPath errorPath = getErrorPath(tok, value, msg);
ErrorPath errorPath = getErrorPath(tok, value, msg);
const bool inconclusive = value != nullptr && !value->isKnown();
reportError(errorPath, inconclusive ? Severity::warning : Severity::error, "negativeMemoryAllocationSize",
reportError(std::move(errorPath), inconclusive ? Severity::warning : Severity::error, "negativeMemoryAllocationSize",
msg, CWE131, inconclusive ? Certainty::inconclusive : Certainty::normal);
}

Expand Down
14 changes: 7 additions & 7 deletions lib/checkclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2971,7 +2971,7 @@ void CheckClass::virtualFunctionCallInConstructorError(
}
}

reportError(errorPath, Severity::style, "virtualCallInConstructor",
reportError(std::move(errorPath), Severity::style, "virtualCallInConstructor",
"Virtual function '" + funcname + "' is called from " + scopeFunctionTypeName + " '" + constructorName + "' at line " + std::to_string(lineNumber) + ". Dynamic binding is not used.", CWE(0U), Certainty::normal);
}

Expand All @@ -2989,7 +2989,7 @@ void CheckClass::pureVirtualFunctionCallInConstructorError(
if (!errorPath.empty())
errorPath.back().second = purefuncname + " is a pure virtual function without body";

reportError(errorPath, Severity::warning, "pureVirtualCall",
reportError(std::move(errorPath), Severity::warning, "pureVirtualCall",
"$symbol:" + purefuncname +"\n"
"Call of pure virtual function '$symbol' in " + scopeFunctionTypeName + ".\n"
"Call of pure virtual function '$symbol' in " + scopeFunctionTypeName + ". The call will fail during runtime.", CWE(0U), Certainty::normal);
Expand Down Expand Up @@ -3120,7 +3120,7 @@ void CheckClass::duplInheritedMembersError(const Token *tok1, const Token* tok2,
const std::string message = "The " + std::string(derivedIsStruct ? "struct" : "class") + " '" + derivedName +
"' defines member " + member + " with name '" + memberName + "' also defined in its parent " +
std::string(baseIsStruct ? "struct" : "class") + " '" + baseName + "'.";
reportError(errorPath, Severity::warning, "duplInheritedMember", symbols + '\n' + message, CWE398, Certainty::normal);
reportError(std::move(errorPath), Severity::warning, "duplInheritedMember", symbols + '\n' + message, CWE398, Certainty::normal);
}


Expand Down Expand Up @@ -3230,7 +3230,7 @@ void CheckClass::overrideError(const Function *funcInBase, const Function *funcI
errorPath.emplace_back(funcInDerived->tokenDef, char(std::toupper(funcType[0])) + funcType.substr(1) + " in derived class");
}

reportError(errorPath, Severity::style, "missingOverride",
reportError(std::move(errorPath), Severity::style, "missingOverride",
"$symbol:" + functionName + "\n"
"The " + funcType + " '$symbol' overrides a " + funcType + " in a base class but is not marked with a 'override' specifier.",
CWE(0U) /* Unknown CWE! */,
Expand All @@ -3254,7 +3254,7 @@ void CheckClass::uselessOverrideError(const Function *funcInBase, const Function
}
else
errStr += "just delegates back to the base class.";
reportError(errorPath, Severity::style, "uselessOverride",
reportError(std::move(errorPath), Severity::style, "uselessOverride",
"$symbol:" + functionName +
errStr,
CWE(0U) /* Unknown CWE! */,
Expand Down Expand Up @@ -3525,10 +3525,10 @@ bool CheckClass::checkThisUseAfterFreeRecursive(const Scope *classScope, const F
void CheckClass::thisUseAfterFree(const Token *self, const Token *free, const Token *use)
{
std::string selfPointer = self ? self->str() : "ptr";
const ErrorPath errorPath = { ErrorPathItem(self, "Assuming '" + selfPointer + "' is used as 'this'"), ErrorPathItem(free, "Delete '" + selfPointer + "', invalidating 'this'"), ErrorPathItem(use, "Call method when 'this' is invalid") };
ErrorPath errorPath = { ErrorPathItem(self, "Assuming '" + selfPointer + "' is used as 'this'"), ErrorPathItem(free, "Delete '" + selfPointer + "', invalidating 'this'"), ErrorPathItem(use, "Call method when 'this' is invalid") };
const std::string usestr = use ? use->str() : "x";
const std::string usemsg = use && use->function() ? ("Calling method '" + usestr + "()'") : ("Using member '" + usestr + "'");
reportError(errorPath, Severity::warning, "thisUseAfterFree",
reportError(std::move(errorPath), Severity::warning, "thisUseAfterFree",
"$symbol:" + selfPointer + "\n" +
usemsg + " when 'this' might be invalid",
CWE(0), Certainty::normal);
Expand Down
20 changes: 10 additions & 10 deletions lib/checkcondition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ void CheckCondition::duplicateConditionError(const Token *tok1, const Token *tok

std::string msg = "The if condition is the same as the previous if condition";

reportError(errorPath, Severity::style, "duplicateCondition", msg, CWE398, Certainty::normal);
reportError(std::move(errorPath), Severity::style, "duplicateCondition", msg, CWE398, Certainty::normal);
}

void CheckCondition::multiCondition()
Expand Down Expand Up @@ -590,7 +590,7 @@ void CheckCondition::oppositeElseIfConditionError(const Token *ifCond, const Tok
errorPath.emplace_back(ifCond, "first condition");
errorPath.emplace_back(elseIfCond, "else if condition is opposite to first condition");

reportError(errorPath, Severity::style, "multiCondition", errmsg.str(), CWE398, Certainty::normal);
reportError(std::move(errorPath), Severity::style, "multiCondition", errmsg.str(), CWE398, Certainty::normal);
}

//---------------------------------------------------------------------------
Expand Down Expand Up @@ -867,7 +867,7 @@ void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token*

const std::string msg("Opposite inner '" + innerSmt + "' condition leads to a dead code block.\n"
"Opposite inner '" + innerSmt + "' condition leads to a dead code block (outer condition is '" + s1 + "' and inner condition is '" + s2 + "').");
reportError(errorPath, Severity::warning, "oppositeInnerCondition", msg, CWE398, Certainty::normal);
reportError(std::move(errorPath), Severity::warning, "oppositeInnerCondition", msg, CWE398, Certainty::normal);
}

void CheckCondition::identicalInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath)
Expand All @@ -882,7 +882,7 @@ void CheckCondition::identicalInnerConditionError(const Token *tok1, const Token

const std::string msg("Identical inner '" + innerSmt + "' condition is always true.\n"
"Identical inner '" + innerSmt + "' condition is always true (outer condition is '" + s1 + "' and inner condition is '" + s2 + "').");
reportError(errorPath, Severity::warning, "identicalInnerCondition", msg, CWE398, Certainty::normal);
reportError(std::move(errorPath), Severity::warning, "identicalInnerCondition", msg, CWE398, Certainty::normal);
}

void CheckCondition::identicalConditionAfterEarlyExitError(const Token *cond1, const Token* cond2, ErrorPath errorPath)
Expand All @@ -898,7 +898,7 @@ void CheckCondition::identicalConditionAfterEarlyExitError(const Token *cond1, c
errorPath.emplace_back(cond1, "If condition '" + cond + "' is true, the function will return/exit");
errorPath.emplace_back(cond2, (isReturnValue ? "Returning identical expression '" : "Testing identical condition '") + cond + "'");

reportError(errorPath,
reportError(std::move(errorPath),
Severity::warning,
"identicalConditionAfterEarlyExit",
isReturnValue
Expand Down Expand Up @@ -1353,12 +1353,12 @@ void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::st
return;
errors.emplace_back(tok, "");
if (always)
reportError(errors, Severity::warning, "incorrectLogicOperator",
reportError(std::move(errors), Severity::warning, "incorrectLogicOperator",
"Logical disjunction always evaluates to true: " + condition + ".\n"
"Logical disjunction always evaluates to true: " + condition + ". "
"Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?", CWE571, inconclusive ? Certainty::inconclusive : Certainty::normal);
else
reportError(errors, Severity::warning, "incorrectLogicOperator",
reportError(std::move(errors), Severity::warning, "incorrectLogicOperator",
"Logical conjunction always evaluates to false: " + condition + ".\n"
"Logical conjunction always evaluates to false: " + condition + ". "
"Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?", CWE570, inconclusive ? Certainty::inconclusive : Certainty::normal);
Expand Down Expand Up @@ -1647,8 +1647,8 @@ void CheckCondition::alwaysTrueFalseError(const Token* tok, const Token* conditi
const std::string expr = tok ? tok->expressionString() : std::string("x");
const std::string conditionStr = (Token::simpleMatch(condition, "return") ? "Return value" : "Condition");
const std::string errmsg = conditionStr + " '" + expr + "' is always " + bool_to_string(alwaysTrue);
const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
reportError(errorPath,
ErrorPath errorPath = getErrorPath(tok, value, errmsg);
reportError(std::move(errorPath),
Severity::style,
"knownConditionTrueFalse",
errmsg,
Expand Down Expand Up @@ -1880,7 +1880,7 @@ void CheckCondition::duplicateConditionalAssignError(const Token *condTok, const
}

reportError(
errors, Severity::style, "duplicateConditionalAssign", msg, CWE398, Certainty::normal);
std::move(errors), Severity::style, "duplicateConditionalAssign", msg, CWE398, Certainty::normal);
}


Expand Down
Loading
Loading