From eca7ee92600501a9a34378f5b20329f55e5ad813 Mon Sep 17 00:00:00 2001
From: Ken-Patrick Lehrmann <kp.lehrmann+github@gmail.com>
Date: Sat, 4 Jan 2020 11:36:45 +0100
Subject: [PATCH] 9356: Prevent false positive when passing non-const reference
 to member constructor (#2370)

* Add cases for 9356

* 9356: Prevent false positive when passing non-const reference to member constructor

This workarounds false positives 'Parameter  can be declared with const [constParameter]'
when said parameter is used in constructor call. It assume the
constructor call might change the parameter (without any checks.
The drawback is that we have false negative, in cases where we could
check the constructor actually takes a const reference, or a copied by
value parameter.

* Add todo comment in isVariableMutableInInitializer
---
 lib/checkother.cpp |  4 +++
 test/testother.cpp | 84 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index ef52024db..91763c6a8 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -1299,6 +1299,10 @@ static bool isVariableMutableInInitializer(const Token* start, const Token * end
             const Token * memberTok = tok->astParent()->previous();
             if (Token::Match(memberTok, "%var% (") && memberTok->variable()) {
                 const Variable * memberVar = memberTok->variable();
+                if(memberVar->isClass())
+                    //TODO: check if the called constructor could live with a const variable
+                    // pending that, assume the worst (that it can't)
+                    return true;
                 if (!memberVar->isReference())
                     continue;
                 if (memberVar->isConst())
diff --git a/test/testother.cpp b/test/testother.cpp
index 6bbf2bc37..715a5a6ac 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -2107,6 +2107,90 @@ private:
               "void an();\n"
               "void h();\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("class C\n"
+              "{\n"
+              "public:\n"
+              "  explicit C(int&);\n"
+              "};\n"
+              "\n"
+              "class D\n"
+              "{\n"
+              "public:\n"
+              "  explicit D(int&);\n"
+              "\n"
+              "private:\n"
+              "  C c;\n"
+              "};\n"
+              "\n"
+              "D::D(int& i)\n"
+              "  : c(i)\n"
+              "{\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("class C\n"
+              "{\n"
+              "public:\n"
+              "  explicit C(const int&);\n"
+              "};\n"
+              "\n"
+              "class D\n"
+              "{\n"
+              "public:\n"
+              "  explicit D(int&);\n"
+              "\n"
+              "private:\n"
+              "  C c;\n"
+              "};\n"
+              "\n"
+              "D::D(int& i)\n"
+              "  : c(i)\n"
+              "{\n"
+              "}\n");
+        TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared with const\n", "", errout.str());
+
+        check("class C\n"
+              "{\n"
+              "public:\n"
+              "  explicit C(int);\n"
+              "};\n"
+              "\n"
+              "class D\n"
+              "{\n"
+              "public:\n"
+              "  explicit D(int&);\n"
+              "\n"
+              "private:\n"
+              "  C c;\n"
+              "};\n"
+              "\n"
+              "D::D(int& i)\n"
+              "  : c(i)\n"
+              "{\n"
+              "}\n");
+        TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared with const\n", "", errout.str());
+
+        check("class C\n"
+              "{\n"
+              "public:\n"
+              "  explicit C(int, int);\n"
+              "};\n"
+              "\n"
+              "class D\n"
+              "{\n"
+              "public:\n"
+              "  explicit D(int&);\n"
+              "\n"
+              "private:\n"
+              "  C c;\n"
+              "};\n"
+              "\n"
+              "D::D(int& i)\n"
+              "  : c(0, i)\n"
+              "{\n"
+              "}\n");
+        TODO_ASSERT_EQUALS("[test.cpp:16]: (style) Parameter 'i' can be declared with const\n", "", errout.str());
     }
 
     void switchRedundantAssignmentTest() {