fix ticket 8570: passedByValue with member initializer list and std::move (danmar#1316)

lordylike · danmar · commit 12e58c8521b4 · 2018-07-22T15:01:18.000+02:00
* fix ticket 8570

allow member initializer list variables that are moved to be non-const

* review feedback

* replace tabs with spaces in test code
diff --git 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
@@ -165,6 +165,7 @@ class TestOther : public TestFixture {
         TEST_CASE(checkCastIntToCharAndBack); // ticket #160
 
         TEST_CASE(checkCommaSeparatedReturn);
+        TEST_CASE(checkPassByReference);
 
         TEST_CASE(checkComparisonFunctionIsAlwaysTrueOrFalse);
 
@@ -6024,6 +6025,77 @@ class TestOther : public TestFixture {
         ASSERT_EQUALS("", errout.str());
     }
 
+	void checkPassByReference() {
+		// #8570 passByValue when std::move is used
+		check("struct A\n"
+			"{\n"
+			"    std::vector<int> 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<int> 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<int> 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<int> 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<int> 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"
