[clang] 8e329ca - Reland "[Clang][SemaCXX] Add unused warning for variables declared in condition expressions"

Takuya Shimizu via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 15 01:24:51 PDT 2023


Author: Takuya Shimizu
Date: 2023-08-15T17:24:13+09:00
New Revision: 8e329caa944c377c51ef567d5aa67cfac9ffd0fa

URL: https://github.com/llvm/llvm-project/commit/8e329caa944c377c51ef567d5aa67cfac9ffd0fa
DIFF: https://github.com/llvm/llvm-project/commit/8e329caa944c377c51ef567d5aa67cfac9ffd0fa.diff

LOG: Reland "[Clang][SemaCXX] Add unused warning for variables declared in condition expressions"

This patch marks the declarations with initializations in condition expressions such as
if (int var = init) as unreferenced so that -Wunused can warn on them.

Fixes https://github.com/llvm/llvm-project/issues/61681

Reviewed By: cor3ntin
Differential Revision: https://reviews.llvm.org/D152495

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaDecl.cpp
    clang/lib/Sema/SemaExprCXX.cpp
    clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
    clang/test/SemaCXX/warn-unused-variables.cpp
    llvm/lib/Support/VirtualFileSystem.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cd7beff546c932..96a9ec14112292 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -129,6 +129,9 @@ Improvements to Clang's diagnostics
   of a base class is not called in the constructor of its derived class.
 - Clang no longer emits ``-Wmissing-variable-declarations`` for variables declared
   with the ``register`` storage class.
+- Clang now warns on unused variables declared and initialized in condition
+  expressions.
+  (`#61681: <https://github.com/llvm/llvm-project/issues/61681>`_)
 
 Bug Fixes in This Version
 -------------------------

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0ad807cca36c7b..db1cd28f7b5a93 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1995,7 +1995,7 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions &LangOpts,
       return false;
   } else if (!D->getDeclName()) {
     return false;
-  } else if (D->isReferenced() || D->isUsed()) {
+  } else if (D->isReferenced() || (!isa<VarDecl>(D) && D->isUsed())) {
     return false;
   }
 

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index e6eb9508f8e6ff..17fda20c1ca11c 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4015,6 +4015,10 @@ ExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar,
       ConditionVar, ConditionVar->getType().getNonReferenceType(), VK_LValue,
       ConditionVar->getLocation());
 
+  // Ensure that `-Wunused-variable` will be emitted for condition variables
+  // that are not referenced later. e.g.: if (int var = init());
+  ConditionVar->setReferenced(/*R=*/false);
+
   switch (CK) {
   case ConditionKind::Boolean:
     return CheckBooleanCondition(StmtLoc, Condition.get());

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index f78d46f5950360..522ec12de3d368 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5352,7 +5352,7 @@ void Sema::BuildVariableInstantiation(
   // will have been deferred.
   if (!NewVar->isInvalidDecl() &&
       NewVar->getDeclContext()->isFunctionOrMethod() &&
-      OldVar->getType()->isDependentType())
+      OldVar->getType()->isDependentType() && !OldVar->isImplicit())
     DiagnoseUnusedDecl(NewVar);
 }
 

diff  --git a/clang/test/SemaCXX/warn-unused-variables.cpp b/clang/test/SemaCXX/warn-unused-variables.cpp
index 4db8bdf12e5de0..db33086436d3af 100644
--- a/clang/test/SemaCXX/warn-unused-variables.cpp
+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
@@ -1,5 +1,5 @@
 // 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++14-extensions -Wno-c++17-extensions -verify -std=c++11 %s
 template<typename T> void f() {
   T t;
   t = 17;
@@ -294,3 +294,115 @@ void RAIIWrapperTest() {
 }
 
 } // namespace gh54489
