Merge pull request #4592 from geoffw0/varnotused

C++: Work around two false positive issues with the UnusedLocals.ql query

Author: jbj

cpp/change-notes/2020-11-02-unused-local-variable.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Two issues causing the 'Unused local variable' query (`cpp/unused-local-variable`) to produce false positive results have been fixed.

cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql

@@ -57,5 +57,12 @@ where
   not declarationHasSideEffects(v) and
   not exists(AsmStmt s | f = s.getEnclosingFunction()) and
   not v.getAnAttribute().getName() = "unused" and
-  not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr likely used `v`
+  not any(ErrorExpr e).getEnclosingFunction() = f and // unextracted expr may use `v`
+  not exists(
+    Literal l // this case can be removed when the `myFunction2( [obj](){} );` test case doesn't depend on this exclusion
+  |
+    l.getEnclosingFunction() = f and
+    not exists(l.getValue())
+  ) and
+  not any(ConditionDeclExpr cde).getEnclosingFunction() = f // this case can be removed when the `if (a = b; a)` test case doesn't depend on this exclusion
 select v, "Variable " + v.getName() + " is not used"

cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedLocals/UnusedLocals.expected

@@ -1,10 +1,12 @@
-| code2.cpp:4:6:4:7 | v1 | Variable v1 is not used |
-| code2.cpp:6:6:6:7 | v3 | Variable v3 is not used |
-| code2.cpp:10:16:10:17 | v7 | Variable v7 is not used |
-| code2.cpp:25:16:25:17 | v1 | Variable v1 is not used |
-| code2.cpp:26:16:26:17 | v2 | Variable v2 is not used |
-| code2.cpp:41:11:41:16 | myVar1 | Variable myVar1 is not used |
-| code2.cpp:63:7:63:8 | v3 | Variable v3 is not used |
+| code2.cpp:5:6:5:7 | v1 | Variable v1 is not used |
+| code2.cpp:7:6:7:7 | v3 | Variable v3 is not used |
+| code2.cpp:11:16:11:17 | v7 | Variable v7 is not used |
+| code2.cpp:26:16:26:17 | v1 | Variable v1 is not used |
+| code2.cpp:27:16:27:17 | v2 | Variable v2 is not used |
+| code2.cpp:42:11:42:16 | myVar1 | Variable myVar1 is not used |
+| code2.cpp:64:7:64:8 | v3 | Variable v3 is not used |
+| code2.cpp:108:11:108:12 | v2 | Variable v2 is not used |
+| code2.cpp:128:9:128:9 | b | Variable b is not used |
 | code.c:10:18:10:18 | y | Variable y is not used |
 | code.c:11:18:11:18 | z | Variable z is not used |
 | code.c:18:7:18:7 | x | Variable x is not used |

cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedLocals/code2.cpp

@@ -1,3 +1,4 @@
+// semmle-extractor-options: -std=c++17
 
 int test_const_init()
 {
@@ -76,3 +77,89 @@ void test_expect()
 		}
 	}
 }
+
+// ---
+
+template<class T>
+class MyContainer
+{
+public:
+	struct Iterator {
+		const T& operator*() const;
+		bool operator!=(const Iterator &rhs) const;
+		Iterator operator++();
+	};
+
+	Iterator begin();
+	Iterator end();
+};
+
+void output(int value);
+
+void test_range_based_for()
+{
+	MyContainer<int> myContainer;
+	
+	for (int v1 : myContainer) // GOOD: v1 is used
+	{
+		output(v1);
+	}
+	
+	for (int v2 : myContainer) // BAD: v2 is not used
+	{
+	}
+}
+
+// ---
+
+int test_lambdas1()
+{
+	int a, b, c, d, e; // (b is not used, but is explicitly captured)
+	auto myLambda = [a, b, &c](int x, int y) -> int // (y is not used, but is a parameter)
+	{
+		return a + c + x;
+	};
+
+	return myLambda(d, e);
+}
+
+int test_lambdas2()
+{
+	int a, b; // BAD: b is not used
+	auto myLambda = [=]() -> int // BAD: myLambda is not used [NOT DETECTED] (due to containing a Constructor)
+	{
+		return a;
+	};
+
+	return 0;
+}
+
+// ---
+
+void test_if_initializer()
+{
+	bool a = false, b = true; // GOOD: a, b are both used
+
+	if (a = b; a)
+	{
+		// ...
+	}
+}
+
+// ---
+
+class MyObj
+{
+public:
+	MyObj();
+};
+
+template<class T>
+void myFunction2(T t);
+
+void test_captured_contructor()
+{
+	const auto &obj = MyObj(); // GOOD: obj is used
+
+	myFunction2( [obj](){} );
+}