From a656f10e47ff21860c74bfca0e194634f410fa33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Mon, 12 Feb 2024 19:28:42 +0100 Subject: [PATCH] avoid some unnecessary copies (#5958) this is mainly based on the warnings of the unfinished `performance-unnecessary-copy-on-last-use` clang-tidy check (see https://github.com/llvm/llvm-project/issues/53489). it also includes some fixes spotted by review as well as some adjustments to the previous Coverity fixes --- cli/cmdlineparser.cpp | 14 +++++++------- gui/mainwindow.cpp | 2 +- gui/projectfiledialog.cpp | 2 +- gui/resultstree.cpp | 2 +- lib/astutils.cpp | 4 ++-- lib/astutils.h | 2 +- lib/checkbufferoverrun.cpp | 2 +- lib/forwardanalyzer.cpp | 4 ++-- lib/importproject.cpp | 2 +- lib/pathanalysis.cpp | 2 +- lib/symboldatabase.cpp | 8 ++++---- lib/symboldatabase.h | 8 ++++---- lib/valueflow.cpp | 8 ++++---- test/testerrorlogger.cpp | 35 +++++++++++++++++------------------ test/testfilelister.cpp | 2 +- test/testimportproject.cpp | 2 +- test/testpathmatch.cpp | 12 ++++++------ 17 files changed, 55 insertions(+), 56 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index d981081bc87..ee1e0d75175 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -93,8 +93,8 @@ static bool addIncludePathsToList(const std::string& fileList, std::listgetCheckStartTime(); mThread->check(checkSettings); mUI->mResults->setCheckSettings(checkSettings); - mThread->setCheckStartTime(saveCheckStartTime); + mThread->setCheckStartTime(std::move(saveCheckStartTime)); } void MainWindow::reAnalyze(bool all) diff --git a/gui/projectfiledialog.cpp b/gui/projectfiledialog.cpp index b305343931a..6ffc4722bf1 100644 --- a/gui/projectfiledialog.cpp +++ b/gui/projectfiledialog.cpp @@ -485,7 +485,7 @@ void ProjectFileDialog::saveToProjectFile(ProjectFile *projectFile) const codingStandards << CODING_STANDARD_MISRA_CPP_2008; if (mUI->mAutosar->isChecked()) codingStandards << CODING_STANDARD_AUTOSAR; - projectFile->setCodingStandards(codingStandards); + projectFile->setCodingStandards(std::move(codingStandards)); projectFile->setCertIntPrecision(mUI->mEditCertIntPrecision->text().toInt()); projectFile->setBughunting(mUI->mBughunting->isChecked()); projectFile->setClangAnalyzer(mUI->mToolClangAnalyzer->isChecked()); diff --git a/gui/resultstree.cpp b/gui/resultstree.cpp index cfefd73ceb3..155a5645999 100644 --- a/gui/resultstree.cpp +++ b/gui/resultstree.cpp @@ -972,7 +972,7 @@ void ResultsTree::recheckSelectedFiles() selectedItems<previous(), "> (") && tok->astOperand2() && tok->astOperand1() && isCPPCastKeyword(tok->astOperand1()); } -bool isConstVarExpression(const Token *tok, std::function skipPredicate) +bool isConstVarExpression(const Token *tok, const std::function& skipPredicate) { if (!tok) return false; @@ -3255,7 +3255,7 @@ bool isConstVarExpression(const Token *tok, std::function sk if (Token::Match(tok, "%cop%|[|.")) { if (tok->astOperand1() && !isConstVarExpression(tok->astOperand1(), skipPredicate)) return false; - if (tok->astOperand2() && !isConstVarExpression(tok->astOperand2(), std::move(skipPredicate))) + if (tok->astOperand2() && !isConstVarExpression(tok->astOperand2(), skipPredicate)) return false; return true; } diff --git a/lib/astutils.h b/lib/astutils.h index 6280965e9cd..e39b61b33d6 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -437,7 +437,7 @@ bool isLikelyStreamRead(bool cpp, const Token *op); bool isCPPCast(const Token* tok); -bool isConstVarExpression(const Token* tok, std::function skipPredicate = nullptr); +bool isConstVarExpression(const Token* tok, const std::function& skipPredicate = nullptr); bool isLeafDot(const Token* tok); diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index d510678ed2e..0dbb06562ff 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -388,7 +388,7 @@ static std::string arrayIndexMessage(const Token* tok, auto add_dim = [](const std::string &s, const Dimension &dim) { return s + "[" + std::to_string(dim.num) + "]"; }; - const std::string array = std::accumulate(dimensions.cbegin(), dimensions.cend(), tok->astOperand1()->expressionString(), add_dim); + const std::string array = std::accumulate(dimensions.cbegin(), dimensions.cend(), tok->astOperand1()->expressionString(), std::move(add_dim)); std::ostringstream errmsg; if (condition) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 1dcd68a9a4e..0dfe80895f8 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -116,7 +116,7 @@ namespace { } template )> - Progress traverseTok(T* tok, F f, bool traverseUnknown, T** out = nullptr) { + Progress traverseTok(T* tok, const F &f, bool traverseUnknown, T** out = nullptr) { if (Token::Match(tok, "asm|goto")) return Break(Analyzer::Terminate::Bail); if (Token::Match(tok, "setjmp|longjmp (")) { @@ -166,7 +166,7 @@ namespace { } template )> - Progress traverseRecursive(T* tok, F f, bool traverseUnknown, unsigned int recursion=0) { + Progress traverseRecursive(T* tok, const F &f, bool traverseUnknown, unsigned int recursion=0) { if (!tok) return Progress::Continue; if (recursion > 10000) diff --git a/lib/importproject.cpp b/lib/importproject.cpp index ea3878ebe07..e5849d3743f 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -137,7 +137,7 @@ static bool simplifyPathWithVariables(std::string &s, std::map& tokensThatAreNotEnumeratorValues) const; - void setValueType(Token* tok, const ValueType& valuetype, SourceLocation loc = SourceLocation::current()); - void setValueType(Token* tok, const Variable& var, SourceLocation loc = SourceLocation::current()); - void setValueType(Token* tok, const Enumerator& enumerator, SourceLocation loc = SourceLocation::current()); + void setValueType(Token* tok, const ValueType& valuetype, const SourceLocation &loc = SourceLocation::current()); + void setValueType(Token* tok, const Variable& var, const SourceLocation &loc = SourceLocation::current()); + void setValueType(Token* tok, const Enumerator& enumerator, const SourceLocation &loc = SourceLocation::current()); Tokenizer& mTokenizer; const Settings &mSettings; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index d9d6d5d63f3..062dfcc868e 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -807,7 +807,7 @@ static void setTokenValue(Token* tok, if (ret) return; - ValueFlow::Value v(value); + ValueFlow::Value v(std::move(value)); v.conditional = true; v.changeKnownToPossible(); @@ -1053,7 +1053,7 @@ static void setTokenValue(Token* tok, } else if (Token::Match(parent, ":: %name%") && parent->astOperand2() == tok) { - setTokenValue(parent, value, settings); + setTokenValue(parent, std::move(value), settings); } // Calling std::size or std::empty on an array else if (value.isTokValue() && Token::simpleMatch(value.tokvalue, "{") && tok->variable() && @@ -1061,12 +1061,12 @@ static void setTokenValue(Token* tok, std::vector args = getArguments(value.tokvalue); if (const Library::Function* f = settings.library.getFunction(parent->previous())) { if (f->containerYield == Library::Container::Yield::SIZE) { - ValueFlow::Value v(value); + ValueFlow::Value v(std::move(value)); v.valueType = ValueFlow::Value::ValueType::INT; v.intvalue = args.size(); setTokenValue(parent, std::move(v), settings); } else if (f->containerYield == Library::Container::Yield::EMPTY) { - ValueFlow::Value v(value); + ValueFlow::Value v(std::move(value)); v.intvalue = args.empty(); v.valueType = ValueFlow::Value::ValueType::INT; setTokenValue(parent, std::move(v), settings); diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index ddd7d61e514..d02bfa5acdc 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -115,7 +115,7 @@ class TestErrorLogger : public TestFixture { void ErrorMessageConstruct() const { std::list locs(1, fooCpp5); - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.", "errorId", Certainty::normal); ASSERT_EQUALS(1, msg.callStack.size()); ASSERT_EQUALS("Programming error.", msg.shortMessage()); ASSERT_EQUALS("Programming error.", msg.verboseMessage()); @@ -125,7 +125,7 @@ class TestErrorLogger : public TestFixture { void ErrorMessageConstructLocations() const { std::list locs = { fooCpp5, barCpp8 }; - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.", "errorId", Certainty::normal); ASSERT_EQUALS(2, msg.callStack.size()); ASSERT_EQUALS("Programming error.", msg.shortMessage()); ASSERT_EQUALS("Programming error.", msg.verboseMessage()); @@ -135,7 +135,7 @@ class TestErrorLogger : public TestFixture { void ErrorMessageVerbose() const { std::list locs(1, fooCpp5); - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); ASSERT_EQUALS(1, msg.callStack.size()); ASSERT_EQUALS("Programming error.", msg.shortMessage()); ASSERT_EQUALS("Verbose error", msg.verboseMessage()); @@ -145,7 +145,7 @@ class TestErrorLogger : public TestFixture { void ErrorMessageVerboseLocations() const { std::list locs = { fooCpp5, barCpp8 }; - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); ASSERT_EQUALS(2, msg.callStack.size()); ASSERT_EQUALS("Programming error.", msg.shortMessage()); ASSERT_EQUALS("Verbose error", msg.verboseMessage()); @@ -185,7 +185,7 @@ class TestErrorLogger : public TestFixture { void CustomFormat() const { std::list locs(1, fooCpp5); - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); ASSERT_EQUALS(1, msg.callStack.size()); ASSERT_EQUALS("Programming error.", msg.shortMessage()); ASSERT_EQUALS("Verbose error", msg.verboseMessage()); @@ -195,7 +195,7 @@ class TestErrorLogger : public TestFixture { void CustomFormat2() const { std::list locs(1, fooCpp5); - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); ASSERT_EQUALS(1, msg.callStack.size()); ASSERT_EQUALS("Programming error.", msg.shortMessage()); ASSERT_EQUALS("Verbose error", msg.verboseMessage()); @@ -206,7 +206,7 @@ class TestErrorLogger : public TestFixture { void CustomFormatLocations() const { // Check that first location from location stack is used in template std::list locs = { fooCpp5, barCpp8 }; - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); ASSERT_EQUALS(2, msg.callStack.size()); ASSERT_EQUALS("Programming error.", msg.shortMessage()); ASSERT_EQUALS("Verbose error", msg.verboseMessage()); @@ -216,7 +216,7 @@ class TestErrorLogger : public TestFixture { void ToXmlV2() const { std::list locs(1, fooCpp5); - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); std::string header("\n\n"); header += " locs = { fooCpp5, barCpp8 }; locs.back().setinfo("ä"); - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal); std::string header("\n\n"); header += " locs; - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nComparing \"\203\" with \"\003\"", "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error.\nComparing \"\203\" with \"\003\"", "errorId", Certainty::normal); const std::string expected(" "); ASSERT_EQUALS(expected, msg.toXML()); } { const char code1[]="äöü"; const char code2[]="\x12\x00\x00\x01"; - std::list locs; - ErrorMessage msg1(locs, emptyString, Severity::error, std::string("Programming error.\nReading \"")+code1+"\"", "errorId", Certainty::normal); + ErrorMessage msg1({}, emptyString, Severity::error, std::string("Programming error.\nReading \"")+code1+"\"", "errorId", Certainty::normal); ASSERT_EQUALS(" ", msg1.toXML()); - ErrorMessage msg2(locs, emptyString, Severity::error, std::string("Programming error.\nReading \"")+code2+"\"", "errorId", Certainty::normal); + ErrorMessage msg2({}, emptyString, Severity::error, std::string("Programming error.\nReading \"")+code2+"\"", "errorId", Certainty::normal); ASSERT_EQUALS(" ", msg2.toXML()); } } @@ -301,7 +300,7 @@ class TestErrorLogger : public TestFixture { std::list locs(1, fooCpp5); // Inconclusive error message - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); // xml version 2 error message ASSERT_EQUALS(" \n" @@ -313,7 +312,7 @@ class TestErrorLogger : public TestFixture { void SerializeInconclusiveMessage() const { // Inconclusive error message std::list locs; - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); msg.file0 = "test.cpp"; const std::string msg_str = msg.serialize(); @@ -402,7 +401,7 @@ class TestErrorLogger : public TestFixture { void SerializeSanitize() const { std::list locs; - ErrorMessage msg(locs, emptyString, Severity::error, std::string("Illegal character in \"foo\001bar\""), "errorId", Certainty::normal); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, std::string("Illegal character in \"foo\001bar\""), "errorId", Certainty::normal); msg.file0 = "1.c"; const std::string msg_str = msg.serialize(); @@ -429,9 +428,9 @@ class TestErrorLogger : public TestFixture { loc1.setfile("[]:;,()"); loc1.setinfo("abcd:/,"); - std::list locs{loc1}; + std::list locs{std::move(loc1)}; - ErrorMessage msg(locs, emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); + ErrorMessage msg(std::move(locs), emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); const std::string msg_str = msg.serialize(); ASSERT_EQUALS("7 errorId" diff --git a/test/testfilelister.cpp b/test/testfilelister.cpp index 60ebc78fcce..c2adf2438d6 100644 --- a/test/testfilelister.cpp +++ b/test/testfilelister.cpp @@ -60,7 +60,7 @@ class TestFileLister : public TestFixture { // Recursively add add files.. std::list> files; std::vector masks; - PathMatch matcher(masks); + PathMatch matcher(std::move(masks)); std::string err = FileLister::recursiveAddFiles(files, adddir, matcher); ASSERT_EQUALS("", err); diff --git a/test/testimportproject.cpp b/test/testimportproject.cpp index df88f8ce1ca..049e399a561 100644 --- a/test/testimportproject.cpp +++ b/test/testimportproject.cpp @@ -374,7 +374,7 @@ class TestImportProject : public TestFixture { fs1.filename = "foo/bar"; fs2.filename = "qwe/rty"; TestImporter project; - project.fileSettings = {fs1, fs2}; + project.fileSettings = {std::move(fs1), std::move(fs2)}; project.ignorePaths({"*foo", "bar*"}); ASSERT_EQUALS(2, project.fileSettings.size()); diff --git a/test/testpathmatch.cpp b/test/testpathmatch.cpp index 3ed2aede94a..4aa580b478e 100644 --- a/test/testpathmatch.cpp +++ b/test/testpathmatch.cpp @@ -92,7 +92,7 @@ class TestPathMatch : public TestFixture { void onemasksamepathdifferentcase() const { std::vector masks(1, "sRc/"); - PathMatch match(masks, false); + PathMatch match(std::move(masks), false); ASSERT(match.match("srC/")); } @@ -139,25 +139,25 @@ class TestPathMatch : public TestFixture { void twomasklongerpath1() const { std::vector masks = { "src/", "module/" }; - PathMatch match(masks); + PathMatch match(std::move(masks)); ASSERT(!match.match("project/")); } void twomasklongerpath2() const { std::vector masks = { "src/", "module/" }; - PathMatch match(masks); + PathMatch match(std::move(masks)); ASSERT(match.match("project/src/")); } void twomasklongerpath3() const { std::vector masks = { "src/", "module/" }; - PathMatch match(masks); + PathMatch match(std::move(masks)); ASSERT(match.match("project/module/")); } void twomasklongerpath4() const { std::vector masks = { "src/", "module/" }; - PathMatch match(masks); + PathMatch match(std::move(masks)); ASSERT(match.match("project/src/module/")); } @@ -168,7 +168,7 @@ class TestPathMatch : public TestFixture { void filemaskdifferentcase() const { std::vector masks(1, "foo.cPp"); - PathMatch match(masks, false); + PathMatch match(std::move(masks), false); ASSERT(match.match("fOo.cpp")); }