+
+namespace inside_condition {
+  void ifs() {
+    if (int hoge = 0) // expected-warning {{unused variable 'hoge'}}
+      return;
+    if (const int const_hoge = 0) // expected-warning {{unused variable 'const_hoge'}}
+      return;
+    else if (int fuga = 0)
+      (void)fuga;
+    else if (int used = 1; int catched = used) // expected-warning {{unused variable 'catched'}}
+      return;
+    else if (int refed = 1; int used = refed)
+      (void)used;
+    else if (int unused1 = 2; int unused2 = 3) // expected-warning {{unused variable 'unused1'}} \
+                                               // expected-warning {{unused variable 'unused2'}}
+      return;
+    else if (int unused = 4; int used = 5) // expected-warning {{unused variable 'unused'}}
+      (void)used;
+    else if (int used = 6; int unused = 7) // expected-warning {{unused variable 'unused'}}
+      (void)used;
+    else if (int used1 = 8; int used2 = 9)
+      (void)(used1 + used2);
+    else if (auto [a, b] = (int[2]){ 1, 2 }; 1) // expected-warning {{unused variable '[a, b]'}}
+      return;
+    else if (auto [a, b] = (int[2]){ 1, 2 }; a)
+      return;
+  }
+
+  void fors() {
+    for (int i = 0;int unused = 0;); // expected-warning {{unused variable 'i'}} \
+                                     // expected-warning {{unused variable 'unused'}}
+    for (int i = 0;int used = 0;) // expected-warning {{unused variable 'i'}}
+      (void)used;
+      while(int var = 1) // expected-warning {{unused variable 'var'}}
+        return;
+  }
+
+  void whiles() {
+    while(int unused = 1) // expected-warning {{unused variable 'unused'}}
+      return;
+    while(int used = 1)
+      (void)used;
+  }
+
+
+  void switches() {
+    switch(int unused = 1) { // expected-warning {{unused variable 'unused'}}
+      case 1: return;
+    }
+    switch(constexpr int used = 3; int unused = 4) { // expected-warning {{unused variable 'unused'}}
+      case used: return;
+    }
+    switch(int used = 3; int unused = 4) { // expected-warning {{unused variable 'unused'}}
+      case 3: (void)used;
+    }
+    switch(constexpr int used1 = 0; constexpr int used2 = 6) {
+      case (used1+used2): return;
+    }
+    switch(auto [a, b] = (int[2]){ 1, 2 }; 1) { // expected-warning {{unused variable '[a, b]'}}
+      case 1: return;
+    }
+    switch(auto [a, b] = (int[2]){ 1, 2 }; b) {
+      case 1: return;
+    }
+    switch(auto [a, b] = (int[2]){ 1, 2 }; 1) {
+      case 1: (void)a;
+    }
+  }
+  template <typename T>
+  struct Vector {
+    void doIt() {
+      for (auto& e : elements){} // expected-warning {{unused variable 'e'}}
+    }
+    T elements[10];
+  };
+  void ranged_for() {
+    Vector<int>    vector;
+    vector.doIt(); // expected-note {{here}}
+  }
+
+
+  struct RAII {
+    int &x;
+    RAII(int &ref) : x(ref) {}
+    ~RAII() { x = 0;}
+    operator int() const { return 1; }
+  };
+  void aggregate() {
+    int x = 10;
+    int y = 10;
+    if (RAII var = x) {}
+    for(RAII var = x; RAII var2 = y;) {}
+    while (RAII var = x) {}
+    switch (RAII var = x) {}
+  }
+
+  struct TrivialDtor{
+    int &x;
+    TrivialDtor(int &ref) : x(ref) { ref = 32; }
+    operator int() const { return 1; }
+  };
+  void trivial_dtor() {
+    int x = 10;
+    int y = 10;
+    if (TrivialDtor var = x) {} // expected-warning {{unused variable 'var'}}
+    for(TrivialDtor var = x; TrivialDtor var2 = y;) {} // expected-warning {{unused variable 'var'}} \
+                                         // expected-warning {{unused variable 'var2'}}
+    while (TrivialDtor var = x) {} // expected-warning {{unused variable 'var'}}
+    switch (TrivialDtor var = x) {} // expected-warning {{unused variable 'var'}}
+  }
+
+} // namespace inside_condition

diff  --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index d381d79fba9663..e36151038b2d33 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -1337,7 +1337,7 @@ std::error_code RedirectingFileSystem::isLocal(const Twine &Path_,
   SmallString<256> Path;
   Path_.toVector(Path);
 
-  if (std::error_code EC = makeCanonical(Path))
+  if (makeCanonical(Path))
     return {};
 
   return ExternalFS->isLocal(Path, Result);


        


More information about the cfe-commits mailing list