[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019
+  // Ensure that `-Wunused-variable` will be emitted for condition variables
+  // that are not referenced later. e.g.: if (int var = init());
+  if (!T->isAggregateType())
+    ConditionVar->setReferenced(/*R=*/false);
----------------
`isAggregateType()` includes arrays and I think we still want to diagnose an unused array.


================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5348
       NewVar->getDeclContext()->isFunctionOrMethod() &&
-      OldVar->getType()->isDependentType())
+      OldVar->getType()->isDependentType() && !OldVar->isImplicit())
     DiagnoseUnusedDecl(NewVar);
----------------
Which test case covers this change?


================
Comment at: clang/test/SemaCXX/warn-unused-variables.cpp:2
 // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -Wno-c++1z-extensions -verify -std=c++11 %s
 template<typename T> void f() {
----------------



================
Comment at: clang/test/SemaCXX/warn-unused-variables.cpp:317-318
+      (void)used;
+    else if (int used1 = 8; int used2 = 9)
+      (void)(used1 + used2);
+  }
----------------
This should diagnose the decomposition declaration as being unused, but that already works today (so let's make sure we don't get duplicate diagnostics for this case): https://godbolt.org/z/6jP6sW1x6

An additional test case after this would be to add:
```
else if (auto [a, b] = (int[2]){ 1, 2 }; a)
  return;
```
which should get no diagnostics.

We should also have these tests for the other kinds of conditions (for, switch).


================
Comment at: clang/test/SemaCXX/warn-unused-variables.cpp:321
+
+  void fors() {
+    for (int i = 0;int unused = 0;); // expected-warning {{unused variable 'i'}} \
----------------
Can you also add coverage for `switch`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152495/new/

https://reviews.llvm.org/D152495