From 12e58c8521b4101a76b084cc32164514ec37094c Mon Sep 17 00:00:00 2001 From: lordylike Date: Sun, 22 Jul 2018 15:01:18 +0200 Subject: [PATCH] fix ticket 8570: passedByValue with member initializer list and std::move (#1316) * fix ticket 8570 allow member initializer list variables that are moved to be non-const * review feedback * replace tabs with spaces in test code --- lib/checkother.cpp | 15 ++++++++++ test/testother.cpp | 72 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 5405f0944..d2a90124a 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1354,6 +1354,21 @@ static std::size_t estimateSize(const Type* type, const Settings* settings, cons static bool canBeConst(const Variable *var) { + { // check initializer list. If variable is moved from it can't be const. + const Function* func_scope = var->scope()->function; + if (func_scope->type == Function::Type::eConstructor) { + //could be initialized in initializer list + if (func_scope->arg->link()->next()->str() == ":") { + for (const Token* tok2 = func_scope->arg->link()->next()->next(); tok2 != var->scope()->bodyStart; tok2 = tok2->next()) { + if (tok2->varId() != var->declarationId()) + continue; + const Token* parent = tok2->astParent(); + if (parent && Token::simpleMatch(parent->previous(), "move (")) + return false; + } + } + } + } for (const Token* tok2 = var->scope()->bodyStart; tok2 != var->scope()->bodyEnd; tok2 = tok2->next()) { if (tok2->varId() != var->declarationId()) continue; diff --git a/test/testother.cpp b/test/testother.cpp index af0590329..8f0cee4d0 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -165,6 +165,7 @@ private: TEST_CASE(checkCastIntToCharAndBack); // ticket #160 TEST_CASE(checkCommaSeparatedReturn); + TEST_CASE(checkPassByReference); TEST_CASE(checkComparisonFunctionIsAlwaysTrueOrFalse); @@ -6024,6 +6025,77 @@ private: ASSERT_EQUALS("", errout.str()); } + void checkPassByReference() { + // #8570 passByValue when std::move is used + check("struct A\n" + "{\n" + " std::vector x;\n" + "};\n" + "\n" + "struct B\n" + "{\n" + " explicit B(A a) : a(std::move(a)) {}\n" + " void Init(A _a) { a = std::move(_a); }\n" + " A a;" + "};", nullptr, false, false, true); + ASSERT_EQUALS("", errout.str()); + + check("struct A\n" + "{\n" + " std::vector x;\n" + "};\n" + "\n" + "struct B\n" + "{\n" + " explicit B(A a) : a{std::move(a)} {}\n" + " void Init(A _a) { a = std::move(_a); }\n" + " A a;" + "};", nullptr, false, false, true); + ASSERT_EQUALS("", errout.str()); + + check("struct A\n" + "{\n" + " std::vector x;\n" + "};\n" + "\n" + "struct B\n" + "{\n" + " B(A a, A a2) : a{std::move(a)}, a2{std::move(a2)} {}\n" + " void Init(A _a) { a = std::move(_a); }\n" + " A a;" + " A a2;" + "};", nullptr, false, false, true); + ASSERT_EQUALS("", errout.str()); + + check("struct A\n" + "{\n" + " std::vector x;\n" + "};\n" + "\n" + "struct B\n" + "{\n" + " B(A a, A a2) : a{std::move(a)}, a2{a2} {}\n" + " void Init(A _a) { a = std::move(_a); }\n" + " A a;" + " A a2;" + "};", nullptr, false, false, true); + ASSERT_EQUALS("[test.cpp:8]: (performance) Function parameter 'a2' should be passed by const reference.\n", errout.str()); + + check("struct A\n" + "{\n" + " std::vector x;\n" + "};\n" + "\n" + "struct B\n" + "{\n" + " B(A a, A a2) : a{std::move(a)}, a2(a2) {}\n" + " void Init(A _a) { a = std::move(_a); }\n" + " A a;" + " A a2;" + "};", nullptr, false, false, true); + ASSERT_EQUALS("[test.cpp:8]: (performance) Function parameter 'a2' should be passed by const reference.\n", errout.str()); + } + void checkComparisonFunctionIsAlwaysTrueOrFalse() { // positive test check("bool f(int x){\n"